feat: add support for compileFunction allowing us to avoid the modu…

…le wrapper (#9252)
jestjs · Dec 2, 2019 · ad53773 · ad53773
1 parent 1d8245d
commit ad53773
Show file tree

Hide file tree

Showing 9 changed files with 138 additions and 41 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -17,15 +17,18 @@
 - `[jest-diff]` Add `changeColor` and `patchColor` options ([#8911](https://github.com/facebook/jest/pull/8911))
 - `[jest-diff]` Add `trailingSpaceFormatter` option and replace cyan with `commonColor` ([#8927](https://github.com/facebook/jest/pull/8927))
 - `[jest-diff]` Add `firstOrLastEmptyLineReplacement` option and export 3 `diffLines` functions ([#8955](https://github.com/facebook/jest/pull/8955))
+- `[jest-environment]` Add optional `compileFunction` next to `runScript` ([#9252](https://github.com/facebook/jest/pull/9252))
 - `[jest-environment-jsdom]` Add `fakeTimersLolex` ([#8925](https://github.com/facebook/jest/pull/8925))
 - `[jest-environment-node]` Add `fakeTimersLolex` ([#8925](https://github.com/facebook/jest/pull/8925))
 - `[jest-environment-node]` Add `queueMicrotask` ([#9140](https://github.com/facebook/jest/pull/9140))
+- `[jest-environment-node]` Implement `compileFunction` ([#9140](https://github.com/facebook/jest/pull/9140))
 - `[@jest/fake-timers]` Add Lolex as implementation of fake timers ([#8897](https://github.com/facebook/jest/pull/8897))
 - `[jest-get-type]` Add `BigInt` support. ([#8382](https://github.com/facebook/jest/pull/8382))
 - `[jest-matcher-utils]` Add `BigInt` support to `ensureNumbers` `ensureActualIsNumber`, `ensureExpectedIsNumber` ([#8382](https://github.com/facebook/jest/pull/8382))
 - `[jest-reporters]` Export utils for path formatting ([#9162](https://github.com/facebook/jest/pull/9162))
 - `[jest-runner]` Warn if a worker had to be force exited ([#8206](https://github.com/facebook/jest/pull/8206))
 - `[jest-runtime]` [**BREAKING**] Do not export `ScriptTransformer` - it can be imported from `@jest/transform` instead ([#9256](https://github.com/facebook/jest/pull/9256))
+- `[jest-runtime]` Use `JestEnvironment.compileFunction` if available to avoid the module wrapper ([#9252](https://github.com/facebook/jest/pull/9252))
 - `[jest-snapshot]` Display change counts in annotation lines ([#8982](https://github.com/facebook/jest/pull/8982))
 - `[jest-snapshot]` [**BREAKING**] Improve report when the matcher has properties ([#9104](https://github.com/facebook/jest/pull/9104))
 - `[jest-snapshot]` Improve colors when snapshots are updatable ([#9132](https://github.com/facebook/jest/pull/9132))

diff --git a/e2e/__tests__/__snapshots__/globals.test.ts.snap b/e2e/__tests__/__snapshots__/globals.test.ts.snap
@@ -18,20 +18,6 @@ Ran all test suites.
 `;
 
 exports[`cannot have describe with no implementation 1`] = `
-FAIL __tests__/onlyConstructs.test.js
-  ● Test suite failed to run
-
-    Missing second argument. It must be a callback function.
-
-      1 | 
-    > 2 |     describe('describe, no implementation');
-        |              ^
-      3 |   
-
-      at Object.<anonymous> (__tests__/onlyConstructs.test.js:2:14)
-`;
-
-exports[`cannot have describe with no implementation 2`] = `
 Test Suites: 1 failed, 1 total
 Tests:       0 total
 Snapshots:   0 total

diff --git a/e2e/__tests__/errorOnDeprecated.test.ts b/e2e/__tests__/errorOnDeprecated.test.ts
@@ -43,18 +43,24 @@ testFiles.forEach(testFile => {
     expect(result.exitCode).toBe(1);
     let {rest} = extractSummary(result.stderr);
 
-    if (
-      nodeMajorVersion < 12 &&
-      testFile === 'defaultTimeoutInterval.test.js'
-    ) {
+    if (testFile === 'defaultTimeoutInterval.test.js') {
       const lineEntry = '(__tests__/defaultTimeoutInterval.test.js:10:3)';
 
-      expect(rest).toContain(`at Object.<anonymous>.test ${lineEntry}`);
+      if (nodeMajorVersion < 10) {
+        expect(rest).toContain(`at Object.<anonymous>.test ${lineEntry}`);
 
-      rest = rest.replace(
-        `at Object.<anonymous>.test ${lineEntry}`,
-        `at Object.<anonymous> ${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}`,
+        );
+      }
     }
 
     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 < 12) {
+  if (nodeMajorVersion < 10) {
     const lineEntry = '(__tests__/duringTests.test.js:38:8)';
 
     expect(stderr).toContain(`at Object.<anonymous>.done ${lineEntry}`);
@@ -48,6 +48,15 @@ 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/e2e/__tests__/globals.test.ts b/e2e/__tests__/globals.test.ts
@@ -7,6 +7,7 @@
 
 import * as path from 'path';
 import {tmpdir} from 'os';
+import {compileFunction} from 'vm';
 import {wrap} from 'jest-snapshot-serializer-raw';
 import runJest from '../runJest';
 import {
@@ -125,7 +126,46 @@ test('cannot have describe with no implementation', () => {
 
   const rest = cleanStderr(stderr);
   const {summary} = extractSummary(stderr);
-  expect(wrap(rest)).toMatchSnapshot();
+
+  const rightTrimmedRest = rest
+    .split('\n')
+    .map(l => l.trimRight())
+    .join('\n')
+    .trim();
+
+  if (typeof compileFunction === 'function') {
+    expect(rightTrimmedRest).toEqual(
+      `
+FAIL __tests__/onlyConstructs.test.js
+  ● Test suite failed to run
+
+    Missing second argument. It must be a callback function.
+
+      1 |
+    > 2 |     describe('describe, no implementation');
+        |     ^
+      3 |
+
+      at Object.describe (__tests__/onlyConstructs.test.js:2:5)
+    `.trim(),
+    );
+  } else {
+    expect(rightTrimmedRest).toEqual(
+      `
+FAIL __tests__/onlyConstructs.test.js
+  ● Test suite failed to run
+
+    Missing second argument. It must be a callback function.
+
+      1 |
+    > 2 |     describe('describe, no implementation');
+        |              ^
+      3 |
+
+      at Object.<anonymous> (__tests__/onlyConstructs.test.js:2:14)
+    `.trim(),
+    );
+  }
   expect(wrap(summary)).toMatchSnapshot();
 });
 

diff --git a/packages/jest-environment-node/src/index.ts b/packages/jest-environment-node/src/index.ts
@@ -5,7 +5,13 @@
  * LICENSE file in the root directory of this source tree.
  */
 
-import {Context, Script, createContext, runInContext} from 'vm';
+import {
+  Context,
+  Script,
+  compileFunction,
+  createContext,
+  runInContext,
+} from 'vm';
 import {Config, Global} from '@jest/types';
 import {ModuleMocker} from 'jest-mock';
 import {installCommonGlobals} from 'jest-util';
@@ -110,6 +116,22 @@ class NodeEnvironment implements JestEnvironment {
     }
     return null;
   }
+
+  compileFunction(code: string, params: Array<string>, filename: string) {
+    if (this.context) {
+      return compileFunction(code, params, {
+        filename,
+        parsingContext: this.context,
+      }) as any;
+    }
+    return null;
+  }
+}
+
+// `jest-runtime` checks for `compileFunction`, so this makes sure to not expose that function if it's unsupported by this version of node
+// Should be removed when we drop support for node 8
+if (typeof compileFunction !== 'function') {
+  delete NodeEnvironment.prototype.compileFunction;
 }
 
 export = NodeEnvironment;
diff --git a/packages/jest-environment/src/index.ts b/packages/jest-environment/src/index.ts
@@ -44,6 +44,11 @@ export declare class JestEnvironment {
   fakeTimersLolex: LolexFakeTimers | null;
   moduleMocker: jestMock.ModuleMocker | null;
   runScript<T = unknown>(script: Script): T | null;
+  compileFunction?<T = unknown>(
+    code: string,
+    params: Array<string>,
+    filename: string,
+  ): T | null;
   setup(): Promise<void>;
   teardown(): Promise<void>;
   handleTestEvent?(event: Circus.Event, state: Circus.State): void;

diff --git a/packages/jest-runtime/src/index.ts b/packages/jest-runtime/src/index.ts
@@ -730,20 +730,41 @@ class Runtime {
       }
     }
 
-    const script = this.createScriptFromCode(transformedFile.code, filename);
+    let compiledFunction: ModuleWrapper | null;
+
+    if (typeof this._environment.compileFunction === 'function') {
+      try {
+        compiledFunction = this._environment.compileFunction<ModuleWrapper>(
+          transformedFile.code,
+          this.constructInjectedModuleParameters(),
+          filename,
+        );
+      } catch (e) {
+        throw handlePotentialSyntaxError(e);
+      }
+    } else {
+      const script = this.createScriptFromCode(transformedFile.code, filename);
 
-    const runScript = this._environment.runScript<RunScriptEvalResult>(script);
+      const runScript = this._environment.runScript<RunScriptEvalResult>(
+        script,
+      );
 
-    if (runScript === null) {
+      if (runScript === null) {
+        compiledFunction = null;
+      } else {
+        compiledFunction = runScript[EVAL_RESULT_VARIABLE];
+      }
+    }
+
+    if (compiledFunction === null) {
       this._logFormattedReferenceError(
         'You are trying to `import` a file after the Jest environment has been torn down.',
       );
       process.exitCode = 1;
       return;
     }
 
-    //Wrapper
-    runScript[EVAL_RESULT_VARIABLE].call(
+    compiledFunction.call(
       localModule.exports,
       localModule as NodeModule, // module object
       localModule.exports, // module exports
@@ -1107,7 +1128,19 @@ class Runtime {
   }
 
   private wrapCodeInModuleWrapper(content: string) {
-    const args = [
+    const args = this.constructInjectedModuleParameters();
+
+    return (
+      '({"' +
+      EVAL_RESULT_VARIABLE +
+      `":function(${args.join(',')}){` +
+      content +
+      '\n}});'
+    );
+  }
+
+  private constructInjectedModuleParameters() {
+    return [
       'module',
       'exports',
       'require',
@@ -1117,14 +1150,6 @@ class Runtime {
       'jest',
       ...this._config.extraGlobals,
     ];
-
-    return (
-      '({"' +
-      EVAL_RESULT_VARIABLE +
-      `":function(${args.join(',')}){` +
-      content +
-      '\n}});'
-    );
   }
 }
 

diff --git a/packages/jest-transform/src/enhanceUnexpectedTokenMessage.ts b/packages/jest-transform/src/enhanceUnexpectedTokenMessage.ts
@@ -17,7 +17,8 @@ export default function handlePotentialSyntaxError(
   }
 
   if (
-    e instanceof SyntaxError &&
+    // `instanceof` might come from the wrong context
+    e.name === 'SyntaxError' &&
     (e.message.includes('Unexpected token') ||
       e.message.includes('Cannot use import')) &&
     !e.message.includes(' expected')