-
-
Couldn't load subscription status.
- Fork 2.7k
Remove references to plotly.js CDN and requirejs #4762
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
Latest plotly.js used to be on a CDN, but it isn't anymore.
| If 'cdn', a script tag that references the plotly.js CDN is included | ||
| in the output. The url used is versioned to match the bundled plotly.js. | ||
| HTML files generated with this option are about 3MB smaller than those |
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 think we should keep cdn option as it could help reduce smaller HTML files.
But for loading that we shouldn't use requirejs.
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.
Oh ok - but we don't have a cdn currently right?
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.
Yes we publish every version to CDN.
It's only the -latest version is no longer updated (plotly/plotly.js#5697) since plotly.js v2 and stayed as latest v1 version i.e. 1.58.5.
So we should be able to continue loading from CDN by specifying the exact version.
To achieve that you could add a separate script tag instead of requirejs to load e.g.
<script src="https://cdn.plot.ly/plotly-2.35.1.min.js" charset="utf-8"></script>Or alternatively for async loading
<script type="module">
import "https://cdn.plot.ly/plotly-2.35.2.min.js"
Plotly.newPlot("gd", [{ y: [1, 2, 3] }])
</script>
This removes references in the base renderers to the CDN and requirejs.
window._Plotlybecause that appears to be unusedThis should fix #4336.