Uh oh!
There was an error while loading.Please reload this page.
- Notifications
You must be signed in to change notification settings - Fork2.8k
Use modernnative ES6 import to load plotly.js bundle instead ofrequirejs which is no longer under active development#4763
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 ourterms of service andprivacy statement. We’ll occasionally send you account related emails.
Already on GitHub?Sign in to your account
Uh oh!
There was an error while loading.Please reload this page.
Conversation
archmoj commentedSep 11, 2024
Great PR. |
marthacryan commentedSep 12, 2024
Note: the orca tests are also failing on master |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
alexcjohnson left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
💃 I'm on board - not aware of any good use cases for requirejs here. Maybe there were in the past, but mostly those use cases would have applied to direct plotly.js use rather than plotly.py anyway.
archmoj commentedSep 17, 2024
Please update the description of the PR and include links & info about |
Uh oh!
There was an error while loading.Please reload this page.
Uh oh!
There was an error while loading.Please reload this page.
native ES6 import to load plotly.js bundle instead ofrequirejs which is no longer under active developmentCo-authored-by: Mojtaba Samimi <33888540+archmoj@users.noreply.github.com>
archmoj left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others.Learn more.
Great PR 🏆
Nicely completed.
💃
archmoj commentedSep 23, 2024
@LiamConnors do you have permission to merge this? |
LiamConnors commentedOct 2, 2024
@marthacryan and@archmoj, when building Plotly.py using this PR and the master branch of Plotly.js, figures don't load in a classic notebook and the following error is in the console.  |
gvwilson commentedOct 17, 2024
@archmoj@marthacryan please sync with@LiamConnors about the issue he flagged (previous comment in this thread) |
LiamConnors commentedOct 21, 2024
Regarding the error saw in my previous comment. It only is an issue with notebook<7 |
archmoj commentedOct 21, 2024
Is there any blocker for merging this PR? |
marthacryan commentedOct 21, 2024
@archmoj@LiamConnors@gvwilson I believe there isn't anything blocking merging since Liam was able to use this with Notebook 7. |
archmoj commentedOct 21, 2024
💃 |
gvwilson commentedOct 21, 2024
merge it! |
Uh oh!
There was an error while loading.Please reload this page.
Note: this PR breaks support for notebook < 7.#4805 is already working on removing that code and adding a warning.
This removes references in the base renderers to requirejs. Requirejs is essentially unmaintained and newer versions of jupyter lab and jupyter notebook don't support it anymore, so this fixes some of the notebook 7 and lab 4 issues we've been seeing. Replaces requirejs support with native javascript modules:https://developer.mozilla.org/en-US/docs/Web/JavaScript/Guide/Modules
Alternative to#4762 that doesn't remove the CDN.
Fixes#4336.