chore: drop usage of compileFunction (#10586)

jestjs · Oct 5, 2020 · a2090a0 · a2090a0
1 parent 97e683b
commit a2090a0
Show file tree

Hide file tree

Showing 11 changed files with 61 additions and 69 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -12,6 +12,7 @@
 - `[jest-circus, jest-jasmine2]` Find correct location for `test.each` tests ([#10413](https://github.com/facebook/jest/pull/10413))
 - `[jest-console]` Add `Console` constructor to `console` object ([#10502](https://github.com/facebook/jest/pull/10502))
 - `[jest-globals]` Fix lifecycle hook function types ([#10480](https://github.com/facebook/jest/pull/10480))
+- `[jest-runtime]` Remove usage of `vm.compileFunction` due to a performance issue ([#10586](https://github.com/facebook/jest/pull/10586))
 
 ### Chore & Maintenance
 

diff --git a/e2e/__tests__/__snapshots__/consoleLogOutputWhenRunInBand.test.ts.snap b/e2e/__tests__/__snapshots__/consoleLogOutputWhenRunInBand.test.ts.snap
@@ -20,6 +20,6 @@ exports[`prints console.logs when run with forceExit 3`] = `
   console.log
     Hey
 
-      at Object.log (__tests__/a-banana.js:1:30)
+      at Object.<anonymous> (__tests__/a-banana.js:1:1)
 
 `;
diff --git a/e2e/__tests__/__snapshots__/globals.test.ts.snap b/e2e/__tests__/__snapshots__/globals.test.ts.snap
@@ -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.describe (__tests__/onlyConstructs.test.js:1:1)
+      at Object.<anonymous> (__tests__/onlyConstructs.test.js:1:10)
 `;
 
 exports[`cannot have describe with no implementation 2`] = `

diff --git a/e2e/__tests__/consoleLogOutputWhenRunInBand.test.ts b/e2e/__tests__/consoleLogOutputWhenRunInBand.test.ts
@@ -15,6 +15,8 @@ const DIR = path.resolve(__dirname, '../console-log-output-when-run-in-band');
 beforeEach(() => cleanup(DIR));
 afterAll(() => cleanup(DIR));
 
+const nodeMajorVersion = Number(process.versions.node.split('.')[0]);
+
 test('prints console.logs when run with forceExit', () => {
   writeFiles(DIR, {
     '__tests__/a-banana.js': `
@@ -23,14 +25,26 @@ test('prints console.logs when run with forceExit', () => {
     'package.json': '{}',
   });
 
-  const {stderr, stdout, exitCode} = runJest(DIR, [
+  const {stderr, exitCode, ...res} = runJest(DIR, [
     '-i',
     '--ci=false',
     '--forceExit',
   ]);
+  let {stdout} = res;
 
   const {rest, summary} = extractSummary(stderr);
 
+  if (nodeMajorVersion < 12) {
+    expect(stdout).toContain(
+      'at Object.<anonymous>.test (__tests__/a-banana.js:1:1)',
+    );
+
+    stdout = stdout.replace(
+      'at Object.<anonymous>.test (__tests__/a-banana.js:1:1)',
+      'at Object.<anonymous> (__tests__/a-banana.js:1:1)',
+    );
+  }
+
   expect(exitCode).toBe(0);
   expect(wrap(rest)).toMatchSnapshot();
   expect(wrap(summary)).toMatchSnapshot();

diff --git a/e2e/__tests__/errorOnDeprecated.test.ts b/e2e/__tests__/errorOnDeprecated.test.ts
@@ -43,24 +43,18 @@ testFiles.forEach(testFile => {
     expect(result.exitCode).toBe(1);
     let {rest} = extractSummary(result.stderr);
 
-    if (testFile === 'defaultTimeoutInterval.test.js') {
+    if (
+      nodeMajorVersion < 12 &&
+      testFile === 'defaultTimeoutInterval.test.js'
+    ) {
       const lineEntry = '(__tests__/defaultTimeoutInterval.test.js:10:3)';
 
-      if (nodeMajorVersion < 10) {
-        expect(rest).toContain(`at Object.<anonymous>.test ${lineEntry}`);
+      expect(rest).toContain(`at Object.<anonymous>.test ${lineEntry}`);
 
-        rest = rest.replace(
-          `at Object.<anonymous>.test ${lineEntry}`,
-          `at Object.<anonymous> ${lineEntry}`,
-        );
-      } else if (nodeMajorVersion < 12) {
-        expect(rest).toContain(`at Object.test ${lineEntry}`);
-
-        rest = rest.replace(
-          `at Object.test ${lineEntry}`,
-          `at Object.<anonymous> ${lineEntry}`,
-        );
-      }
+      rest = rest.replace(
+        `at Object.<anonymous>.test ${lineEntry}`,
+        `at Object.<anonymous> ${lineEntry}`,
+      );
     }
 
     expect(wrap(rest)).toMatchSnapshot();

diff --git a/e2e/__tests__/failures.test.ts b/e2e/__tests__/failures.test.ts
@@ -39,7 +39,7 @@ test('not throwing Error objects', () => {
   expect(wrap(cleanStderr(stderr))).toMatchSnapshot();
   stderr = runJest(dir, ['duringTests.test.js']).stderr;
 
-  if (nodeMajorVersion < 10) {
+  if (nodeMajorVersion < 12) {
     const lineEntry = '(__tests__/duringTests.test.js:38:8)';
 
     expect(stderr).toContain(`at Object.<anonymous>.done ${lineEntry}`);
@@ -48,15 +48,6 @@ test('not throwing Error objects', () => {
       `at Object.<anonymous>.done ${lineEntry}`,
       `at Object.<anonymous> ${lineEntry}`,
     );
-  } else if (nodeMajorVersion < 12) {
-    const lineEntry = '(__tests__/duringTests.test.js:38:8)';
-
-    expect(stderr).toContain(`at Object.done ${lineEntry}`);
-
-    stderr = stderr.replace(
-      `at Object.done ${lineEntry}`,
-      `at Object.<anonymous> ${lineEntry}`,
-    );
   }
 
   expect(wrap(cleanStderr(stderr))).toMatchSnapshot();

diff --git a/packages/jest-reporters/src/coverage_reporter.ts b/packages/jest-reporters/src/coverage_reporter.ts
@@ -10,6 +10,7 @@ import * as fs from 'graceful-fs';
 import type {Config} from '@jest/types';
 import type {
   AggregatedResult,
+  RuntimeTransformResult,
   TestResult,
   V8CoverageResult,
 } from '@jest/test-result';
@@ -24,7 +25,6 @@ import Worker from 'jest-worker';
 import glob = require('glob');
 import v8toIstanbul = require('v8-to-istanbul');
 import type {RawSourceMap} from 'source-map';
-import type {TransformResult} from '@jest/transform';
 import BaseReporter from './base_reporter';
 import type {
   Context,
@@ -425,7 +425,7 @@ export default class CoverageReporter extends BaseReporter {
         this._v8CoverageResults.map(cov => ({result: cov.map(r => r.result)})),
       );
 
-      const fileTransforms = new Map<string, TransformResult>();
+      const fileTransforms = new Map<string, RuntimeTransformResult>();
 
       this._v8CoverageResults.forEach(res =>
         res.forEach(r => {
@@ -453,7 +453,7 @@ export default class CoverageReporter extends BaseReporter {
 
           const converter = v8toIstanbul(
             res.url,
-            0,
+            fileTransform?.wrapperLength ?? 0,
             fileTransform && sourcemapContent
               ? {
                   originalSource: fileTransform.originalCode,

diff --git a/packages/jest-runtime/src/index.ts b/packages/jest-runtime/src/index.ts
@@ -16,7 +16,6 @@ import {
   Context as VMContext,
   // @ts-expect-error: experimental, not added to the types
   Module as VMModule,
-  compileFunction,
 } from 'vm';
 import * as nativeModule from 'module';
 import type {Config, Global} from '@jest/types';
@@ -35,12 +34,11 @@ import {escapePathForRegex} from 'jest-regex-util';
 import {
   ScriptTransformer,
   ShouldInstrumentOptions,
-  TransformResult,
   TransformationOptions,
   handlePotentialSyntaxError,
   shouldInstrument,
 } from '@jest/transform';
-import type {V8CoverageResult} from '@jest/test-result';
+import type {RuntimeTransformResult, V8CoverageResult} from '@jest/test-result';
 import {CoverageInstrumenter, V8Coverage} from 'collect-v8-coverage';
 import * as fs from 'graceful-fs';
 import jestMock = require('jest-mock');
@@ -170,7 +168,7 @@ class Runtime {
   private _shouldUnmockTransitiveDependenciesCache: BooleanMap;
   private _sourceMapRegistry: StringMap;
   private _scriptTransformer: ScriptTransformer;
-  private _fileTransforms: Map<string, TransformResult>;
+  private _fileTransforms: Map<string, RuntimeTransformResult>;
   private _v8CoverageInstrumenter: CoverageInstrumenter | undefined;
   private _v8CoverageResult: V8Coverage | undefined;
   private _transitiveShouldMock: BooleanMap;
@@ -830,7 +828,7 @@ class Runtime {
       });
   }
 
-  // TODO - remove in Jest 26
+  // TODO - remove in Jest 27
   getSourceMapInfo(_coveredFiles: Set<string>): Record<string, string> {
     return {};
   }
@@ -1005,36 +1003,23 @@ class Runtime {
 
     let compiledFunction: ModuleWrapper | null = null;
 
+    const script = this.createScriptFromCode(transformedCode, filename);
+
+    let runScript: RunScriptEvalResult | null = null;
+
     // Use this if available instead of deprecated `JestEnvironment.runScript`
     if (typeof this._environment.getVmContext === 'function') {
       const vmContext = this._environment.getVmContext();
 
       if (vmContext) {
-        try {
-          compiledFunction = compileFunction(
-            transformedCode,
-            this.constructInjectedModuleParameters(),
-            {
-              filename,
-              parsingContext: vmContext,
-            },
-          ) as ModuleWrapper;
-        } catch (e) {
-          throw handlePotentialSyntaxError(e);
-        }
+        runScript = script.runInContext(vmContext, {filename});
       }
     } else {
-      const script = this.createScriptFromCode(transformedCode, filename);
-
-      const runScript = this._environment.runScript<RunScriptEvalResult>(
-        script,
-      );
+      runScript = this._environment.runScript<RunScriptEvalResult>(script);
+    }
 
-      if (runScript === null) {
-        compiledFunction = null;
-      } else {
-        compiledFunction = runScript[EVAL_RESULT_VARIABLE];
-      }
+    if (runScript !== null) {
+      compiledFunction = runScript[EVAL_RESULT_VARIABLE];
     }
 
     if (compiledFunction === null) {
@@ -1097,7 +1082,10 @@ class Runtime {
       source,
     );
 
-    this._fileTransforms.set(filename, transformedFile);
+    this._fileTransforms.set(filename, {
+      ...transformedFile,
+      wrapperLength: this.constructModuleWrapperStart().length,
+    });
 
     if (transformedFile.sourceMapPath) {
       this._sourceMapRegistry.set(filename, transformedFile.sourceMapPath);
@@ -1602,15 +1590,13 @@ class Runtime {
   }
 
   private wrapCodeInModuleWrapper(content: string) {
+    return this.constructModuleWrapperStart() + content + '\n}});';
+  }
+
+  private constructModuleWrapperStart() {
     const args = this.constructInjectedModuleParameters();
 
-    return (
-      '({"' +
-      EVAL_RESULT_VARIABLE +
-      `":function(${args.join(',')}){` +
-      content +
-      '\n}});'
-    );
+    return '({"' + EVAL_RESULT_VARIABLE + `":function(${args.join(',')}){`;
   }
 
   private constructInjectedModuleParameters(): Array<string> {

diff --git a/packages/jest-test-result/src/index.ts b/packages/jest-test-result/src/index.ts
@@ -19,6 +19,7 @@ export type {
   FailedAssertion,
   FormattedTestResults,
   Milliseconds,
+  RuntimeTransformResult,
   SerializableError,
   SnapshotSummary,
   Status,

diff --git a/packages/jest-test-result/src/types.ts b/packages/jest-test-result/src/types.ts
@@ -10,8 +10,13 @@ import type {ConsoleBuffer} from '@jest/console';
 import type {Config, TestResult, TransformTypes} from '@jest/types';
 import type {V8Coverage} from 'collect-v8-coverage';
 
+export interface RuntimeTransformResult extends TransformTypes.TransformResult {
+  // TODO: Make mandatory in Jest 27
+  wrapperLength?: number;
+}
+
 export type V8CoverageResult = Array<{
-  codeTransformResult: TransformTypes.TransformResult | undefined;
+  codeTransformResult: RuntimeTransformResult | undefined;
   result: V8Coverage[number];
 }>;
 

diff --git a/packages/jest-types/src/Transform.ts b/packages/jest-types/src/Transform.ts
@@ -9,6 +9,6 @@
 export type TransformResult = {
   code: string;
   originalCode: string;
-  mapCoverage?: boolean; // TODO - Remove in Jest 26
+  mapCoverage?: boolean; // TODO - Remove in Jest 27
   sourceMapPath: string | null;
 };