feat: return transformed code as a string, do not wrap in vm.Script (…

…#9253)

CHANGELOG.md

@@ -31,6 +31,7 @@
 - `[jest-snapshot]` Improve colors when snapshots are updatable ([#9132](https://github.com/facebook/jest/pull/9132))
 - `[jest-snapshot]` Ignore indentation for most serialized objects ([#9203](https://github.com/facebook/jest/pull/9203))
 - `[jest-transform]` Create `createTranspilingRequire` function for easy transpiling modules ([#9194](https://github.com/facebook/jest/pull/9194))
+- `[jest-transform]` [**BREAKING**] Return transformed code as a string, do not wrap in `vm.Script` ([#9253](https://github.com/facebook/jest/pull/9253))
 - `[@jest/test-result]` Create method to create empty `TestResult` ([#8867](https://github.com/facebook/jest/pull/8867))
 - `[jest-worker]` [**BREAKING**] Return a promise from `end()`, resolving with the information whether workers exited gracefully ([#8206](https://github.com/facebook/jest/pull/8206))
 - `[jest-reporters]` Transform file paths into hyperlinks ([#8980](https://github.com/facebook/jest/pull/8980))

packages/jest-environment/package.json

@@ -11,7 +11,6 @@
   "types": "build/index.d.ts",
   "dependencies": {
     "@jest/fake-timers": "^24.9.0",
-    "@jest/transform": "^24.9.0",
     "@jest/types": "^24.9.0",
     "jest-mock": "^24.9.0"
   },

packages/jest-environment/src/index.ts

@@ -8,7 +8,6 @@
 import {Script} from 'vm';
 import {Circus, Config, Global} from '@jest/types';
 import jestMock = require('jest-mock');
-import {ScriptTransformer} from '@jest/transform';
 import {
   JestFakeTimers as LegacyFakeTimers,
   LolexFakeTimers,
@@ -27,6 +26,7 @@ export type EnvironmentContext = Partial<{
 
 // Different Order than https://nodejs.org/api/modules.html#modules_the_module_wrapper , however needs to be in the form [jest-transform]ScriptTransformer accepts
 export type ModuleWrapper = (
+  this: Module['exports'],
   module: Module,
   exports: Module['exports'],
   require: Module['require'],
@@ -43,9 +43,7 @@ export declare class JestEnvironment {
   fakeTimers: LegacyFakeTimers<unknown> | null;
   fakeTimersLolex: LolexFakeTimers | null;
   moduleMocker: jestMock.ModuleMocker | null;
-  runScript(
-    script: Script,
-  ): {[ScriptTransformer.EVAL_RESULT_VARIABLE]: ModuleWrapper} | null;
+  runScript<T = unknown>(script: Script): T | null;
   setup(): Promise<void>;
   teardown(): Promise<void>;
   handleTestEvent?(event: Circus.Event, state: Circus.State): void;

packages/jest-environment/tsconfig.json

@@ -6,7 +6,6 @@
   },
   "references": [
     {"path": "../jest-fake-timers"},
-    {"path": "../jest-transform"},
     {"path": "../jest-types"},
     {"path": "../jest-util"}
   ]

packages/jest-runtime/src/__tests__/__snapshots__/runtime_wrap.js.snap

@@ -0,0 +1,11 @@
+// Jest Snapshot v1, https://goo.gl/fbAQLP
+
+exports[`Runtime wrapCodeInModuleWrapper generates the correct args for the module wrapper 1`] = `
+({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest){module.exports = "Hello!"
+}});
+`;
+
+exports[`Runtime wrapCodeInModuleWrapper injects "extra globals" 1`] = `
+({"Object.<anonymous>":function(module,exports,require,__dirname,__filename,global,jest,Math){module.exports = "Hello!"
+}});
+`;

packages/jest-runtime/src/__tests__/instrumentation.test.ts

@@ -6,7 +6,6 @@
  *
  */
 
-import * as vm from 'vm';
 import * as path from 'path';
 import * as os from 'os';
 import {ScriptTransformer} from '@jest/transform';
@@ -32,10 +31,9 @@ it('instruments files', () => {
       ...makeGlobalConfig({collectCoverage: true}),
       changedFiles: undefined,
     },
-  ).script;
-  expect(instrumented instanceof vm.Script).toBe(true);
+  );
   // We can't really snapshot the resulting coverage, because it depends on
   // absolute path of the file, which will be different on different
   // machines
-  expect(vm.Script.mock.calls[0][0]).toMatch(`gcv = "__coverage__"`);
+  expect(instrumented.code).toMatch(`gcv = "__coverage__"`);
 });

packages/jest-runtime/src/__tests__/runtime_wrap.js

@@ -0,0 +1,34 @@
+/**
+ * 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 {wrap} from 'jest-snapshot-serializer-raw';
+let createRuntime;
+
+describe('Runtime', () => {
+  beforeEach(() => {
+    createRuntime = require('createRuntime');
+  });
+
+  describe('wrapCodeInModuleWrapper', () => {
+    it('generates the correct args for the module wrapper', async () => {
+      const runtime = await createRuntime(__filename);
+
+      expect(
+        wrap(runtime.wrapCodeInModuleWrapper('module.exports = "Hello!"')),
+      ).toMatchSnapshot();
+    });
+
+    it('injects "extra globals"', async () => {
+      const runtime = await createRuntime(__filename, {extraGlobals: ['Math']});
+
+      expect(
+        wrap(runtime.wrapCodeInModuleWrapper('module.exports = "Hello!"')),
+      ).toMatchSnapshot();
+    });
+  });
+});

packages/jest-runtime/src/index.ts

@@ -6,12 +6,14 @@
  */
 
 import * as path from 'path';
+import {Script} from 'vm';
 import {Config} from '@jest/types';
 import {
   Jest,
   JestEnvironment,
   LocalModuleRequire,
   Module,
+  ModuleWrapper,
 } from '@jest/environment';
 import {SourceMapRegistry} from '@jest/source-map';
 import jestMock = require('jest-mock');
@@ -25,6 +27,7 @@ import {
   ScriptTransformer,
   ShouldInstrumentOptions,
   TransformationOptions,
+  handlePotentialSyntaxError,
   shouldInstrument,
 } from '@jest/transform';
 import * as fs from 'graceful-fs';
@@ -78,6 +81,10 @@ const getModuleNameMapper = (config: Config.ProjectConfig) => {
 
 const unmockRegExpCache = new WeakMap();
 
+const EVAL_RESULT_VARIABLE = 'Object.<anonymous>';
+
+type RunScriptEvalResult = {[EVAL_RESULT_VARIABLE]: ModuleWrapper};
+
 /* eslint-disable-next-line no-redeclare */
 class Runtime {
   private _cacheFS: CacheFS;
@@ -487,7 +494,6 @@ class Runtime {
       collectCoverage: this._coverageOptions.collectCoverage,
       collectCoverageFrom: this._coverageOptions.collectCoverageFrom,
       collectCoverageOnlyFrom: this._coverageOptions.collectCoverageOnlyFrom,
-      extraGlobals: this._config.extraGlobals,
     };
   }
 
@@ -724,7 +730,9 @@ class Runtime {
       }
     }
 
-    const runScript = this._environment.runScript(transformedFile.script);
+    const script = this.createScriptFromCode(transformedFile.code, filename);
+
+    const runScript = this._environment.runScript<RunScriptEvalResult>(script);
 
     if (runScript === null) {
       this._logFormattedReferenceError(
@@ -735,7 +743,7 @@ class Runtime {
     }
 
     //Wrapper
-    runScript[ScriptTransformer.EVAL_RESULT_VARIABLE].call(
+    runScript[EVAL_RESULT_VARIABLE].call(
       localModule.exports,
       localModule as NodeModule, // module object
       localModule.exports, // module exports
@@ -762,6 +770,19 @@ class Runtime {
     this._currentlyExecutingModulePath = lastExecutingModulePath;
   }
 
+  private createScriptFromCode(scriptSource: string, filename: string) {
+    try {
+      return new Script(this.wrapCodeInModuleWrapper(scriptSource), {
+        displayErrors: true,
+        filename: this._resolver.isCoreModule(filename)
+          ? `jest-nodejs-core-${filename}`
+          : filename,
+      });
+    } catch (e) {
+      throw handlePotentialSyntaxError(e);
+    }
+  }
+
   private _requireCoreModule(moduleName: string) {
     if (moduleName === 'process') {
       return this._environment.global.process;
@@ -1084,6 +1105,27 @@ class Runtime {
         formatStackTrace(stack, this._config, {noStackTrace: false}),
     );
   }
+
+  private wrapCodeInModuleWrapper(content: string) {
+    const args = [
+      'module',
+      'exports',
+      'require',
+      '__dirname',
+      '__filename',
+      'global',
+      'jest',
+      ...this._config.extraGlobals,
+    ];
+
+    return (
+      '({"' +
+      EVAL_RESULT_VARIABLE +
+      `":function(${args.join(',')}){` +
+      content +
+      '\n}});'
+    );
+  }
 }
 
 export = Runtime;

packages/jest-transform/src/ScriptTransformer.ts

@@ -7,7 +7,6 @@
 
 import {createHash} from 'crypto';
 import * as path from 'path';
-import {Script} from 'vm';
 import {Config} from '@jest/types';
 import {createDirectory, interopRequireDefault, isPromise} from 'jest-util';
 import * as fs from 'graceful-fs';
@@ -28,7 +27,7 @@ import {
   Transformer,
 } from './types';
 import shouldInstrument from './shouldInstrument';
-import enhanceUnexpectedTokenMessage from './enhanceUnexpectedTokenMessage';
+import handlePotentialSyntaxError from './enhanceUnexpectedTokenMessage';
 
 type ProjectCache = {
   configString: string;
@@ -60,7 +59,6 @@ async function waitForPromiseWithCleanup(
 }
 
 export default class ScriptTransformer {
-  static EVAL_RESULT_VARIABLE: 'Object.<anonymous>';
   private _cache: ProjectCache;
   private _config: Config.ProjectConfig;
   private _transformCache: Map<Config.Path, Transformer>;
@@ -347,12 +345,11 @@ export default class ScriptTransformer {
   ): TransformResult {
     const isInternalModule = !!(options && options.isInternalModule);
     const isCoreModule = !!(options && options.isCoreModule);
-    const extraGlobals = (options && options.extraGlobals) || [];
     const content = stripShebang(
       fileSource || fs.readFileSync(filename, 'utf8'),
     );
 
-    let wrappedCode: string;
+    let code = content;
     let sourceMapPath: string | null = null;
     let mapCoverage = false;
 
@@ -369,36 +366,18 @@ export default class ScriptTransformer {
           instrument,
         );
 
-        wrappedCode = wrap(transformedSource.code, ...extraGlobals);
+        code = transformedSource.code;
         sourceMapPath = transformedSource.sourceMapPath;
         mapCoverage = transformedSource.mapCoverage;
-      } else {
-        wrappedCode = wrap(content, ...extraGlobals);
       }
 
       return {
+        code,
         mapCoverage,
-        script: new Script(wrappedCode, {
-          displayErrors: true,
-          filename: isCoreModule ? 'jest-nodejs-core-' + filename : filename,
-        }),
         sourceMapPath,
       };
     } catch (e) {
-      if (e.codeFrame) {
-        e.stack = e.message + '\n' + e.codeFrame;
-      }
-
-      if (
-        e instanceof SyntaxError &&
-        (e.message.includes('Unexpected token') ||
-          e.message.includes('Cannot use import')) &&
-        !e.message.includes(' expected')
-      ) {
-        throw enhanceUnexpectedTokenMessage(e);
-      }
-
-      throw e;
+      throw handlePotentialSyntaxError(e);
     }
   }
 
@@ -698,27 +677,3 @@ const calcTransformRegExp = (config: Config.ProjectConfig) => {
 
   return transformRegexp;
 };
-
-const wrap = (content: string, ...extras: Array<string>) => {
-  const globals = new Set([
-    'module',
-    'exports',
-    'require',
-    '__dirname',
-    '__filename',
-    'global',
-    'jest',
-    ...extras,
-  ]);
-
-  return (
-    '({"' +
-    ScriptTransformer.EVAL_RESULT_VARIABLE +
-    `":function(${Array.from(globals).join(',')}){` +
-    content +
-    '\n}});'
-  );
-};
-
-// TODO: Can this be added to the static property?
-ScriptTransformer.EVAL_RESULT_VARIABLE = 'Object.<anonymous>';