-
Notifications
You must be signed in to change notification settings - Fork 29.8k
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
wasm: add instantiateStreaming support #22332
Conversation
This commit adds InstantiateStreaming support to allow for some compatability with Web browser environments. Refs: nodejs#21130
@danbev sadly an error occured when I tried to trigger a build :( |
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.
we should support passing a readable stream.
@nodejs/v8 would it be possible to get v8::WasmStreaming before 7.0? i'd like to implement something like devsnek@ab9c03a |
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.
Unfortunately, I don't think this PR quite changes the status quo with regards to the stated goal:
My motivation for wanting this is to allow a developer to write code that could be used in both web browser environments and in node.
One could already use other mechanisms to compile WASM modules on Node.js, and even if this PR were to be merged one would still have to use different code on Node.js vs. browsers to compile a module.
In fact, since this is incompatible with browser's instantiateStreaming
implementation (which does not support Promise<TypedArray>
as input), I don't believe this PR should land in Node.js.
Ah I see, I was mistakingly thinking that Sorry about the noice and thanks @TimothyGu and @devsnek for your feedback! |
This commit adds instantiateStreaming support to allow for some
compatability with Web browser environments.
Refs: #21130
Whether or not this feature should be added as been discussed in above referenced issue. My motivation for wanting this is to allow a developer to write code that could be used in both web browser environments and in node.
Checklist
make -j4 test
(UNIX), orvcbuild test
(Windows) passes