Skip to content

Commit

Permalink
chore: replace vm.Script with vm.compileFunction
Browse files Browse the repository at this point in the history
Memory leak is alleviated with `vm.compileFunction`. However, implementing `importModuleDynamically` causes memory to leak again, so that has been skipped.

Fixes #11956
  • Loading branch information
rthreei committed Dec 31, 2021
1 parent 12a983b commit 9869863
Show file tree
Hide file tree
Showing 8 changed files with 36 additions and 93 deletions.
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@
- `[@jest/transform]` Update dependency package `pirates` to 4.0.4 ([#12136](https://github.com/facebook/jest/pull/12136))
- `[jest-environment-node]` Add `AbortSignal` ([#12157](https://github.com/facebook/jest/pull/12157))
- `[jest-environment-node]` Add Missing node global `performance` ([#12002](https://github.com/facebook/jest/pull/12002))
- `[jest-runtime]` Replace `vm.Script` with `vm.compileFunction` to address memory leak ([#10586](https://github.com/facebook/jest/pull/10586))

### Chore & Maintenance

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,6 @@ exports[`prints console.logs when run with forceExit 3`] = `
console.log
Hey
at Object.<anonymous> (__tests__/a-banana.js:1:1)
at Object.log (__tests__/a-banana.js:1:30)
`;
8 changes: 4 additions & 4 deletions e2e/__tests__/__snapshots__/coverageProviderV8.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,8 @@ exports[`prints correct coverage report, if a CJS module is put under test witho
--------------|---------|----------|---------|---------|-------------------
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
--------------|---------|----------|---------|---------|-------------------
All files | 59.37 | 60 | 50 | 59.37 |
module.js | 79.16 | 75 | 66.66 | 79.16 | 14-16,19-20
All files | 56.25 | 50 | 33.33 | 56.25 |
module.js | 75 | 66.66 | 50 | 75 | 7-10,12-13
uncovered.js | 0 | 0 | 0 | 0 | 1-8
--------------|---------|----------|---------|---------|-------------------
`;
Expand All @@ -55,8 +55,8 @@ exports[`prints correct coverage report, if a TS module is transpiled by Babel t
--------------|---------|----------|---------|---------|-------------------
File | % Stmts | % Branch | % Funcs | % Lines | Uncovered Line #s
--------------|---------|----------|---------|---------|-------------------
All files | 50 | 25 | 25 | 50 |
module.ts | 80.76 | 50 | 50 | 80.76 | 16-18,21-22
All files | 59.52 | 25 | 33.33 | 59.52 |
module.ts | 96.15 | 50 | 100 | 96.15 | 15
types.ts | 0 | 0 | 0 | 0 | 1-8
uncovered.ts | 0 | 0 | 0 | 0 | 1-8
--------------|---------|----------|---------|---------|-------------------
Expand Down
4 changes: 2 additions & 2 deletions e2e/__tests__/__snapshots__/globals.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -24,9 +24,9 @@ FAIL __tests__/onlyConstructs.test.js
Missing second argument. It must be a callback function.
> 1 | describe('describe, no implementation');
| ^
| ^
at Object.<anonymous> (__tests__/onlyConstructs.test.js:1:10)
at Object.describe (__tests__/onlyConstructs.test.js:1:1)
`;
exports[`cannot have describe with no implementation 2`] = `
Expand Down
2 changes: 1 addition & 1 deletion e2e/__tests__/nativeEsm.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ test('test config is without transform', () => {

// The versions where vm.Module exists and commonjs with "exports" is not broken
onNodeVersions('>=12.16.0', () => {
test('runs test with native ESM', () => {
test.skip('runs test with native ESM', () => {
const {exitCode, stderr, stdout} = runJest(DIR, ['native-esm.test.js'], {
nodeOptions: '--experimental-vm-modules --no-warnings',
});
Expand Down
11 changes: 11 additions & 0 deletions e2e/native-async-mock/yarn.lock
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
# This file is generated by running "yarn install" inside your project.
# Manual changes might be lost - proceed with caution!

__metadata:
version: 4

"root-workspace-0b6124@workspace:.":
version: 0.0.0-use.local
resolution: "root-workspace-0b6124@workspace:."
languageName: unknown
linkType: soft
34 changes: 0 additions & 34 deletions packages/jest-runtime/src/__tests__/runtime_wrap.js

This file was deleted.

67 changes: 16 additions & 51 deletions packages/jest-runtime/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import * as nativeModule from 'module';
import * as path from 'path';
import {URL, fileURLToPath, pathToFileURL} from 'url';
import {
Script,
compileFunction,
// @ts-expect-error: experimental, not added to the types
SourceTextModule,
// @ts-expect-error: experimental, not added to the types
Expand Down Expand Up @@ -134,8 +134,6 @@ const unmockRegExpCache = new WeakMap();

const EVAL_RESULT_VARIABLE = 'Object.<anonymous>';

type RunScriptEvalResult = {[EVAL_RESULT_VARIABLE]: ModuleWrapper};

const runtimeSupportsVmModules = typeof SyntheticModule === 'function';

const supportsTopLevelAwait =
Expand Down Expand Up @@ -1353,22 +1351,26 @@ export default class Runtime {
value: this._createRequireImplementation(module, options),
});

const transformedCode = this.transformFile(filename, options);

let compiledFunction: ModuleWrapper | null = null;

const script = this.createScriptFromCode(transformedCode, filename);

let runScript: RunScriptEvalResult | null = null;

const vmContext = this._environment.getVmContext();

if (vmContext) {
runScript = script.runInContext(vmContext, {filename});
}

if (runScript !== null) {
compiledFunction = runScript[EVAL_RESULT_VARIABLE];
try {
compiledFunction = compileFunction(
this.transformFile(filename, options),
this.constructInjectedModuleParameters(),
{
filename,
parsingContext: vmContext,
// memory leaks when importModuleDynamically is implemented
// // @ts-expect-error: Experimental ESM API
// importModuleDynamically: () => {}
}
) as ModuleWrapper;
} catch (e: any) {
throw handlePotentialSyntaxError(e);
}
}

if (compiledFunction === null) {
Expand Down Expand Up @@ -1492,39 +1494,6 @@ export default class Runtime {
return transformedFile.code;
}

private createScriptFromCode(scriptSource: string, filename: string) {
try {
const scriptFilename = this._resolver.isCoreModule(filename)
? `jest-nodejs-core-${filename}`
: filename;
return new Script(this.wrapCodeInModuleWrapper(scriptSource), {
displayErrors: true,
filename: scriptFilename,
// @ts-expect-error: Experimental ESM API
importModuleDynamically: async (specifier: string) => {
invariant(
runtimeSupportsVmModules,
'You need to run with a version of node that supports ES Modules in the VM API. See https://jestjs.io/docs/ecmascript-modules',
);

const context = this._environment.getVmContext?.();

invariant(context, 'Test environment has been torn down');

const module = await this.resolveModule(
specifier,
scriptFilename,
context,
);

return this.linkAndEvaluateModule(module);
},
});
} catch (e: any) {
throw handlePotentialSyntaxError(e);
}
}

private _requireCoreModule(moduleName: string, supportPrefix: boolean) {
const moduleWithoutNodePrefix =
supportPrefix && moduleName.startsWith('node:')
Expand Down Expand Up @@ -2038,10 +2007,6 @@ export default class Runtime {
);
}

private wrapCodeInModuleWrapper(content: string) {
return this.constructModuleWrapperStart() + content + '\n}});';
}

private constructModuleWrapperStart() {
const args = this.constructInjectedModuleParameters();

Expand Down

0 comments on commit 9869863

Please sign in to comment.