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(runtime): use compileFunction over new Script #15461

Open
wants to merge 7 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
- `[jest-runtime]` [**BREAKING**] Make it mandatory to pass `globalConfig` to the `Runtime` constructor ([#15044](https://github.com/jestjs/jest/pull/15044))
- `[jest-runtime]` Add `unstable_unmockModule` ([#15080](https://github.com/jestjs/jest/pull/15080))
- `[jest-runtime]` Add `onGenerateMock` transformer callback for auto generated callbacks ([#15433](https://github.com/jestjs/jest/pull/15433))
- `[jest-runtime]` [**BREAKING**] User `vm.compileFunction` over `vm.Script` ([#15461](https://github.com/jestjs/jest/pull/15461))
SimenB marked this conversation as resolved.
Show resolved Hide resolved
- `[@jest/schemas]` Upgrade `@sinclair/typebox` to v0.34 ([#15450](https://github.com/jestjs/jest/pull/15450))
- `[@jest/types]` `test.each()`: Accept a readonly (`as const`) table properly ([#14565](https://github.com/jestjs/jest/pull/14565))
- `[@jest/types]` Improve argument type inference passed to `test` and `describe` callback functions from `each` tables ([#14920](https://github.com/jestjs/jest/pull/14920))
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:41)
at Object.log (__tests__/a-banana.js:1:30)
"
`;
4 changes: 2 additions & 2 deletions e2e/__tests__/__snapshots__/coverageProviderV8.test.ts.snap
Original file line number Diff line number Diff line change
Expand Up @@ -9,8 +9,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 | 59.37 | 50 | 33.33 | 59.37 |
module.js | 79.16 | 66.66 | 50 | 79.16 | 14-16,19-20
uncovered.js | 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 @@ exports[`cannot have describe with no implementation 1`] = `
Missing second argument. It must be a callback function.

> 1 | describe('describe, no implementation');
| ^
| ^

at Object.<anonymous> (__tests__/onlyConstructs.test.js:1:40)"
at Object.describe (__tests__/onlyConstructs.test.js:1:1)"
`;

exports[`cannot have describe with no implementation 2`] = `
Expand Down
21 changes: 12 additions & 9 deletions e2e/__tests__/jestEnvironmentJsdom.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import {tmpdir} from 'os';
import * as path from 'path';
import {onNodeVersions} from '@jest/test-utils';
import {cleanup, writeFiles} from '../Utils';
import runJest from '../runJest';

Expand All @@ -15,14 +16,16 @@ const DIR = path.resolve(tmpdir(), 'jest_environment_jsdom_test');
beforeEach(() => cleanup(DIR));
afterAll(() => cleanup(DIR));

test('check is not leaking memory', () => {
writeFiles(DIR, {
'__tests__/a.test.js': "test('a', () => console.log('a'));",
'__tests__/b.test.js': "test('b', () => console.log('b'));",
'package.json': JSON.stringify({jest: {testEnvironment: 'jsdom'}}),
});
onNodeVersions('> 16', () => {
Copy link
Member Author

Choose a reason for hiding this comment

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

this is unfortunate, but hopefully most people have moved on from node 16 😀

test('check is not leaking memory', () => {
writeFiles(DIR, {
'__tests__/a.test.js': "test('a', () => console.log('a'));",
'__tests__/b.test.js': "test('b', () => console.log('b'));",
'package.json': JSON.stringify({jest: {testEnvironment: 'jsdom'}}),
});

const {stderr} = runJest(DIR, ['--detect-leaks', '--runInBand']);
expect(stderr).toMatch(/PASS\s__tests__\/a.test.js/);
expect(stderr).toMatch(/PASS\s__tests__\/b.test.js/);
const {stderr} = runJest(DIR, ['--detect-leaks', '--runInBand']);
expect(stderr).toMatch(/PASS\s__tests__\/a.test.js/);
expect(stderr).toMatch(/PASS\s__tests__\/b.test.js/);
});
});
2 changes: 1 addition & 1 deletion packages/jest-reporters/src/CoverageReporter.ts
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ export default class CoverageReporter extends BaseReporter {

const converter = v8toIstanbul(
res.url,
fileTransform?.wrapperLength ?? 0,
0,
fileTransform && sourcemapContent
? {
originalSource: fileTransform.originalCode,
Expand Down

This file was deleted.

41 changes: 33 additions & 8 deletions packages/jest-runtime/src/__tests__/runtime_wrap.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,23 +13,48 @@ describe('Runtime', () => {
createRuntime = require('createRuntime');
});

describe('wrapCodeInModuleWrapper', () => {
it('generates the correct args for the module wrapper', async () => {
describe('constructInjectedModuleParameters', () => {
it('generates the correct args', async () => {
const runtime = await createRuntime(__filename);

expect(
runtime.wrapCodeInModuleWrapper('module.exports = "Hello!"'),
).toMatchSnapshot();
expect(runtime.constructInjectedModuleParameters()).toEqual([
'module',
'exports',
'require',
'__dirname',
'__filename',
'jest',
]);
});

it('injects "extra globals"', async () => {
const runtime = await createRuntime(__filename, {
sandboxInjectedGlobals: ['Math'],
});

expect(
runtime.wrapCodeInModuleWrapper('module.exports = "Hello!"'),
).toMatchSnapshot();
expect(runtime.constructInjectedModuleParameters()).toEqual([
'module',
'exports',
'require',
'__dirname',
'__filename',
'jest',
'Math',
]);
});

it('avoid injecting `jest` if `injectGlobals = false`', async () => {
const runtime = await createRuntime(__filename, {
injectGlobals: false,
});

expect(runtime.constructInjectedModuleParameters()).toEqual([
'module',
'exports',
'require',
'__dirname',
'__filename',
]);
});
});
});
107 changes: 39 additions & 68 deletions packages/jest-runtime/src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,14 +9,14 @@ import nativeModule = require('module');
import * as path from 'path';
import {URL, fileURLToPath, pathToFileURL} from 'url';
import {
Script,
// @ts-expect-error: experimental, not added to the types
SourceTextModule,
// @ts-expect-error: experimental, not added to the types
SyntheticModule,
type Context as VMContext,
// @ts-expect-error: experimental, not added to the types
type Module as VMModule,
compileFunction,
} from 'vm';
import {parse as parseCjs} from 'cjs-module-lexer';
import {CoverageInstrumenter, type V8Coverage} from 'collect-v8-coverage';
Expand All @@ -33,15 +33,12 @@ import type {
import type {LegacyFakeTimers, ModernFakeTimers} from '@jest/fake-timers';
import type {expect, jest} from '@jest/globals';
import type {SourceMapRegistry} from '@jest/source-map';
import type {
RuntimeTransformResult,
TestContext,
V8CoverageResult,
} from '@jest/test-result';
import type {TestContext, V8CoverageResult} from '@jest/test-result';
import {
type CallerTransformOptions,
type ScriptTransformer,
type ShouldInstrumentOptions,
type TransformResult,
type TransformationOptions,
handlePotentialSyntaxError,
shouldInstrument,
Expand Down Expand Up @@ -145,10 +142,6 @@ const isWasm = (modulePath: string): boolean => modulePath.endsWith('.wasm');

const unmockRegExpCache = new WeakMap();

const EVAL_RESULT_VARIABLE = 'Object.<anonymous>';

type RunScriptEvalResult = {[EVAL_RESULT_VARIABLE]: ModuleWrapper};

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

const supportsNodeColonModulePrefixInRequire = (() => {
Expand Down Expand Up @@ -203,11 +196,11 @@ export default class Runtime {
>;
private readonly _sourceMapRegistry: SourceMapRegistry;
private readonly _scriptTransformer: ScriptTransformer;
private readonly _fileTransforms: Map<string, RuntimeTransformResult>;
private readonly _fileTransforms: Map<string, TransformResult>;
private readonly _fileTransformsMutex: Map<string, Promise<void>>;
private _v8CoverageInstrumenter: CoverageInstrumenter | undefined;
private _v8CoverageResult: V8Coverage | undefined;
private _v8CoverageSources: Map<string, RuntimeTransformResult> | undefined;
private _v8CoverageSources: Map<string, TransformResult> | undefined;
private readonly _transitiveShouldMock: Map<string, boolean>;
private _unmockList: RegExp | undefined;
private readonly _virtualMocks: Map<string, boolean>;
Expand Down Expand Up @@ -1593,21 +1586,10 @@ export default class Runtime {

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];
}
const compiledFunction = this.createScriptFromCode(
transformedCode,
filename,
);

if (compiledFunction === null) {
this._logFormattedReferenceError(
Expand Down Expand Up @@ -1680,10 +1662,7 @@ export default class Runtime {
source,
);

this._fileTransforms.set(filename, {
...transformedFile,
wrapperLength: this.constructModuleWrapperStart().length,
});
this._fileTransforms.set(filename, transformedFile);

if (transformedFile.sourceMapPath) {
this._sourceMapRegistry.set(filename, transformedFile.sourceMapPath);
Expand All @@ -1708,10 +1687,7 @@ export default class Runtime {
);

if (this._fileTransforms.get(filename)?.code !== transformedFile.code) {
this._fileTransforms.set(filename, {
...transformedFile,
wrapperLength: 0,
});
this._fileTransforms.set(filename, transformedFile);
}

if (transformedFile.sourceMapPath) {
Expand All @@ -1721,34 +1697,39 @@ export default class Runtime {
}

private createScriptFromCode(scriptSource: string, filename: string) {
const vmContext = this._environment.getVmContext();

if (vmContext == null) {
return null;
}

try {
const scriptFilename = this._resolver.isCoreModule(filename)
? `jest-nodejs-core-${filename}`
: filename;
return new Script(this.wrapCodeInModuleWrapper(scriptSource), {
columnOffset: this._fileTransforms.get(filename)?.wrapperLength,
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);
return compileFunction(
scriptSource,
this.constructInjectedModuleParameters(),
{
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 module = await this.resolveModule(
specifier,
scriptFilename,
vmContext,
);

return this.linkAndEvaluateModule(module);
},
parsingContext: vmContext,
},
});
) as ModuleWrapper;
} catch (error: any) {
throw handlePotentialSyntaxError(error);
}
Expand Down Expand Up @@ -2466,16 +2447,6 @@ export default class Runtime {
);
}

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

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

return `({"${EVAL_RESULT_VARIABLE}":function(${args.join(',')}){`;
}

private constructInjectedModuleParameters(): Array<string> {
return [
'module',
Expand Down
4 changes: 1 addition & 3 deletions packages/jest-test-result/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,7 @@ import type {Circus, Config, TestResult, TransformTypes} from '@jest/types';
import type {IHasteFS, IModuleMap} from 'jest-haste-map';
import type Resolver from 'jest-resolve';

export interface RuntimeTransformResult extends TransformTypes.TransformResult {
wrapperLength: number;
}
export type RuntimeTransformResult = TransformTypes.TransformResult;

export type V8CoverageResult = Array<{
codeTransformResult: RuntimeTransformResult | undefined;
Expand Down
Loading