-
-
Notifications
You must be signed in to change notification settings - Fork 393
docs(importmaps): use importmaps for external script #3200
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
base: master
Are you sure you want to change the base?
Conversation
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.
There are other places that reference https://unpkg.com/vtk.js
, you might want to modify those as well...
@floryst should we mention both options ? with and without import maps ?
a607867
to
e7023df
Compare
e7023df
to
e3b5626
Compare
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.
LGTM
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.
Overall looks good. I would like to request that we add the ES module code in addition to the UMD bundle. We can have two headings, such as "Using ES Modules" and "Using the UMD bundle", in that order.
@floryst when you say two headings, you mean one for |
Yes. I see you replaced the UMD code with the importmaps code. While I agree with moving forward with importmaps (esp given its support across major browsers), I would like to keep some of the UMD docs around for a bit longer. |
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.
In revisiting this, I think we can go ahead and drop the UMD instructions. Modern baseline browsers support importmaps, and if we really need to support much older browsers, something else will likely break.
Context
Recommend the use of importmaps
Results
Changes
PR and Code Checklist
npm run reformat
to have correctly formatted codeTesting