search

plotly / react-plotly.js

A plotly.js React component from Plotly 📈
MIT License
1.01k stars 136 forks source link

import plotly.js-dist v2 instead of v1 plotly.js/dist #260

Open archmoj opened 3 years ago

archmoj commented 3 years ago

Supersedes #205. Closes #248 and fixes #258. Addresses https://github.com/plotly/plotly.js/issues/5925#issuecomment-913687380.

Please note that this appears to be a breaking change for this repository.

@plotly/plotly_js

dmbarry86 commented 3 years ago

I have tested this with our own components and can confirm that this now allows the use of plotly.js-dist alongside this library. https://github.com/plotly/plotly.js/issues/5925#issuecomment-914549350

If this was to be merged then I think the installation section of the readme would also need updating.

This would also be considered a breaking change for anybody currently using this library with plotly.js.

nicolaskruchten commented 3 years ago

It should already be possible to use a dist bundle with this library without changing package.json ... Check out this section of the readme: https://github.com/plotly/react-plotly.js#customizing-the-plotlyjs-bundle

dmbarry86 commented 3 years ago

@nicolaskruchten I think there's a broken link there. When clicking on "assemble you own customized bundle" it just takes to main plotly.js readme.

I think the point here is that this library is now no longer aligned with the recommendations from plotly.js which itself now promotes the use of plotly.js-dist rather than plotly.js. Sure, we could all go ahead and write some custom code to use a different bundle but then how many different people need to do that? Makes sense to me to align the default setup of this library with the default (as of v2) recommendations from plotly.js.

nicolaskruchten commented 3 years ago

Yes, we should, I agree, but given that we're feeling pretty short-handed at the moment, I'm providing you with options that you can use today :)

nicolaskruchten commented 3 years ago

Here's where that link should point: https://github.com/plotly/plotly.js/blob/master/CUSTOM_BUNDLE.md

jacksongoode commented 1 year ago

This would reduce the bundle size no? @archmoj

saimidu commented 2 months ago

Hi @nicolaskruchten @dmbarry86, were there any further steps to resolve this dependency issue? I see that this PR has been open for a number of years without any resolution.

jacksongoode commented 2 months ago

@saimidu I don't think either of them still work at Plotly.