Do not import bundled dist version

[m90]

Fixes issues with browserify seen in #93 and plotly/plotly.js#2902

[nicolaskruchten]

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...

[m90]

#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.

[nicolaskruchten]

OK. Are you not able to use the factory approach and provide your own plotly object?

[nicolaskruchten]

(thanks again for persisting with this issue, I appreciate the assistance, having not worked very much with Browserify)

[m90]

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.

[nicolaskruchten]

Well, it's documented here, but I supposed I should make it clearer: https://github.com/plotly/react-plotly.js#customizing-the-plotlyjs-bundle

[m90]

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.

[m90]

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?

[nicolaskruchten]

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 :)

[m90]

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 🎉

[nicolaskruchten]

Great, thanks again! But can we still switch to using the -dist bundle as well? I do want to move towards this as that bundle is so much slimmer due to the lack of dependencies.

…

On Thu, Aug 16, 2018 at 10:38 Frederik Ring ***@***.***> wrote: So my curiosity just made me look into if the upstream change actually fixes the issue, and I was able to use npm link <https://docs.npmjs.com/cli/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 🎉 — You are receiving this because you commented. Reply to this email directly, view it on GitHub <#95 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AAMbA6WULc_1wZtJnkcPYQPyyV0-e3Mcks5uRYPwgaJpZM4V8D7w> .

[m90]

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.