Trace Module Functionality

[mdtusz]

There isn't much actual change in code, more just rearrangement and a few different accessors to use trace modules.

The traces still use the original register and getModule functions in src/plots/plots, however instead of being called from the trace module itself, register is now called from a register function exposed by Plotly so that end users will be able to pick and choose traces of their liking (once the trace modules are a bit more modularized out)

Because of the changes, some "side-effects" needed to be fixed - mainly with pathing in tests and a few other files.

[mdtusz]

In all the traces/*/index.js, we can now get rid of the Plotly dependency, and no longer call Plots.register - instead calling it from the register function in src/index.js.

[etpinard]

🍻

[etpinard]

Let's require in pie/style_one.js directly, so that once we split out Pie from the core, legend won't bundle all of Pie into its module.

[etpinard]

Nice!

Bringing @bpostlethwaite and @alexcjohnson for some thoughts on the API:

• I'm thinking that making Plotly.register take an option object might not be necessary.

// this might be sufficient
Plotly.register([require('plotly.js/bar'), require('plotly.js/box')]);

// or even where we would loop over the argument object
Plotly.register(require('plotly.js/bar'), require('plotly.js/box'));

Granted, passing an object with key traces makes the API easily extendible to registering components down the road. But, do users need to know that they are registering trace or component modules? Could we handle the trace / component split behind the scene inside the register function (e.g. by adding a type property to each trace module)?

[mdtusz]

The trace modules have _type in them right now, but it's more their name - I just followed the names of the arguments for the Plots.register function. I'd be happy to go either way with this though - the object notation allows for easily extending functionality in the future, but it is a bit more verbose. Up to you guys.

[alexcjohnson]

I'd go for the simplest form (loop over arguments). Another reason: you're not using the keys of this object - which is good, because we don't want users making their own names for the trace types, this should come from the module itself. But then why bother having an object with keys?

Re: components - either behind the scenes, or just make a different function like registerComponent. Doesn't make a big difference, I don't think.

[etpinard]

+1 for loop over arguments.

[cldougl]

👍 simplest

[bpostlethwaite]

What makes a component registerable? How can users do

Plotly.register(myComponent)

if we want users to be able to do that in the future an object notation may be necessary so we can allow certain requirements to be turned on/off during the registration process.

[etpinard]

@bpostlethwaite

it would be nice to make it clear what is registerable though.
What makes a component registerable?

Maybe we could store another meta property in the trace modules? e.g.

var PlotlyBar = require('plotly.js/lib/bar');

console.log(PlotlyBar.moduleType);  // => 'trace'

and down the road, registerable component modules could have moduleType set to 'component'.

Moreover, Plotly.register should error out if given a module without moduleType.

[etpinard]

I don't like using arguments unless it's the only way because it isn't as easily portable (you would have to use Array.spread)

Wouldn't arguments.length by enough?

@mdtusz that said, I'm ok with Plotly.register taken a single array as arguments. Your call.

Can we go with registerTraces as the function name,

I'd vote for a single Plotly.register and taking care of the distinction between trace and component modules behind the scenes.

[mdtusz]

👍

I'll add a moduleType field too then.

Do we want to get rid of the _'s though? I'm happy either way, so its up to you.

[etpinard]

Let's get rid of the _'s. No need to add complexity here.

[etpinard]

🐄 could you bump this line back up against var Gl2d = ...

[etpinard]

@mdtusz Question:

So, we're still building the list of registered trace modules in the Plots objects inside the plots/plots.js closure. I think you told me that this pattern would not work with Webpack (as Wepback can't share the Plots state between files). Can you comment on this?

[etpinard]

@mdtusz Awesome work. plotly.js is now only a few step away from modularity 🎉

TODO:

• come to a consensus on Trace Module Functionality #180 (comment) (cc @alexcjohnson @bpostlethwaite @cldougl )
• Write up some jasmine tests for Plotly.register

In a future PR:

• Move the the geo, gl3d and gl2d registerSubplot calls into Plotly.register (e.g. using a check for Plots.traceIs('gl3d'))
• Expose Plotly.register in the index.js

[mdtusz]

Regarding Webpack: I think I was wrong in my understanding of how it bundles 😐. After some basic testing, it seems that you can modify modules from anywhere and have the changes persist throughout the bundle. Because Webpack doesn't know how to deal with transforms in the browserify package.json field, we will need to do one of a few options:

1. Change the gl-* modules to use a built entry point that is javascript all the way down
2. Provide built bundles of the core and traces in a directory that is made in the release build step
3. Provide a "parallel" src directory that is pure javascript with the glsl files already transformed

They all have their plusses and minuses so I'd like to hear your thoughts on it. It will also depend on how we would like to expose the trace modules for users.

[etpinard]

After some basic testing, it seems that you can modify modules from anywhere and have the changes persist throughout the bundle.

fantastic 🍻

we will need to do one of a few options:

I think (1) is possible. Only with this option can we guarantee that no code will be duplicated require statement to another while bundling. Here's the list of all modules required in plotly.js to need a browserify transform:

We have access to all the gl modules where we could add a pure-js built file in each module. We could then add a webpack field in the package.json pointing to those pure-js files. The other two modules are scijs modules and I doubt that the maintainers would disagree with us trying to support webpack.

[etpinard]

💃 💃 💃