Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Modularise the library #41

Closed
eoinmurray opened this issue Nov 27, 2015 · 17 comments
Closed

Modularise the library #41

eoinmurray opened this issue Nov 27, 2015 · 17 comments
Assignees
Labels
feature something new

Comments

@eoinmurray
Copy link

Plotly is a really great library, I think its the best js library out there for scientific graphs in javascript.

I wonder how difficult it is to modularise Plotly - as the current minified cdn version is 1.1MB.

This is quite heavy for someone who, for example, just wants to implement scatter plots.

I think the library would gain a lot of adoption if you could pick and choose which features to include into someonese project.

@ryanlin1986
Copy link

+1.
1MB is too big for mobile app, I think we should have a basic version contains scatter, bar, pie and have rest feature in advanced version, or something like plugin.

@etpinard
Copy link
Contributor

@eoinmurray thanks for writing in.

We hope to achieve some sort of modularizibilty in the near future.

I would be pretty easy to add a plotly-basic.js bundle to the dist folder. For example, making a scatter-only bundle sums up to applying this patch and running npm run build.

The main requirement for adding plotly-basic.js would be to solidify+generalize our image test framework so that multiple bundle types could be tested in the same run.

Down the road, I'm thinking (cc @alexcjohnson) of allowing users to make their own bundles using some sort of plugin signature e.g:

var Plotly = require('plotly.js/lib/base');
require('plotly.js/lib/cartesian')(Plotly);
require('plotly.js/lib/scatter')(Plotly);

// and then maybe also modularizing non-trace components:
require('plotly.js/lib/errorbars')(Plotly);
require('plotly.js/lib/colorbar')(Plotly); 
require('plotly.js/lib/legend')(Plotly)
// ...

and a browserify step. That way users could include only the components and traces types they need.

Achieving this would take some work cleaning up circular dependencies inside the src tree, but is definitively doable. Most things in the plotly.js src are already modules of their own. We just need to make a little more orthogonal to one another.

@mbonaci
Copy link
Contributor

mbonaci commented Nov 27, 2015

+:100: for modularization/plugins (e.g. a download page that lets you pick and choose), we are considering using plot.ly and the biggest blocker is the size of the minified version.
As the first step maybe just do a split of web.gl from the "stuff" that don't require it (haven't looked at the code to see whether that's easily achievable).
Regardless of all that, great library and stunning examples.

@otisg
Copy link

otisg commented Nov 30, 2015

+1 for modularization so the large download can be avoided. @etpinard & @alexcjohnson - do you have a rough idea when you will be able to tackle this? Are you thinking days, weeks, or months? TIA

@Hypercubed
Copy link
Contributor

It would be very nice to have d3 and the webgl stuff separate and optional. Having webgl stuff optional could help #22 .

BTW, here is a map of the entire plotly.js dist file: http://bl.ocks.org/Hypercubed/e13c3ba8eb86ce0fc3ae

@etpinard etpinard added the feature something new label Dec 4, 2015
@H1D
Copy link

H1D commented Dec 8, 2015

apart from size issue there is and issue when you already have d3 loaded on page with some plugins. Loading plotly will rewrite d3 with plugins =(

any big single-page app will face this issue while trying to use plotly

@mdtusz mdtusz closed this as completed Dec 8, 2015
@mdtusz mdtusz reopened this Dec 8, 2015
@etpinard
Copy link
Contributor

etpinard commented Dec 8, 2015

@H1D

You could use browserify's exclude option to remove d3 from the plotly.js bundle:

browserify -x d3 src/index.js > plotly-without-d3.js

@H1D
Copy link

H1D commented Dec 8, 2015

@etpinard thx.. I'll consider switching to browserify =)

@etpinard
Copy link
Contributor

For the first iteration:

1) Split library into a core lib module and trace-type lib modules

Each trace module would register the appropriate plot-type module (e.g. the scatter3d module will register the gl3d plot type).

The core module would include all the cartesian plot code, all the components and the polar code for now. As it stands right now, the core module would be ~275 kb minified excluding d3.

