-
-
Notifications
You must be signed in to change notification settings - Fork 78
Use plotly.js browser script vs. npm module #32
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
149eef1
to
b2be961
Compare
loading = true; | ||
const script = document.createElement('script'); | ||
script.type = 'text/javascript'; | ||
script.text = require('raw-loader!../vendor/plotly-1.28.3.min.js'); |
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.
@blink1073 Ideally, we could just set script.src
to the relative path of the plotly.js file, but unfortunately that won't work once it's all bundled with webpack. Using raw-loader, we can get the content of the plotly.js file and set script.text
, but this seems a little hacky... 🤔
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.
You don't even need to vendor the file if you use the raw-loader
, you can use raw-loader!plotly/dist/plotly.min.js
. I think that this approach is fine.
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.
Do you mean raw-load it from a CDN?
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.
No, as a third party import that webpack will resolve.
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.
But we don't load the script until it is actually needed.
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.
Can you show me what you mean?
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.
script.text = require('!raw-loader!plotly/dist/plotly.min.js')
, where plotly/dist/plotly.min.js
is the location of the file in node_modules
.
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, I like that better 👍
Closes #21
Resolves an issue with bundling plotly.js using webpack: https://github.com/plotly/plotly.js#building-plotlyjs-with-webpack