Package broken when installing in JSPM

[lyschoening]

Not that plotly would necessarily have to support JSPM, but it's also not possible to shim around the issue because there is neither a true source nor a true distribution format in the NPM package.

The package.json points to src/index.js, which relies on build/ploticon and other files in the build folder. Meanwhile, dist/plotly.js – presumably intended for distribution given the name – begins with var Plotly = require('../src/plotly'); which seems wrong.

[Hypercubed]

After this #18 is merged dist/plotly.js should work in JSPM.

Run jspm install plotlyjs=github:plotly/plotly.js then import using import Plotly from 'plotlyjs/dist/plotly';

Note: Not at the my work computer now so I may have missed something

[etpinard]

We'll make sure the dist/plotly.js import will work in the next release.

[etpinard]

Should be fixed in v1.1.0.

Can you confirm? (I'm having a hard time setting up jspm for some reason).

[lyschoening]

Hi @etpinard. I gave it another try. Unfortunately it doesn't work yet.

The primary issues I am seeing right now:

1. There are sill plenty of require() lookups in the dist/ version that refer to paths in the src/ version (some with incorrect paths):
   • var Plotly = require("../src/plotly");
   • require("../../plots/cartesian/layout_attributes");
   • etc.
2. The list of dependencies in package.json is incomplete. (I assume this works in Node because some of its other dependencies may also 'happen' to have them as dependencies). You require at least the following packages from within plotly.js, without listing them as dependencies:
   • cubic-hermite
   • binary-search-bounds.
   • mat4-interpolate
   • gl-vec3
   • mat4-recompose
   • quat-slerp
   • filtered-vector
   • turntable-camera-controller
   • orbit-camera-controller
   • matrix-camera-contoller
   • alpha-complex
   • simplicial-complex-boundary
   • circumradius
   • circumcenter
   • dup
   • robus-linear-solve
   • boundary-cells
   • reduce-simplicial-complex
   • compare-cell
   • compate-oriented-cell
   • base64-js
   • ieee754
   • is-array
   • monotone-convex-hull-2d
   • incemental-covex-hull
   • affine-hull
   • cwise-compiler
   • uniq
3. The entrypoint ("src/index.js") is of course not right. (It can be overridden as @Hypercubed pointed out, so it's not a breaking issue). If you want to set the entrypoint only for JSPM, though, adding the following to your package.json should do the trick:

    "jspm": {
        "main": "dist/plotly.js"
    }

[Hypercubed]

I have it working with JSPM. Regading your points:

1. All the requires are resolved by the UMD bundle.
2. The dependancies don't matter if you are installing from github and using the dist.
3. Yes, for now you have use overrides in your app or require('plotlyjs/dist/plotly').

[lyschoening]

You're right, that works. Sorry, I failed to see that the dependencies are bundled into the file. So in that case, installation from github (github:plotly/plotly.js) works because installation from github does not try to process dependencies and treats the module format as global; installation from NPM (npm:plotly.js) works with this override:

{
    "main": "dist/plotly.js",
    "format": "global",
    "dependencies": []
}

[Hypercubed]

I think maybe the "dependencies" should be an empty object {}.

Kind of a side note I'm not personally suggesting that anything be added to plotly's package.json (at least at this point) because I still hope to get the CJS and install via npm registry to work directly. Currently plotly includes d3 and other libraries in the bundle, which means my app, if I include the dist/plotly bundle, has a lot of redundant code.

[Hypercubed]

The only thing currently preventing me from using plotly.js with SystemJS/JSPM is glslify-deps. glslify-deps requires graceful-fs here: https://github.com/stackgl/glslify-deps/blob/master/index.js#L2 which causes Uncaught Error: process.binding is not supported . If I comment out that line (likely breaking disabling glsl) plotly runs in the browser.

I don't know enough about glslify to suggest a solution at this point... except maybe modularize the code in a way that separates the glsl stuff from the d3 stuff.

[etpinard]

@Hypercubed the only plotly.js dependency that depends on glslify-deps is glslify @^1.2.0

I'm not sure how jspm updates the dependencies from patch to patch, but using npm, npm install would in this case update the glslify-deps module to @1.2.5 without modifying anything in this repo.

[Hypercubed]

That's correct... but I'm still having some issues.

[Hypercubed]

fs.ReadStream (https://github.com/isaacs/node-graceful-fs/blob/master/graceful-fs.js#L154) is undefined in nodelibs-fs.

[Hypercubed]

Not a great solution but you can force jspm to skip graceful-fs entirely with the override:

      "npm:[email protected]": {
        "main": "fs.js"
      },

I think all the gl plots won't work but d3 will.

[Hypercubed]

Also, it will not work unless the /build files are added to the npm package.

[etpinard]

@Hypercubed

it will not work unless the /build files are added to the npm package.

So, jspm doesn't implement npm's postinstall hook.

Sounds to me like the only robust of using plotly.js with jspm is to use the bundled up dist/plotly.js version.

Is there a way to make dist/plotly.js the default entry for jspm?

[Hypercubed]

That's correct. JSPM explicitly does not run postinstall (jspm/npm#36 (comment)). Building build in postinstall means you need to keep many potential development dependencies as production deps. I would recommend putting the build directory in the package, it can remain ignored from the git. That's just my opinion.

Anyway, yes, the dist file is more robust. The advantage of using the src directly is to avoid module redundancy in the larger app. I felt like I was getting close yesterday...

To make jspm use the dist version you can add this to package.json:

"jspm": {
  "main": "dist/plotly.js"
}

[Hypercubed]

BTW...

here is a jspm test app (install via github): https://gist.github.com/Hypercubed/05c132fc2996fc7795da

install via npm (doesn't work because it is missing the build files): https://gist.github.com/Hypercubed/9bc0a1549d992239aa22

[lyschoening]

@etpinard Unless there is a particular reason for building on installation, the build step could be moved to "prepublish".

[subash-a]

Sounds to me like the only robust of using plotly.js with jspm is to use the bundled up dist/plotly.js version.

The issue here is that multiple dependences are then duplicated (e.g. d3 to list just one). For us this would be very costly.

As @Hypercubed said:

The advantage of using the src directly is to avoid module redundancy in the larger app. I felt like I was getting close yesterday…

We have a working jspm install using src/index as main for a very basic graph (see end of post) that is based on the following override:

{
    "main": "src/index",
    "registry": "npm"
}

It required just two changes:

• graceful-fs 4.1.2 no longer exports ReadStream or WriteStream. If by hand we duplicate these two lines and follow them with:

    fs.ReadStream = ReadStream;
    fs.WriteStream = WriteStream;

This patching by hand was just to make the minimum of changes; clearly this ‘problem’ can be fixed by depending on specific versions etc

• again as mentioned by @Hypercubed, the build files plotcss.js and ploticon.js need to be in place

So it seems if these two problems are addressed, we will be able to install the non-dist version of plotly.js… which will be a big win!

Any thoughts on how best to go about solving these?

import Plotly from "plotly";

// ...
Plotly.newPlot(container, {
    x: ['2013-10-04 22:23:00', '2013-11-04 22:23:00', '2013-12-04 22:23:00'],
    y: [1, 3, 6],
    type: 'scatter'
});

[Hypercubed]

@subash-a The issue regarding graceful-fs (if I recall correctly) is that it requires node's fs which, in jspm, means loading nodelibs-fs. nodelibs-fs does not have a shim for ReadStream or WriteStream. This causes an error when glslify-deps loads graceful-fs. Turns out that glslify-deps doesn't actually need ReadStream or WriteStream because it only uses fs.readFile. My solution is to override glslify-deps to bypass graceful-fs (see here).

However, nodelibs-fs also does not shim readFile. So while this solution (and adding the build files) gets plotly to run all the D3 stuff.. I doubt anything relying on glslify will work. If you want to use the webgl stuff you need to use the dist version.

[myitcv]

nodelibs-fs does not have a shim for ReadStream or WriteStream.... nodelibs-fs also does not shim readFile

@guybedford - please can you help advise on the best way forward?

[Hypercubed]

@myitcv I have doubts that shiming readFile will make glslify work. I think glslify is just not made to run in the browser. Maybe need to build the glsl stuff separately.

[myitcv]

@Hypercubed - I think all we're talking about here is the jspm load of plotly gaining parity in terms of features supported with the dist build. If feature X works in the dist build, we should be able to get X working via a jspm load of src/index

[Hypercubed]

I'm not very familiar with glslify... But my understanding is that it is like Browserify for shaders. When generating the dist files Browserify is used but not bundled. Likewise when using the CJS version Browserify is not used at all. glslify, like Browserify, is usually part of the build step. For webpack they created glslify-loader. I'm speculating that they need a glslify-loader` build step somewhere pre JSPM/SysyemJS.

Feel free to say I miss understand... I may.

[myitcv]

Feel free to say I miss understand... I may.

No, not saying you've misunderstood here at all. Indeed your comments about nodelibs-fs were extremely useful.

Rather that for us (I work with @subash-a), loading the dist version of plotly isn't really an option... because it effectively duplicates all of the dependencies (d3 etc) we already have loaded. Hence why we're keen to get a non-dist solution

[Hypercubed]

Same here, same goal. My point is I think the webgl/gsgl stuff is still going to be a problem even if we solve the above issues. Personally I'm interested in the d3 plotting.... If the webgl stuff were a separate module or somehow allow a modular build I'd be happy.

[guybedford]

I don't have that much knowledge of this use case so I'm not sure how much I can really help. Note that jspm 0.17 will be coming with full server side support so these issues should go away. Since this is only for running in the browser though can you not just do a map: { graceful-fs: '@empty' } override on install of whatever uses graceful-fs? Is it glsify that does this?

[Hypercubed]

@guybedford glslify requires glslify-deps which requires graceful-fs My suggestion was to override graceful-fs to use fs directly (see https://gist.github.com/Hypercubed/9bc0a1549d992239aa22#file-package-json-L11). Which means (with JSPM) using nodelibs-fs. The only method from graceful-fs used by glslify-deps is readFile. That was the reason behind this issue: jspm/nodelibs-fs#2

If this allows glslify to run in the browser is TBD.

[guybedford]

glslify is a browserify transform, so is used on the server when creating a browserify bundle. Replacing fs with a browser fs therefore won't help this use case I don't think? Unless there is a runtime component of glslify that is being used in the browser (which I don't think there is?). So perhaps the issue here really is just that jspm doesn't support browserify transforms, while a dependency of this project requires a glslify transform to run on bundle?

If so, that support is tracking in jspm/npm#10.

[myitcv]

So perhaps the issue here really is just that jspm doesn't support browserify transforms, while a dependency of this project requires a glslify transform to run on bundle?

Sounds like it exactly, thanks for weighing in to pick things apart.

[etpinard]

@lyschoening @Hypercubed

So, jspm doesn't implement npm's postinstall hook.

After #164, the css and svg built files are checked in to git and published on npm. No need to worry about the npm postinstall hook.

[etpinard]

Using our "sub" npm dist package (added in #2670, which have 0️⃣ dependency) should be a breeze with jspm, so I'll close this issue, but please notify us if that's not the case.