TODO:

var Plotly = require('plotly.js/lib/core');
var PlotlyScatter3D = require('plotly.js/lib/scatter3d');  // i.e. lib / <trace type>

Plotly.register(PlotlyGl3d);  // or a better api?

Plotly.plot('graph', [ { type: 'scatter3d', x: [], y: [], z: [] } ]);

2) Register d3 instead of requiring it into the plotly source files

So that the users that already have d3 in their scopes could:

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

Plotly.d3 = window.d3

// or something more sugary e.g. 
Plotly.register({d3: window.d3})

TODO:

  • we would need to replace all var d3 = require('d3') from the src files for var d3 = Plotly.d3.

N.B. The dist bundles would still include d3 and we should warn users that use a different version of d3 than the one that ships with the plotly.js dist bundles.

@alexcjohnson @mdtusz @bpostlethwaite

@alexcjohnson
Copy link
Collaborator

re: (1), library modules
Thinking about this from a user's perspective, people will know what kind of traces they want to draw, and what kind of components (shapes, annotations, colorbars, legends...) they want to add. Just based on that, we can determine what plot types (gl3d, gl2d, geo) the bundle needs to include. We should make the source follow that logic as well. That means that:

  • we group the resources needed for each plot type into an easily requireable module, as @etpinard describes.
  • each trace module requires the plot type it, er, requires.
  • components get registered too (in a separate registry from traces, we wouldn't want to try and draw a 'shapes' trace!) and when a plot or a trace on a plot wants to use a component, it has to look up the component in the registry, and only continue if it finds said component.
  • then the bundle index wouldn't directly include anything in the plots section (except the first file - that's where this register stuff is! that still needs a bit of refactoring I suppose...), and the process of making a custom build would be just to omit whichever traces and components you don't want.
  • That said, most of the size optimization is accomplished by filtering out any plot types you don't need - at a first cut we can make this easier by grouping traces and components by plot type within the bundle index, then down the line we could have a script to automatically prepare filtered index files.

re: (2), d3
👍

@etpinard
Copy link
Contributor

@alexcjohnson

each trace module requires the plot type it, er, requires.

I'd prefer exposing the explicit:

var Plotly = require('plotly.js/lib/core');
var PlotlyScatter3D = require('plotly.js/lib/scatter3d');

Plotly.register({traces: [PlotlyScatter3D]});

over the implicit:

var Plotly = require('plotly.js/lib/core');
require('plotly.js/lib/scatter3d');  // where Plotly.Plots.register('scatter3d') would get called

@alexcjohnson
Copy link
Collaborator

I'd prefer exposing the explicit over the implicit

Ah I see, so the other args to the existing Plotly.Plots.register get exported as module attributes and picked up by the new Plotly.register - sounds great. This requires a 1:1 correspondence between trace modules and trace types, but that has a whole bunch of benefits of its own 🌟

The call could even be Plotly.register(require('plotly.js/traces/scatter3d')); etc. for each trace type... but I don't feel strongly about that. Though, one possible benefit of that would be to allow including modules post-build... so the build includes some modules, but then in your app after loading plotly.js you can bring in another module as a separate js file and register it.

@Hypercubed
Copy link
Contributor

@etpinard Just want to say I think this is a great plan.

@etpinard etpinard added this to the Modular plotly.js milestone Dec 15, 2015
@etpinard
Copy link
Contributor

@etpinard
Copy link
Contributor

Hi all,

PR #202 will bring modularity to plotly.js version 1.5.0. Feel free to comment on the PR.

Note that d3 will still be part of the core bundle following #202. We haven't quite settled on the best way to remove it from the plotly.js core. Please see https://gist.github.com/etpinard/4e72f1a430ba01f9cbd7 for an overview.

@chriddyp
Copy link
Member

👏

@mdtusz
Copy link
Contributor

mdtusz commented Jan 25, 2016

🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature something new
Projects
None yet
Development

No branches or pull requests

10 participants