-
-
Notifications
You must be signed in to change notification settings - Fork 139
Do not import bundled dist version #95
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
Conversation
Thanks for looking so hard for solutions on this! Unfortunately we can't do this for Browserify because it makes Webpack much harder to use...
Does #94 not work with Browserify either? I'd be more inclined to merge that one...
#94 does not work properly with Browserify either (in my setup). I think the only long-term solution is using derequire
as described in plotly/plotly.js#2902 or sitting it out and wait for Browserify 17, hoping they will fix it for us. Alternatively it'd be about properly documenting the limitation and its workaround.
That being said I will hopefully find the time to look into using derequire
and if it helps tomorrow, so I'll let you know what happens here.
OK. Are you not able to use the factory approach and provide your own plotly object?
(thanks again for persisting with this issue, I appreciate the assistance, having not worked very much with Browserify)
I can use the factory approach just fine, but as stated it's not really what's documented right now, meaning it takes people quite some time to find out about it.
Well, it's documented here, but I supposed I should make it clearer: https://github.com/plotly/react-plotly.js#customizing-the-plotlyjs-bundle
It should mention that if using Browserify it's what needs to be done in any case, not only if you are looking into shaving bytes.
That being said, I'd wait to see if applying derequire
on the bundling of plotly itself helps before moving around too much stuff and introducing a Webpack / Browserify binary earlier than it needs to be introduced. Then again that's up to you.
I think we can close this now that plotly/plotly.js#2905 has been merged. I wonder what the best way of creating a "pre-version" of react-plotly.js
is now, so that we can confirm it's behaving as expected before 1.40.0
is published to npm?
I don't know if we'll be able to test this before 1.40.0 lands, but thanks for your contribution upstream of this project! Hopefully things will go as planned but if they don't, we can always issue a patch release of plotly.js or fix stuff on this side :)
So my curiosity just made me look into if the upstream change actually fixes the issue, and I was able to use npm link to make react-plotly
use current upstream master. Bundling with Browserify now works as expected and there is no more need for the workaround 🎉
I do not know how the -dist
package is being created and published, but in case the plotly.js
it ships with is the one that is in the upstream /dist
folder it should work just as fine once it's at 1.40.0
.
Fixes issues with browserify seen in #93 and plotly/plotly.js#2902