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
Expose method to allow synchronous initialization #2924
Conversation
This seems reasonable enough to me, thanks for the PR! Can you add some documentation for this as well? |
Yep, will add some documentation. |
@alexcrichton Do we want to expose an |
Honestly this is so far outside my expertise I don't really have an opinion. It's not "obviously wrong" but I know very little about all the myriad embeddings that JS can be run in, so I have no idea how to gauge which is better to provide. I'm basically leaving such a judgement call to you. |
ec3f22f
to
f72b9c0
Compare
@@ -469,7 +469,8 @@ impl<'a> Context<'a> { | |||
OutputMode::Web => { | |||
self.imports_post.push_str("let wasm;\n"); | |||
init = self.gen_init(needs_manual_start, Some(&mut imports))?; | |||
footer.push_str("export default init;\n"); | |||
footer.push_str("export { initSync }\n"); | |||
footer.push_str("export default init;"); |
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 have noticed there was two newlines at the end of the generated JS so I removed one newline.
f72b9c0
to
5c9a270
Compare
Alright, I have added an example and a guide as well for this. I have also decided not expose the internal helpers and instead expose a new |
imports_init.push_str("imports."); | ||
imports_init.push_str(module_name); | ||
imports_init.push_str(" = {};\n"); |
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 have noticed that if a module has no imports and we don't initialize the imports
with the module_name
(wbg
) that it will break when it tries to initialize the memory, because it gets assigned to imports.wbg.memory
.
{post_instantiate} | ||
{start} | ||
return wasm; | ||
}} | ||
|
||
function initSync(bytes{init_memory_arg}) {{ |
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.
ℹ️ This function gets exported to synchronously initialize the module.
@alexcrichton This PR is ready for review. |
Thanks! |
@alexcrichton Nice! When do you think an we release this as a new version? So that the wasm-bindgen book is also updated with the example etc. |
I'll look into doing a release next week |
Ok I'm doing a publish at #2935 |
* Expose helpers to allow synchronous initialization * fixup: fix tests
Thanks a lot! |
This PR exposes helpers that will allow to synchronously initialize the WASM module. The helpers are only exported when compiling for
web
. But we could also export them forno-modules
.What I have done is to extract some of the functionality from
init()
into helper functions which are now exported.This fixes #2698.
Todos
initSync()
?Background
At StackBlitz we are building WebContainer, a Node.js runtime for the browser, and we could benefit from this addition a lot because it would allow us to lazy load WASM modules on demand. Some features are split into separate WASM modules to improve load performance and those features are often not frequently used. By exposing these helpers we could synchronously instantiate modules in a worker like this then:
ℹ️ The above only really works in a Web Worker but that's ok IMO as the main thread disallows sync APIs for good reasons.
What do you think @alexcrichton? Is that what you were thinking of in #2698?