-
Notifications
You must be signed in to change notification settings - Fork 29.6k
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
esm: --experimental-wasm-modules integration support #27659
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,9 +1,12 @@ | ||
'use strict'; | ||
|
||
/* global WebAssembly */ | ||
|
||
const { | ||
JSON, | ||
Object, | ||
SafeMap, | ||
StringPrototype, | ||
JSON | ||
StringPrototype | ||
} = primordials; | ||
|
||
const { NativeModule } = require('internal/bootstrap/loaders'); | ||
|
@@ -72,11 +75,11 @@ translators.set('commonjs', async function commonjsStrategy(url, isMain) { | |
]; | ||
if (module && module.loaded) { | ||
const exports = module.exports; | ||
return createDynamicModule(['default'], url, (reflect) => { | ||
return createDynamicModule([], ['default'], url, (reflect) => { | ||
reflect.exports.default.set(exports); | ||
}); | ||
} | ||
return createDynamicModule(['default'], url, () => { | ||
return createDynamicModule([], ['default'], url, () => { | ||
debug(`Loading CJSModule ${url}`); | ||
// We don't care about the return val of _load here because Module#load | ||
// will handle it for us by checking the loader registry and filling the | ||
|
@@ -97,7 +100,7 @@ translators.set('builtin', async function builtinStrategy(url) { | |
} | ||
module.compileForPublicLoader(true); | ||
return createDynamicModule( | ||
[...module.exportKeys, 'default'], url, (reflect) => { | ||
[], [...module.exportKeys, 'default'], url, (reflect) => { | ||
debug(`Loading BuiltinModule ${url}`); | ||
module.reflect = reflect; | ||
for (const key of module.exportKeys) | ||
|
@@ -116,7 +119,7 @@ translators.set('json', async function jsonStrategy(url) { | |
let module = CJSModule._cache[modulePath]; | ||
if (module && module.loaded) { | ||
const exports = module.exports; | ||
return createDynamicModule(['default'], url, (reflect) => { | ||
return createDynamicModule([], ['default'], url, (reflect) => { | ||
reflect.exports.default.set(exports); | ||
}); | ||
} | ||
|
@@ -136,8 +139,32 @@ translators.set('json', async function jsonStrategy(url) { | |
throw err; | ||
} | ||
CJSModule._cache[modulePath] = module; | ||
return createDynamicModule(['default'], url, (reflect) => { | ||
return createDynamicModule([], ['default'], url, (reflect) => { | ||
debug(`Parsing JSONModule ${url}`); | ||
reflect.exports.default.set(module.exports); | ||
}); | ||
}); | ||
|
||
// Strategy for loading a wasm module | ||
translators.set('wasm', async function(url) { | ||
const pathname = fileURLToPath(url); | ||
const buffer = await readFileAsync(pathname); | ||
debug(`Translating WASMModule ${url}`); | ||
let compiled; | ||
try { | ||
compiled = await WebAssembly.compile(buffer); | ||
} catch (err) { | ||
err.message = pathname + ': ' + err.message; | ||
throw err; | ||
} | ||
|
||
const imports = | ||
WebAssembly.Module.imports(compiled).map(({ module }) => module); | ||
const exports = WebAssembly.Module.exports(compiled).map(({ name }) => name); | ||
|
||
return createDynamicModule(imports, exports, url, (reflect) => { | ||
const { exports } = new WebAssembly.Instance(compiled, reflect.imports); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In a cycle-closing edge, will reflect.imports usually be an empty object? If so, this will be basically the right semantics. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. On the unexecuted cycle edge, There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually cycles would throw for any accesses to uninitialized let bindings on the namespaces, since these would be in TDZ. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. To summarize:
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. And specifically these errors are only thrown if the WASM module actually attempts to access the given binding. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Right, so in particular, Wasm exports should always act like const bindings, and all imports are accessed during Instantiate (in the ESM evaluate phase). Does this implementation do that? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. yep! |
||
for (const expt of Object.keys(exports)) | ||
reflect.exports[expt].set(exports[expt]); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,15 @@ | ||
// Flags: --experimental-modules --experimental-wasm-modules | ||
import '../common/index.mjs'; | ||
import { add, addImported } from '../fixtures/es-modules/simple.wasm'; | ||
import { state } from '../fixtures/es-modules/wasm-dep.mjs'; | ||
import { strictEqual } from 'assert'; | ||
|
||
strictEqual(state, 'WASM Start Executed'); | ||
|
||
strictEqual(add(10, 20), 30); | ||
|
||
strictEqual(addImported(0), 42); | ||
|
||
strictEqual(state, 'WASM JS Function Executed'); | ||
|
||
strictEqual(addImported(1), 43); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,23 @@ | ||
;; Compiled using the WebAssembly Tootkit (https://github.com/WebAssembly/wabt) | ||
;; $ wat2wasm simple.wat -o simple.wasm | ||
|
||
(module | ||
guybedford marked this conversation as resolved.
Show resolved
Hide resolved
|
||
(import "./wasm-dep.mjs" "jsFn" (func $jsFn (result i32))) | ||
(import "./wasm-dep.mjs" "jsInitFn" (func $jsInitFn)) | ||
(export "add" (func $add)) | ||
(export "addImported" (func $addImported)) | ||
(start $startFn) | ||
(func $startFn | ||
call $jsInitFn | ||
) | ||
(func $add (param $a i32) (param $b i32) (result i32) | ||
local.get $a | ||
local.get $b | ||
i32.add | ||
) | ||
(func $addImported (param $a i32) (result i32) | ||
local.get $a | ||
call $jsFn | ||
i32.add | ||
) | ||
) |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
import { strictEqual } from 'assert'; | ||
|
||
export function jsFn () { | ||
state = 'WASM JS Function Executed'; | ||
return 42; | ||
} | ||
|
||
export let state = 'JS Function Executed'; | ||
|
||
export function jsInitFn () { | ||
strictEqual(state, 'JS Function Executed'); | ||
state = 'WASM Start Executed'; | ||
} |
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.
There's one additional mention of the non-existent
--entry-type
CLI flag 7 lines below this one. Is this the right PR to remove or update that material?