-
-
Notifications
You must be signed in to change notification settings - Fork 6.5k
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
feat: support WebAssembly (Wasm) imports in ESM modules #13505
Changes from 2 commits
95ef976
645db88
9985179
5d73f9a
bc6d38c
734bbc5
612d480
dcff11e
e524e40
08c8906
66c8312
6586a24
01f8d78
27d73b6
2aea674
79e0f40
0d4945e
26db3fd
1e6d75b
d4e9b73
ad32edd
397bcd1
976ef61
66a2312
7188086
26e7022
607fc0f
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 |
---|---|---|
@@ -0,0 +1,23 @@ | ||
/** | ||
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. | ||
* | ||
* This source code is licensed under the MIT license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
*/ | ||
|
||
import {resolve} from 'path'; | ||
import {json as runJest} from '../runJest'; | ||
|
||
const DIR = resolve(__dirname, '../native-esm-wasm'); | ||
|
||
test('runs WASM test with native ESM', () => { | ||
const {exitCode, json} = runJest(DIR, [], { | ||
nodeOptions: | ||
'--experimental-vm-modules --experimental-wasm-modules --no-warnings', | ||
}); | ||
|
||
expect(exitCode).toBe(0); | ||
|
||
expect(json.numTotalTests).toBe(1); | ||
expect(json.numPassedTests).toBe(1); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,12 @@ | ||
/** | ||
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. | ||
* | ||
* This source code is licensed under the MIT license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
*/ | ||
|
||
import {getAnswer} from '../answer'; | ||
|
||
test('test answer', () => { | ||
expect(getAnswer()).toBe(42); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,13 @@ | ||
/** | ||
* Copyright (c) Facebook, Inc. and its affiliates. All Rights Reserved. | ||
* | ||
* This source code is licensed under the MIT license found in the | ||
* LICENSE file in the root directory of this source tree. | ||
*/ | ||
|
||
import * as wasm from './answer.wasm'; | ||
kachkaev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
export function getAnswer() { | ||
const ret = wasm.getAnswer(); | ||
return ret; | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
{ | ||
"type": "module", | ||
"name": "native-esm-wasm", | ||
"jest": { | ||
"testEnvironment": "node", | ||
"transform": {} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -398,9 +398,12 @@ export default class Runtime { | |
|
||
// unstable as it should be replaced by https://github.com/nodejs/modules/issues/393, and we don't want people to use it | ||
unstable_shouldLoadAsEsm(path: string): boolean { | ||
return Resolver.unstable_shouldLoadAsEsm( | ||
path, | ||
this._config.extensionsToTreatAsEsm, | ||
return ( | ||
path.endsWith('wasm') || | ||
Resolver.unstable_shouldLoadAsEsm( | ||
path, | ||
this._config.extensionsToTreatAsEsm, | ||
) | ||
); | ||
} | ||
|
||
|
@@ -441,6 +444,15 @@ export default class Runtime { | |
'Promise initialization should be sync - please report this bug to Jest!', | ||
); | ||
|
||
if (modulePath.endsWith('wasm')) { | ||
const wasm = this._importWasmModule(modulePath, context); | ||
|
||
this._esmoduleRegistry.set(cacheKey, wasm); | ||
|
||
transformResolve(); | ||
return wasm; | ||
} | ||
|
||
if (this._resolver.isCoreModule(modulePath)) { | ||
const core = this._importCoreModule(modulePath, context); | ||
this._esmoduleRegistry.set(cacheKey, core); | ||
|
@@ -568,7 +580,7 @@ export default class Runtime { | |
|
||
const mime = match.groups.mime; | ||
if (mime === 'application/wasm') { | ||
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. Quite a few lines below are marked as changed because of extra indent. Hiding whitespace in diff viewer makes it a bit easier to see the real difference. |
||
throw new Error('WASM is currently not supported'); | ||
throw new Error('WASM in data URLs is currently not supported'); | ||
kachkaev marked this conversation as resolved.
Show resolved
Hide resolved
|
||
} | ||
|
||
const encoding = match.groups.encoding; | ||
|
@@ -1640,6 +1652,49 @@ export default class Runtime { | |
return evaluateSyntheticModule(module); | ||
} | ||
|
||
private async _importWasmModule(moduleName: string, context: VMContext) { | ||
const wasmModule = await WebAssembly.compile(fs.readFileSync(moduleName)); | ||
|
||
const exports = WebAssembly.Module.exports(wasmModule); | ||
const imports = WebAssembly.Module.imports(wasmModule); | ||
|
||
const moduleLookup: Record<string, VMModule> = {}; | ||
for (const {module} of imports) { | ||
if (moduleLookup[module] === undefined) { | ||
moduleLookup[module] = await this.linkAndEvaluateModule( | ||
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. should these be put into 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. I’ve replaced 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. Hmmm I should have kept
This happens when Wasm imports refer back to a javascript file that calls it. E.g.:
which is then mistakenly passed to to I can try submiting a follow-up PR with a fix in a few days. 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. Thanks! 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. |
||
await this.resolveModule(module, moduleName, context), | ||
); | ||
} | ||
} | ||
|
||
const syntheticModule = new SyntheticModule( | ||
exports.map(({name}) => name), | ||
function () { | ||
const importsObject: WebAssembly.Imports = {}; | ||
for (const {module, name} of imports) { | ||
if (!importsObject[module]) { | ||
importsObject[module] = {}; | ||
} | ||
importsObject[module][name] = moduleLookup[module].namespace[name]; | ||
} | ||
const wasmInstance = new WebAssembly.Instance( | ||
wasmModule, | ||
importsObject, | ||
); | ||
for (const {name} of exports) { | ||
// @ts-expect-error: TS doesn't know what `this` is | ||
this.setExport(name, wasmInstance.exports[name]); | ||
} | ||
}, | ||
{ | ||
context, | ||
identifier: moduleName, | ||
}, | ||
); | ||
|
||
return syntheticModule; | ||
} | ||
|
||
private _getMockedNativeModule(): typeof nativeModule.Module { | ||
if (this._moduleImplementation) { | ||
return this._moduleImplementation; | ||
|
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.
Interesting: CI has passed with and without
--experimental-wasm-modules
. I guess that in our case the usage of this flag is not necessary, because wasm imports are implemented vianew SyntheticModule(...)
(--experimental-vm-modules
).Let me try to get rid of the flag completely and merge
native-esm-wasm
withnative-esm
for simplicity.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.
Done in dcff11e. Because
--experimental-wasm-modules
is not necessary, seems like we don’t even have to update the docs! WASM just works™Existing markdown still looks accurate: https://jestjs.io/docs/ecmascript-modules. It does not mention all supported cases, so not sure if wasm should be any special here.
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.
Awesome! It might make sense to mention that wasm doesn't need an extra flag (at least at the moment - node might decide to unflag
vm
modules and at that point require it for wasm? unsure)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.
Done in 27d73b6 (happy to improve the docs further based on your suggestions).