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

feat: support WebAssembly (Wasm) imports in ESM modules #13505

Merged
merged 27 commits into from Nov 6, 2022
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
27 commits
Select commit Hold shift + click to select a range
95ef976
Add failing test
kachkaev Oct 24, 2022
645db88
Update `jest-runtime` to fix tests
kachkaev Oct 24, 2022
9985179
Support data:application/wasm imports
kachkaev Oct 27, 2022
5d73f9a
Run tests twice (with and without --experimental-wasm-modules)
kachkaev Oct 27, 2022
bc6d38c
Merge remote-tracking branch 'u/main' into native-esm-wasm
kachkaev Oct 27, 2022
734bbc5
Cover more rows
kachkaev Oct 27, 2022
612d480
Add CHANGELOG entry
kachkaev Oct 27, 2022
dcff11e
Simplify tests
kachkaev Oct 27, 2022
e524e40
Improve `path.endsWith`
kachkaev Oct 27, 2022
08c8906
Delete unused file
kachkaev Oct 27, 2022
66c8312
Make changelog phrase more explicit
kachkaev Oct 27, 2022
6586a24
Fix Wasm casing
kachkaev Oct 27, 2022
01f8d78
Update CHANGELOG.md
kachkaev Oct 30, 2022
27d73b6
Mention Wasm in docs
kachkaev Oct 30, 2022
2aea674
Merge remote-tracking branch 'origin/main' into native-esm-wasm
kachkaev Oct 30, 2022
79e0f40
Use Wasm file from mdn examples
kachkaev Oct 30, 2022
0d4945e
Implement `readFileBuffer`
kachkaev Oct 30, 2022
26db3fd
Call loadEsmModule instead of linkAndEvaluateModule in _importWasmModule
kachkaev Oct 30, 2022
1e6d75b
Update e2e/native-esm/__tests__/native-esm-wasm.test.js
kachkaev Oct 31, 2022
d4e9b73
Merge remote-tracking branch 'origin/main' into native-esm-wasm
kachkaev Nov 1, 2022
ad32edd
Add another test case for dynamic import
kachkaev Nov 1, 2022
397bcd1
Extract `isWasm` function
kachkaev Nov 1, 2022
976ef61
Merge remote-tracking branch 'origin/main' into native-esm-wasm
kachkaev Nov 6, 2022
66a2312
Revert "Implement `readFileBuffer`"
SimenB Nov 6, 2022
7188086
separate buffer cache
SimenB Nov 6, 2022
26e7022
tweak
SimenB Nov 6, 2022
607fc0f
clear new cache as well
SimenB Nov 6, 2022
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
23 changes: 23 additions & 0 deletions e2e/__tests__/nativeEsmWasm.test.ts
@@ -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',
Copy link
Contributor Author

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 via new SyntheticModule(...) (--experimental-vm-modules).

Let me try to get rid of the flag completely and merge native-esm-wasm with native-esm for simplicity.

Copy link
Contributor Author

@kachkaev kachkaev Oct 27, 2022

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.

Copy link
Member

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)

Copy link
Contributor Author

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).

});

expect(exitCode).toBe(0);

expect(json.numTotalTests).toBe(1);
expect(json.numPassedTests).toBe(1);
});
12 changes: 12 additions & 0 deletions e2e/native-esm-wasm/__tests__/answer.test.js
@@ -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);
});
13 changes: 13 additions & 0 deletions e2e/native-esm-wasm/answer.js
@@ -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;
}
Binary file added e2e/native-esm-wasm/answer.wasm
Binary file not shown.
8 changes: 8 additions & 0 deletions e2e/native-esm-wasm/package.json
@@ -0,0 +1,8 @@
{
"type": "module",
"name": "native-esm-wasm",
"jest": {
"testEnvironment": "node",
"transform": {}
}
}
2 changes: 1 addition & 1 deletion e2e/native-esm/__tests__/native-esm.test.js
Expand Up @@ -258,7 +258,7 @@ test('imports from "data:text/javascript" URI with invalid data fail', async ()
test('imports from "data:application/wasm" URI not supported', async () => {
await expect(() =>
import('data:application/wasm,96cafe00babe'),
).rejects.toThrow('WASM is currently not supported');
).rejects.toThrow('WASM in data URLs is currently not supported');
});

test('supports imports from "data:application/json" URI', async () => {
Expand Down
63 changes: 59 additions & 4 deletions packages/jest-runtime/src/index.ts
Expand Up @@ -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,
)
);
}

Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -568,7 +580,7 @@ export default class Runtime {

const mime = match.groups.mime;
if (mime === 'application/wasm') {
Copy link
Contributor Author

Choose a reason for hiding this comment

The 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;
Expand Down Expand Up @@ -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(
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should these be put into this. _esmoduleRegistry as well?

Copy link
Contributor Author

@kachkaev kachkaev Oct 30, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I’ve replaced linkAndEvaluateModule with loadEsmModule in 26db3fd. Not 100% sure if this is right, but AFAIU, loadEsmModule is a wrapper function that deals with _esmoduleRegistry. If that’s not the best solution, feel free to push to my branch if the required change is not trivial.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmmm I should have kept linkAndEvaluateModule. Using loadEsmModule here can result

TypeError: modulePath.endsWith is not a function
  at isWasm (../../../node_modules/jest-runtime/build/index.js:210:41)

This happens when Wasm imports refer back to a javascript file that calls it. E.g.:

imports: [
  {
    module: './index_bg.js',
    name: '__wbindgen_json_parse',
    kind: 'function'
  },
  {
    module: './index_bg.js',
    name: '__wbindgen_json_serialize',
    kind: 'function'
  },
  {
    module: './index_bg.js',
    name: '__wbindgen_throw',
    kind: 'function'
  }
]

loadEsmModule returns

modulePath: SourceTextModule {
  status: 'linking',
  identifier: '/path/to/index_bg.js',
  context: {
    clearInterval: [Function: clearInterval],
    clearTimeout: [Function: clearTimeout],
    setInterval: [Function: setInterval],
    setTimeout: [Function: setTimeout] {
      [Symbol(nodejs.util.promisify.custom)]: [Getter]
    },

which is then mistakenly passed to to isWasm instead of a string.

I can try submiting a follow-up PR with a fix in a few days.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks!

Copy link
Contributor Author

Choose a reason for hiding this comment

The 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;
Expand Down