Repository navigation
[js-api] Allow a missing importObject for builtin imports - #2264
Open
chicoxyzzy wants to merge 2 commits into
Open
chicoxyzzy wants to merge 2 commits into
chicoxyzzy wants to merge 2 commits into
Conversation
Fixes WebAssembly#2102. read the imports threw TypeError whenever module_imports was non-empty and importObject was missing. A module whose imports are all satisfied by an enabled builtin, or by the imported string constants module, does not read importObject. Throw only if importObject is missing and some import is not supplied by those sources. Do that before allocating builtin instances.
Ms2ger
reviewed
Oct 5, 2026
Comment on lines
509
to
514
| 1. If |builtinOrStringImports|[|moduleName|] [=map/exists=], | ||
| 1. Let |o| be |builtinOrStringImports|[|moduleName|]. | ||
| 1. If |o| [=is not an Object=] or if [=?=] [$HasProperty$](|o|, |componentName|) is false, | ||
| 1. Set |o| to [=?=] [$Get$](|importObject|, |moduleName|). | ||
| 1. Else, | ||
| 1. Let |o| be [=?=] [$Get$](|importObject|, |moduleName|). |
Collaborator
There was a problem hiding this comment.
A quick read does not convince me that we'll never call Get with a missing argument. This needs at least a note to explain the invariant that prevents that.
Member
Author
There was a problem hiding this comment.
If importObject is missing, the opening check throws unless every import is an enabled builtin or an imported string constant. Those are own properties of builtinOrStringImports, so these Gets are not reached. Noted that after the algorithm, and asserted it before each call.
Get requires an Object. importObject can be missing when every import is an enabled builtin or an imported string constant, but those names are own properties of builtinOrStringImports, so the fallback Gets are not reached. Assert that before each fallback Get, and note it with the other notes on this algorithm.
chicoxyzzy
force-pushed
the
js-api-optional-import-object
branch
from
October 5, 2026 21:31
7e669ba to
598c350
Compare
This branch has not been deployed
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Fixes #2102.
read the importsthrew aTypeErrorwhenevermodule_importswas non-empty andimportObjectwas missing. That includes a module whose imports are allwasm:js-stringbuiltins, or all imported string constants. Those imports are supplied by the host. Chrome instantiates them with no import object.The check now runs only when
importObjectis missing, and it throws only if some import is not supplied by an enabled builtin or by the imported string constants module. It runs before builtin instances are allocated.A user import, an unknown name under
wasm:js-string, and a builtin mixed with a user import still throwTypeError. OmittingimportObjectand passingundefinedare the same.