-
-
Notifications
You must be signed in to change notification settings - Fork 6.2k
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
docs(wasm): add sections about Node.js and module access #13978
Conversation
This also renames the example import from `imports` to `someModule`. It's easy to think `imports` is a magic key. Closes vitejs#13965.
Run & review this pull request in StackBlitz Codeflow. |
docs/guide/features.md
Outdated
|
||
```js | ||
init({ | ||
imports: { | ||
someModule: { |
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.
Maybe it is better to align with MDN here, they are using imports
so I would suggest we stick with the current naming: https://developer.mozilla.org/en-US/docs/WebAssembly/JavaScript_interface/instantiate#first_overload_example
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'll note that the spec uses "js" as the sample module name: https://webassembly.github.io/spec/js-api/#sample
(MDN also tripped me up in trying to understand the parameter, so I'd love to not just do what they do. :)
That case having been made, if you want "imports", sure.
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 let @sapphi-red weight on this one. Thanks for the changes!
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.
SG. Thanks for the review and providing Vite!
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'll select imports
as I assume more people would read MDN than the spec.
docs/guide/features.md
Outdated
|
||
```js | ||
init({ | ||
imports: { | ||
someModule: { |
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'll select imports
as I assume more people would read MDN than the spec.
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.
Thanks! I have one nitpick.
Co-authored-by: 翠 / green <[email protected]>
Description
This also renames the example import from
imports
tosomeModule
. It's easy to thinkimports
is a magic key.Closes #13965.
What is the purpose of this pull request?
Before submitting the PR, please make sure you do the following
fixes #123
).