Module Registry

[mdtusz]

@alexcjohnson @etpinard @cldougl

Implementing a registry will be a big help for modularity and reducing the bundle size for people using only a few traces. The question is, how do we want to implement it.

This is one of the ways I have thought about how we could do it, but it would be good to see what you all have in mind as well.

[etpinard]

@mdtusz Looks like you got all the cases covered in that gist.

A few comments (related to #41 (comment)):

• For v1, trace modules and d3 will be register-able. All component modules will be part of core the file. So, we should drop the fetchComponent handler and add d3 key in the register opts i.e. Plotly.register({d3: require('d3')}).
• trace modules upon init will have their required register plot module (e.g. scatter3d will have to register the gl3d plot module). Looks like this could be easily added to your current init step.
• Implementing the require('plotly.js/core') signature will require building the files into the repo's root or is there a package.json trick to link components?

[mdtusz]

Could you elaborate what you mean on the last bullet point?

My thought was that we would have an index file that "builds" a standard version available via require('plotly.js');, but if you are a power user, you can opt to use require('plotly.js/core'); and go from there.

[etpinard]

My thought was that we would have an index file that "builds" a standard version available via require('plotly.js');

Correct and the location of this "dist" file is given by the main field in the package.json.

but if you are a power user, you can opt to use require('plotly.js/core'); and go from there.

That'd be great. But, how do we tell the bundlers where plotly.js/core is located?

[mdtusz]

require's resolve to the node_modules directory if there's no relative pathing given, so the power users would just need to require('plotly.js/core') and it will use the code from whatever file is at node_modules/plotly.js/core/index.js or node_modules/plotly.js/core.js. It just means we'll need to add a core file (or whatever nomenclature we decide on) at the root of the npm package that either mirrors the contents of the main entry, except without any registered traces/components, or perhaps even better, the main entry requires core and just registers the traces. The benefit to this is that our internal build will be written in the exact same way an end user would use the core and anyone looking under the hood will see exactly what's going on.

A similar type of file could be created for the traces if we want to clean up the interface so that users can write things like require('plotly.js/traces').scatter or whatnot - in that scenario, it would just be a file that exports an object of the traces.

e.g.

// plotly.js/index.js
var Core = require('./core');
var Traces = require('./traces');

for(trace in Traces){
  Core.register({ trace: Traces[trace] });
}

// despite the name Core, this would be all of Plotly
module.exports = Core;

// plotly.js/core.js
var Core = {};

// add everything as needed

module.exports = Core;

// plotly.js/traces.js
var Traces = {
  scatter: require('./src/traces/scatter'),
  box: require('./src/traces/box')
  // etc.
}

module.exports = Traces;

[etpinard]

It just means we'll need to add a core file (or whatever nomenclature we decide on) at the root of the npm package that mirrors the contents of the main entry

Right. That's what I wanted to discuss. Adding this core would require an extra build step. We'll have to choose the best way to do so. Moreover, will these component (e.g. core.js) files be checked into git or only publish on npm?

Personally, I'd vote for checking them into git, so that users can look at their content in github. Having all these component files into a separate directory makes more sense to me than polluting the root level. For example, putting them into a ./lib/ directory and requiring them with require('plotly.js/lib/core') , require('plotly.js/lib/scatte3d) ... etc.

That said I have no strong opinion about it. We should maybe look into what other packages are doing.

[mdtusz]

It shouldn't require more of a build step - it will just bundle the code as usual because Browserify will resolve the requires as normal. The "new" index file would work nearly exactly the same as the current plotly.js and index.js files (I think).

We should definitely check the files into the repo though. I'd like to try to keep a clean/minimal path for end users if possible which is why I suggested require('plotly.js/core). If we add a core directory, we can keep it clean by having core/index.js containing that core, then core/traces,js re-exporting the traces, and usage would then be

var Core = require('plotly.js/core');
var Traces = require('plotly.js/core/traces');

and using the main field in the package.json, the regular dist bundle can be wherever/whatever we want.

[etpinard]

It shouldn't require more of a build step - it will just bundle the code as usual because Browserify will resolve the requires as normal

Browserify will do it right. But webpack and the gl modules don't get along (mainly because of glslify).

[etpinard]

I'd like to try to keep a clean/minimal path for end users if possible which is why I suggested

👍 agreed. core doesn't sound quite right though. Maybe parts ?

[mdtusz]

Hmm. I forgot about webpack. It should be possible using glslify-loader but we'd need to test it out. Otherwise you're right and we'd need to add an incremental build step to browserify the traces/components/core.

[etpinard]

@mdtusz We'll have to think also of a clever and consistent way to access registered modules.

e.g.

• in plot_api.js here accessing methods on trace modules
• in plots.js here accessing the style method on each trace module + one component module.

[etpinard]

Moreover, we should get rid of gd._modules entirely and rely only on the registry(ies)

[alexcjohnson]

Just to be clear on what gd._modules is doing though - it's not all modules, it's just the ones you need to draw this plot, because the plot and style steps act on all traces for that module at once. It would be a bit of a waste to call plot and style from every module when you only use one or two.

[etpinard]

@alexcjohnson right, good point. Thanks.

[mdtusz]

Implemented with #180

[gowravshekar]

// in custom-plotly.js
var Plotly = require('plotly.js/lib/core');

// Load in the trace types for pie, and scatter3d
Plotly.register([
    require('plotly.js/lib/pie'),
    require('plotly.js/lib/scatter3d')
]);

module.exports = Plotly;

Unable to pick partial module scatter3d with webpack2.

[etpinard]

@gowravshekar

Have you read

https://github.com/plotly/plotly.js/blob/master/README.md#webpack-usage-with-modules

If so, can you paste your webpack config and the error message you're seeing?

[gowravshekar]

ERROR in ./~/mapbox-gl/package.json
Module parse failed: /node_modules/ify-loader/index.js!/node_modules/mapbox-gl/package.json Unexpected token (2:8)
You may need an appropriate loader to handle this file type.

ERROR in ./~/mapbox-gl-style-spec/reference/v8.min.json
Module parse failed:/node_modules/ify-loader/index.js!/node_modules/mapbox-gl-style-spec/reference/v8.min.json Unexpected token (1:11)
You may need an appropriate loader to handle this file type.

In webpack.config

{ test: /node_modules/, loader: 'ify-loader'},
{ test: /\.json$/, loader: 'json-loader'},

After adding json-loader, the issue is resolved. But why is mapbox-gl included?

[etpinard]

Really, mapbox-gl is included when bundling:

// in custom-plotly.js
var Plotly = require('plotly.js/lib/core');

// Load in the trace types for pie, and scatter3d
Plotly.register([
    require('plotly.js/lib/pie'),
    require('plotly.js/lib/scatter3d')
]);

module.exports = Plotly;

If so, that would be a major bug, but I'm not able to replicate it.

[gowravshekar]

The mapbox-gl is included by aurelia-webpack-plugin. After ignoring plotly.js in its includeDependencies. It worked fine.