Skip to content
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

Support emitting direct imports in wasm files #1689

Merged
merged 1 commit into from
Aug 1, 2019

Conversation

alexcrichton
Copy link
Contributor

Support was previously (re-)added in #1654 for importing direct JS
values into a WebAssembly module by completely skipping JS shim
generation. This commit takes that PR one step further by also
embedding a direct import in the wasm file, where supported. The wasm
file currently largely just imports from the JS shim file that we
generate, but this allows it to directly improt from ES modules where
supported and where possible. Note that like #1654 this only happens
when the function signature doesn't actually require any conversions to
happen in JS (such as handling closures).

For imports from ES modules, local snippets, or inline JS they'll all
have their import directives directly embedded into the final
WebAssembly binary without any shims necessary to hook it all up. For
imports from the global namespace or possibly vendor-prefixed items
these still unconditionally require an import shim to be generated
because there's no way to describe that import in an ES-friendly way
(yet).

There's a few consequences of this commit which are also worth noting:

  • The logic in wasm-bindgen where it gracefully handles (to some
    degree) not-defined items now only is guaranteed to be applied to the
    global namespace. If you import from a module, it'll be an
    instantiation time error rather than today's runtime error when the
    import is called.

  • Handling imports in the wasm module not registered with
    #[wasm_bindgen] has become more strict. Previously these imports
    were basically ignored, leaving them up for interpretation depending
    on the output format. The changes for each output target are:

    • bundler - not much has changed here. Previously these ignored
      imports would have been treated as ES module imports, and after this
      commit there might just be some more of these imports for bundlers
      to resolve.

    • web - previously the ignored imports would likely cause
      instantiation failures because the import object never actually
      included a binding for other imports. After this commit though the
      JS glue which instantiates the module now interprets all
      unrecognized wasm module imports as ES module imports, emitting an
      import directive. This matches what we want for the direct import
      functionality, and is also largely what we want for modules in
      general.

    • nodejs - previously ignored imports were handled in the
      translation shim for Node to generate require statements, so they
      were actually "correctly handled" sort of with module imports. The
      handling of this hasn't changed, and reflects what we want for
      direct imports of values where loading a wasm module in Node ends up
      translating the module field of each import to a require.

    • no-modules - this is very similar to the web target where
      previously this didn't really work one way or the other because we'd
      never fill in more fields of the import object when instantiating
      the module. After this PR though this is a hard-error to have
      unrecognized imports from #[wasm_bindgen] with the no-modules
      output type, because we don't know how to handle the imports.

    Note that this touches on Option to specify extra javascript imports required when instantiating WASM Module #1584 and will likely break the current use
    case being mentioned there. I think though that this tightening up of
    how we handle imports is what we'll want in the long run where
    everything is interpreted as modules, and we'll need to figure out
    best how wasi fits into this.

This commit is unlikely to have any real major immediate effects. The
goal here is to continue to inch us towards a world where there's less
and less JS glue necessary and wasm-bindgen is just a polyfill for web
standards that otherwise all already exist.

Also note that there's no explicitly added tests for this since this is
largely just a refactoring of an internal implementation detail of
wasm-bindgen, but the main wasm test suite has many instances of
this path being taken, for example having imports like:

(import "tests/wasm/duplicates_a.js" "foo" (func $__wbg_foo_969c253238f136f0 (type 1)))
(import "tests/wasm/duplicates_b.js" "foo" (func $__wbg_foo_027958cb2e320a94 (type 0)))
(import "./snippets/wasm-bindgen-3dff2bc911f0a20c/inline0.js" "trivial" (func $__wbg_trivial_75e27c84882af23b (type 1)))
(import "./snippets/wasm-bindgen-3dff2bc911f0a20c/inline0.js" "incoming_bool" (func $__wbg_incomingbool_0f2d9f55f73a256f (type 0)))

Support was previously (re-)added in rustwasm#1654 for importing direct JS
values into a WebAssembly module by completely skipping JS shim
generation. This commit takes that PR one step further by *also*
embedding a direct import in the wasm file, where supported. The wasm
file currently largely just imports from the JS shim file that we
generate, but this allows it to directly improt from ES modules where
supported and where possible. Note that like rustwasm#1654 this only happens
when the function signature doesn't actually require any conversions to
happen in JS (such as handling closures).

For imports from ES modules, local snippets, or inline JS they'll all
have their import directives directly embedded into the final
WebAssembly binary without any shims necessary to hook it all up. For
imports from the global namespace or possibly vendor-prefixed items
these still unconditionally require an import shim to be generated
because there's no way to describe that import in an ES-friendly way
(yet).

There's a few consequences of this commit which are also worth noting:

* The logic in `wasm-bindgen` where it gracefully handles (to some
  degree) not-defined items now only is guaranteed to be applied to the
  global namespace. If you import from a module, it'll be an
  instantiation time error rather than today's runtime error when the
  import is called.

* Handling imports in the wasm module not registered with
  `#[wasm_bindgen]` has become more strict. Previously these imports
  were basically ignored, leaving them up for interpretation depending
  on the output format. The changes for each output target are:

  * `bundler` - not much has changed here. Previously these ignored
    imports would have been treated as ES module imports, and after this
    commit there might just be some more of these imports for bundlers
    to resolve.

  * `web` - previously the ignored imports would likely cause
    instantiation failures because the import object never actually
    included a binding for other imports. After this commit though the
    JS glue which instantiates the module now interprets all
    unrecognized wasm module imports as ES module imports, emitting an
    `import` directive. This matches what we want for the direct import
    functionality, and is also largely what we want for modules in
    general.

  * `nodejs` - previously ignored imports were handled in the
    translation shim for Node to generate `require` statements, so they
    were actually "correctly handled" sort of with module imports. The
    handling of this hasn't changed, and reflects what we want for
    direct imports of values where loading a wasm module in Node ends up
    translating the module field of each import to a `require`.

  * `no-modules` - this is very similar to the `web` target where
    previously this didn't really work one way or the other because we'd
    never fill in more fields of the import object when instantiating
    the module. After this PR though this is a hard-error to have
    unrecognized imports from `#[wasm_bindgen]` with the `no-modules`
    output type, because we don't know how to handle the imports.

  Note that this touches on rustwasm#1584 and will likely break the current use
  case being mentioned there. I think though that this tightening up of
  how we handle imports is what we'll want in the long run where
  everything is interpreted as modules, and we'll need to figure out
  best how wasi fits into this.

This commit is unlikely to have any real major immediate effects. The
goal here is to continue to inch us towards a world where there's less
and less JS glue necessary and `wasm-bindgen` is just a polyfill for web
standards that otherwise all already exist.

Also note that there's no explicitly added tests for this since this is
largely just a refactoring of an internal implementation detail of
`wasm-bindgen`, but the main `wasm` test suite has many instances of
this path being taken, for example having imports like:

    (import "tests/wasm/duplicates_a.js" "foo" (func $__wbg_foo_969c253238f136f0 (type 1)))
    (import "tests/wasm/duplicates_b.js" "foo" (func $__wbg_foo_027958cb2e320a94 (type 0)))
    (import "./snippets/wasm-bindgen-3dff2bc911f0a20c/inline0.js" "trivial" (func $__wbg_trivial_75e27c84882af23b (type 1)))
    (import "./snippets/wasm-bindgen-3dff2bc911f0a20c/inline0.js" "incoming_bool" (func $__wbg_incomingbool_0f2d9f55f73a256f (type 0)))
@alexcrichton alexcrichton merged commit 36db788 into rustwasm:master Aug 1, 2019
@alexcrichton alexcrichton deleted the direct-imports branch August 1, 2019 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants