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

Dist versions do not work with Browserify #2902

Closed
m90 opened this issue Aug 14, 2018 · 6 comments
Closed

Dist versions do not work with Browserify #2902

m90 opened this issue Aug 14, 2018 · 6 comments
Labels
bug something broken

Comments

@m90
Copy link
Contributor

m90 commented Aug 14, 2018

This is picking up: plotly/react-plotly.js#93

I'm running into issues using the dist versions included in plotly.js and plotly.js-dist in a Browserify bundle.

I am basically just doing what the README tells me:

var Plotly = require('plotly.js-dist');

console.log(Plotly);

When I try to Browserify this script it tells me:

Error: Cannot find module '../src/transforms/aggregate' from '/home/frederik/projects/plotly-browserify-issue/node_modules/plotly.js-dist'

When I try to load the dist bundle from the plotly.js package as seen for example in react-plotly.js I will see the following error:

Error: Cannot find module './core' from '/home/frederik/projects/plotly-browserify-issue/node_modules/plotly.js/dist'

I can however bundle plotly if I use require('plotly.js') as the main entry point.

I created an example repository for reproducing the issue over here: https://github.com/m90/plotly-browserify-issue

@m90
Copy link
Contributor Author

m90 commented Aug 14, 2018

It looks like this is currently expected and would need browserify/detective#79 to land in order to work.

Other related issues:

which probably means that dist is not what Browserify users (or libraries that depend on plotly) should be using.

@etpinard
Copy link
Contributor

Thanks very much for the detailed report. I was unaware that browserify couldn't bundle bundles it already bundled.

It looks like browserify/detective#79 is planned for browserify v17, so that's probably the best solution going forward.

At present though, if someone could give derequire a shot, we will gladly merge that PR, but no plotly.js team member will dedicate time on this issue. That's mainly because bundling the "main" plotly.js package with browserify should just work - that what we use internally afterall. In other words, for browserify users, plotly.js-dist only helps them save a few bytes of downloaded npm packages.

@etpinard etpinard added the bug something broken label Aug 14, 2018
@m90
Copy link
Contributor Author

m90 commented Aug 15, 2018

That's mainly because bundling the "main" plotly.js package with browserify should just work - that what we use internally afterall.

While this is 100% correct it still creates the issue of libraries that depend upon plotly referring to the dist versions, thus making it impossible (or require workarounds) to use them with Browserify, see: plotly/react-plotly.js#93 so in this case it'd not only be about the bytes.

@etpinard
Copy link
Contributor

see: plotly/react-plotly.js#93 so in this case it'd not only be about the bytes.

Oh right. I was unaware that react-plotly.js was already using plotly.-js-dist. Thanks for pointing this out!

@alexcjohnson
Copy link
Collaborator

Can we close this now that #2905 is merged?

@etpinard
Copy link
Contributor

Yes. The fix in #2905 is compatible with browserify@17 which is upcoming.

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

No branches or pull requests

3 participants