Cache document.currentScript for non-ESM output #2520
Conversation
Remove unecessary function call, in favor of direct variable lookup.
ghost
changed the title
|
Thanks! Could you leave a comment in the source for why it's done this way? |
This comment has been minimized.
This comment has been minimized.
|
This comment looks fine, thanks for that! I think this needs a rebase now after a previous merge, want to do that and I can merge? |
|
Sure, I'll rebase and deal with the conflicts soon. |
| }; | ||
|
|
||
| let ts = self.ts_for_init_fn( | ||
| has_memory, | ||
| !self.config.omit_default_module_path && !default_module_path.is_empty(), | ||
| !self.config.omit_default_module_path && !default_module_path_creator.is_empty(), |
There was a problem hiding this comment.
If I'm following this logic correctly, the entire expression could be shortened to the following:
!default_module_path_creator.is_empty()cc @diceride
If someone omits the default path, and chooses, say, Node.js, there still won't be a default path, therefore we can just check if there isn't a default path via .is_empty().
|
@alexcrichton It seems like a bunch of runtime tests are now failing after rebasing from the recent merges. Could you take a look into it and help me debug this? |
|
I think you need to run the tests locally with |
|
(or the test expectations of what the output is just need updating) |
ghost
closed this
Fixes issue #2502.
Currently, when using non-ESM module output (i.e. scripts output), the generated JavaScript accesses the
Document#currentScriptgetter at runtime, which will always have a different (incorrect) value by the time that the user runs their code to load their Wasm.With this change, the default source for ESM and web script output is cached in an immutable variable, and accessed when the user doesn't pass an argument.
Other output targets should remain untouched.
As an added side-effect, the default source is always constructed, this should add a small amount of extra overhead to loading the glue JS, but if the user loads the Wasm multiple times (for some reason?), it is only constructed once, which should improve performance.