diff --git a/CHANGELOG.md b/CHANGELOG.md index 467a35388fe7..1ad046caa216 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -30,7 +30,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**] Do not automatically pass arguments to the script wrapper ([#9253](https://github.com/facebook/jest/pull/9253)) +- `[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)) diff --git a/packages/jest-environment/package.json b/packages/jest-environment/package.json index bc0712d971a6..290eacd140fe 100644 --- a/packages/jest-environment/package.json +++ b/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" }, diff --git a/packages/jest-environment/src/index.ts b/packages/jest-environment/src/index.ts index 87951acb0530..d28dba5dda70 100644 --- a/packages/jest-environment/src/index.ts +++ b/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, @@ -44,9 +43,7 @@ export declare class JestEnvironment { fakeTimers: LegacyFakeTimers | null; fakeTimersLolex: LolexFakeTimers | null; moduleMocker: jestMock.ModuleMocker | null; - runScript( - script: Script, - ): {[ScriptTransformer.EVAL_RESULT_VARIABLE]: ModuleWrapper} | null; + runScript(script: Script): T | null; setup(): Promise; teardown(): Promise; handleTestEvent?(event: Circus.Event, state: Circus.State): void; diff --git a/packages/jest-environment/tsconfig.json b/packages/jest-environment/tsconfig.json index cfce4b39485b..658545960324 100644 --- a/packages/jest-environment/tsconfig.json +++ b/packages/jest-environment/tsconfig.json @@ -6,7 +6,6 @@ }, "references": [ {"path": "../jest-fake-timers"}, - {"path": "../jest-transform"}, {"path": "../jest-types"}, {"path": "../jest-util"} ] diff --git a/packages/jest-runtime/src/index.ts b/packages/jest-runtime/src/index.ts index 74e2393091a9..8be50fe0e9b5 100644 --- a/packages/jest-runtime/src/index.ts +++ b/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'); @@ -78,6 +80,10 @@ const getModuleNameMapper = (config: Config.ProjectConfig) => { const unmockRegExpCache = new WeakMap(); +const EVAL_RESULT_VARIABLE = 'Object.'; + +type RunScriptEvalResult = {[EVAL_RESULT_VARIABLE]: ModuleWrapper}; + /* eslint-disable-next-line no-redeclare */ class Runtime { static ScriptTransformer: typeof ScriptTransformer; @@ -489,7 +495,6 @@ class Runtime { collectCoverage: this._coverageOptions.collectCoverage, collectCoverageFrom: this._coverageOptions.collectCoverageFrom, collectCoverageOnlyFrom: this._coverageOptions.collectCoverageOnlyFrom, - moduleArguments: this.constructInjectedModuleArguments(), }; } @@ -726,7 +731,14 @@ class Runtime { } } - const runScript = this._environment.runScript(transformedFile.script); + const script = new Script(this.wrap(transformedFile.code), { + displayErrors: true, + filename: this._resolver.isCoreModule(filename) + ? `jest-nodejs-core-${filename}` + : filename, + }); + + const runScript = this._environment.runScript(script); if (runScript === null) { this._logFormattedReferenceError( @@ -737,7 +749,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 @@ -1087,6 +1099,16 @@ class Runtime { ); } + private wrap(content: string) { + return ( + '({"' + + EVAL_RESULT_VARIABLE + + `":function(${this.constructInjectedModuleArguments().join(',')}){` + + content + + '\n}});' + ); + } + private constructInjectedModuleArguments() { return [ 'module', diff --git a/packages/jest-transform/src/ScriptTransformer.ts b/packages/jest-transform/src/ScriptTransformer.ts index d57959930b67..b87eb648a020 100644 --- a/packages/jest-transform/src/ScriptTransformer.ts +++ b/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'; @@ -60,7 +59,6 @@ async function waitForPromiseWithCleanup( } export default class ScriptTransformer { - static EVAL_RESULT_VARIABLE: 'Object.'; private _cache: ProjectCache; private _config: Config.ProjectConfig; private _transformCache: Map; @@ -347,12 +345,11 @@ export default class ScriptTransformer { ): TransformResult { const isInternalModule = !!(options && options.isInternalModule); const isCoreModule = !!(options && options.isCoreModule); - const moduleArguments = (options && options.moduleArguments) || []; const content = stripShebang( fileSource || fs.readFileSync(filename, 'utf8'), ); - let scriptContent: string; + let code = content; let sourceMapPath: string | null = null; let mapCoverage = false; @@ -369,20 +366,14 @@ export default class ScriptTransformer { instrument, ); - scriptContent = wrap(transformedSource.code, moduleArguments); + code = transformedSource.code; sourceMapPath = transformedSource.sourceMapPath; mapCoverage = transformedSource.mapCoverage; - } else { - scriptContent = wrap(content, moduleArguments); } return { + code, mapCoverage, - script: new Script(scriptContent, { - displayErrors: true, - filename: isCoreModule ? 'jest-nodejs-core-' + filename : filename, - }), - scriptContent, sourceMapPath, }; } catch (e) { @@ -413,7 +404,7 @@ export default class ScriptTransformer { if (!options.isCoreModule) { instrument = shouldInstrument(filename, options, this._config); - scriptCacheKey = getScriptCacheKey(filename, instrument, options); + scriptCacheKey = getScriptCacheKey(filename, instrument); const result = this._cache.transformedFiles.get(scriptCacheKey); if (result) { return result; @@ -667,19 +658,9 @@ const readCacheFile = (cachePath: Config.Path): string | null => { return fileData; }; -const getScriptCacheKey = ( - filename: Config.Path, - instrument: boolean, - options: Options, -) => { +const getScriptCacheKey = (filename: Config.Path, instrument: boolean) => { const mtime = fs.statSync(filename).mtime; - return ( - filename + - '_' + - mtime.getTime() + - (instrument ? '_instrumented' : '') + - (options.moduleArguments ? options.moduleArguments.join('') : '') - ); + return filename + '_' + mtime.getTime() + (instrument ? '_instrumented' : ''); }; const calcIgnorePatternRegExp = (config: Config.ProjectConfig) => { @@ -709,13 +690,3 @@ const calcTransformRegExp = (config: Config.ProjectConfig) => { return transformRegexp; }; - -const wrap = (content: string, moduleArguments: Array) => - '({"' + - ScriptTransformer.EVAL_RESULT_VARIABLE + - `":function(${moduleArguments.join(',')}){` + - content + - '\n}});'; - -// TODO: Can this be added to the static property? -ScriptTransformer.EVAL_RESULT_VARIABLE = 'Object.'; diff --git a/packages/jest-transform/src/__tests__/__snapshots__/script_transformer.test.js.snap b/packages/jest-transform/src/__tests__/__snapshots__/script_transformer.test.js.snap index 6da305887ab6..38b4e2ae1b32 100644 --- a/packages/jest-transform/src/__tests__/__snapshots__/script_transformer.test.js.snap +++ b/packages/jest-transform/src/__tests__/__snapshots__/script_transformer.test.js.snap @@ -75,7 +75,7 @@ Object { `; exports[`ScriptTransformer transforms a file properly 1`] = ` -({"Object.":function(){/* istanbul ignore next */ +/* istanbul ignore next */ function cov_25u22311x4() { var path = "/fruits/banana.js"; var hash = "4be0f6184160be573fc43f7c2a5877c28b7ce249"; @@ -122,11 +122,10 @@ function cov_25u22311x4() { cov_25u22311x4().s[0]++; module.exports = "banana"; -}}); `; exports[`ScriptTransformer transforms a file properly 2`] = ` -({"Object.":function(){/* istanbul ignore next */ +/* istanbul ignore next */ function cov_23yvu8etmu() { var path = "/fruits/kiwi.js"; var hash = "7705dd5fcfbc884dcea7062944cfb8cc5d141d1a"; @@ -217,48 +216,33 @@ module.exports = () => { cov_23yvu8etmu().s[1]++; return "kiwi"; }; -}}); -`; - -exports[`ScriptTransformer transforms a file properly 3`] = ` -({"Object.":function(){module.exports = "banana"; -}}); `; exports[`ScriptTransformer uses multiple preprocessors 1`] = ` -({"Object.":function(){const TRANSFORMED = { +const TRANSFORMED = { filename: '/fruits/banana.js', script: 'module.exports = "banana";', config: '{"automock":false,"browser":false,"cache":true,"cacheDirectory":"/cache/","clearMocks":false,"coveragePathIgnorePatterns":[],"cwd":"/test_root_dir/","detectLeaks":false,"detectOpenHandles":false,"errorOnDeprecated":false,"extraGlobals":[],"filter":null,"forceCoverageMatch":[],"globalSetup":null,"globalTeardown":null,"globals":{},"haste":{"providesModuleNodeModules":[]},"moduleDirectories":[],"moduleFileExtensions":["js"],"moduleLoader":"/test_module_loader_path","moduleNameMapper":[],"modulePathIgnorePatterns":[],"modulePaths":[],"name":"test","prettierPath":"prettier","resetMocks":false,"resetModules":false,"resolver":null,"restoreMocks":false,"rootDir":"/","roots":[],"runner":"jest-runner","setupFiles":[],"setupFilesAfterEnv":[],"skipFilter":false,"skipNodeResolution":false,"snapshotResolver":null,"snapshotSerializers":[],"testEnvironment":"node","testEnvironmentOptions":{},"testLocationInResults":false,"testMatch":[],"testPathIgnorePatterns":[],"testRegex":["\\\\.test\\\\.js$"],"testRunner":"jest-jasmine2","testURL":"http://localhost","timers":"real","transform":[["^.+\\\\.js$","test_preprocessor"],["^.+\\\\.css$","css-preprocessor"]],"transformIgnorePatterns":["/node_modules/"],"unmockedModulePathPatterns":null,"watchPathIgnorePatterns":[]}', }; -}}); `; exports[`ScriptTransformer uses multiple preprocessors 2`] = ` -({"Object.":function(){module.exports = { +module.exports = { filename: /styles/App.css, rawFirstLine: root {, }; -}}); `; -exports[`ScriptTransformer uses multiple preprocessors 3`] = ` -({"Object.":function(){module.exports = "react"; -}}); -`; +exports[`ScriptTransformer uses multiple preprocessors 3`] = `module.exports = "react";`; exports[`ScriptTransformer uses the supplied preprocessor 1`] = ` -({"Object.":function(){const TRANSFORMED = { +const TRANSFORMED = { filename: '/fruits/banana.js', script: 'module.exports = "banana";', config: '{"automock":false,"browser":false,"cache":true,"cacheDirectory":"/cache/","clearMocks":false,"coveragePathIgnorePatterns":[],"cwd":"/test_root_dir/","detectLeaks":false,"detectOpenHandles":false,"errorOnDeprecated":false,"extraGlobals":[],"filter":null,"forceCoverageMatch":[],"globalSetup":null,"globalTeardown":null,"globals":{},"haste":{"providesModuleNodeModules":[]},"moduleDirectories":[],"moduleFileExtensions":["js"],"moduleLoader":"/test_module_loader_path","moduleNameMapper":[],"modulePathIgnorePatterns":[],"modulePaths":[],"name":"test","prettierPath":"prettier","resetMocks":false,"resetModules":false,"resolver":null,"restoreMocks":false,"rootDir":"/","roots":[],"runner":"jest-runner","setupFiles":[],"setupFilesAfterEnv":[],"skipFilter":false,"skipNodeResolution":false,"snapshotResolver":null,"snapshotSerializers":[],"testEnvironment":"node","testEnvironmentOptions":{},"testLocationInResults":false,"testMatch":[],"testPathIgnorePatterns":[],"testRegex":["\\\\.test\\\\.js$"],"testRunner":"jest-jasmine2","testURL":"http://localhost","timers":"real","transform":[["^.+\\\\.js$","test_preprocessor"]],"transformIgnorePatterns":["/node_modules/"],"unmockedModulePathPatterns":null,"watchPathIgnorePatterns":[]}', }; -}}); `; -exports[`ScriptTransformer uses the supplied preprocessor 2`] = ` -({"Object.":function(){module.exports = "react"; -}}); -`; +exports[`ScriptTransformer uses the supplied preprocessor 2`] = `module.exports = "react";`; exports[`ScriptTransformer warns of unparseable inlined source maps from the preprocessor 1`] = `jest-transform: The source map produced for the file /fruits/banana.js by preprocessor-with-sourcemaps was invalid. Proceeding without source mapping for that file.`; diff --git a/packages/jest-transform/src/__tests__/script_transformer.test.js b/packages/jest-transform/src/__tests__/script_transformer.test.js index eaffd3487bdc..55371151e4c1 100644 --- a/packages/jest-transform/src/__tests__/script_transformer.test.js +++ b/packages/jest-transform/src/__tests__/script_transformer.test.js @@ -37,7 +37,6 @@ jest ...jest.requireActual('jest-util'), createDirectory: jest.fn(), })) - .mock('vm') .mock('path', () => jest.requireActual('path').posix); jest.mock( @@ -213,52 +212,51 @@ describe('ScriptTransformer', () => { it('transforms a file properly', () => { const scriptTransformer = new ScriptTransformer(config); - const response = scriptTransformer.transform( + const transformedBananaWithCoverage = scriptTransformer.transform( '/fruits/banana.js', makeGlobalConfig({collectCoverage: true}), - ).script; + ); - expect(response instanceof vm.Script).toBe(true); - expect(wrap(vm.Script.mock.calls[0][0])).toMatchSnapshot(); + expect(wrap(transformedBananaWithCoverage.code)).toMatchSnapshot(); // no-cache case expect(fs.readFileSync).toHaveBeenCalledTimes(1); expect(fs.readFileSync).toBeCalledWith('/fruits/banana.js', 'utf8'); // in-memory cache - const response2 = scriptTransformer.transform( + const transformedBananaWithCoverageAgain = scriptTransformer.transform( '/fruits/banana.js', makeGlobalConfig({collectCoverage: true}), - ).script; - expect(response2).toBe(response); + ); + expect(transformedBananaWithCoverageAgain).toBe( + transformedBananaWithCoverage, + ); - scriptTransformer.transform( + const transformedKiwiWithCoverage = scriptTransformer.transform( '/fruits/kiwi.js', makeGlobalConfig({collectCoverage: true}), ); - const snapshot = vm.Script.mock.calls[1][0]; - expect(wrap(snapshot)).toMatchSnapshot(); + expect(wrap(transformedKiwiWithCoverage.code)).toMatchSnapshot(); - scriptTransformer.transform( + const transformedKiwiWithCoverageAgain = scriptTransformer.transform( '/fruits/kiwi.js', makeGlobalConfig({collectCoverage: true}), ); - expect(vm.Script.mock.calls[0][0]).not.toEqual(snapshot); - expect(vm.Script.mock.calls[0][0]).not.toMatch(/instrumented kiwi/); + expect(transformedBananaWithCoverage.code).not.toEqual( + transformedKiwiWithCoverage.code, + ); + expect(transformedBananaWithCoverage.code).not.toMatch(/instrumented kiwi/); // If we disable coverage, we get a different result. - scriptTransformer.transform( + const transformedKiwiWithoutCoverage = scriptTransformer.transform( '/fruits/kiwi.js', makeGlobalConfig({collectCoverage: false}), ); - expect(vm.Script.mock.calls[1][0]).toEqual(snapshot); - scriptTransformer.transform('/fruits/banana.js', { - // to make sure jest isn't declared twice - extraGlobals: ['Math', 'jest'], - }).script; - expect(wrap(vm.Script.mock.calls[3][0])).toMatchSnapshot(); + expect(transformedKiwiWithoutCoverage.code).not.toEqual( + transformedKiwiWithCoverage.code, + ); }); it('does not transform Node core modules', () => { @@ -272,10 +270,9 @@ describe('ScriptTransformer', () => { 'fs', {isCoreModule: true}, fsSourceCode, - ).script; + ); - expect(response instanceof vm.Script).toBe(true); - expect(vm.Script.mock.calls[0][0]).toContain(fsSourceCode); + expect(response.code).toEqual(fsSourceCode); // Native files should never be transformed. expect(shouldInstrument).toHaveBeenCalledTimes(0); @@ -358,15 +355,15 @@ describe('ScriptTransformer', () => { it('uses the supplied preprocessor', () => { config = {...config, transform: [['^.+\\.js$', 'test_preprocessor']]}; const scriptTransformer = new ScriptTransformer(config); - scriptTransformer.transform('/fruits/banana.js', {}); + const res1 = scriptTransformer.transform('/fruits/banana.js', {}); expect(require('test_preprocessor').getCacheKey).toBeCalled(); - expect(wrap(vm.Script.mock.calls[0][0])).toMatchSnapshot(); + expect(wrap(res1.code)).toMatchSnapshot(); - scriptTransformer.transform('/node_modules/react.js', {}); + const res2 = scriptTransformer.transform('/node_modules/react.js', {}); // ignores preprocessor - expect(wrap(vm.Script.mock.calls[1][0])).toMatchSnapshot(); + expect(wrap(res2.code)).toMatchSnapshot(); }); it('uses multiple preprocessors', () => { @@ -379,17 +376,17 @@ describe('ScriptTransformer', () => { }; const scriptTransformer = new ScriptTransformer(config); - scriptTransformer.transform('/fruits/banana.js', {}); - scriptTransformer.transform('/styles/App.css', {}); + const res1 = scriptTransformer.transform('/fruits/banana.js', {}); + const res2 = scriptTransformer.transform('/styles/App.css', {}); expect(require('test_preprocessor').getCacheKey).toBeCalled(); expect(require('css-preprocessor').getCacheKey).toBeCalled(); - expect(wrap(vm.Script.mock.calls[0][0])).toMatchSnapshot(); - expect(wrap(vm.Script.mock.calls[1][0])).toMatchSnapshot(); + expect(wrap(res1.code)).toMatchSnapshot(); + expect(wrap(res2.code)).toMatchSnapshot(); - scriptTransformer.transform('/node_modules/react.js', {}); + const res3 = scriptTransformer.transform('/node_modules/react.js', {}); // ignores preprocessor - expect(wrap(vm.Script.mock.calls[2][0])).toMatchSnapshot(); + expect(wrap(res3.code)).toMatchSnapshot(); }); it('writes source map if preprocessor supplies it', () => { @@ -686,34 +683,4 @@ describe('ScriptTransformer', () => { ['test_preprocessor', expect.any(Object)], ]); }); - - it('adds `moduleArguments` to transform wrapper', () => { - config = {...config, transform: [['^.+\\.js$', 'test_preprocessor']]}; - const scriptTransformer = new ScriptTransformer(config); - const {scriptContent} = scriptTransformer.transform('/fruits/banana.js', { - moduleArguments: ['foo', 'bar', 'foobar'], - }); - - expect(scriptContent.split('\n')[0]).toEqual( - '({"Object.":function(foo,bar,foobar){const TRANSFORMED = {', - ); - }); - - it('moduleArguments should be part of cache key', () => { - config = {...config, transform: [['^.+\\.js$', 'test_preprocessor']]}; - const scriptTransformer = new ScriptTransformer(config); - let result = scriptTransformer.transform('/fruits/banana.js', {}); - - expect(result.scriptContent.split('\n')[0]).toEqual( - '({"Object.":function(){const TRANSFORMED = {', - ); - - result = scriptTransformer.transform('/fruits/banana.js', { - moduleArguments: ['foo', 'bar', 'foobar'], - }); - - expect(result.scriptContent.split('\n')[0]).toEqual( - '({"Object.":function(foo,bar,foobar){const TRANSFORMED = {', - ); - }); }); diff --git a/packages/jest-transform/src/types.ts b/packages/jest-transform/src/types.ts index 71210bc61b76..064ebf380cb9 100644 --- a/packages/jest-transform/src/types.ts +++ b/packages/jest-transform/src/types.ts @@ -5,7 +5,6 @@ * LICENSE file in the root directory of this source tree. */ -import {Script} from 'vm'; import {RawSourceMap} from 'source-map'; import {Config} from '@jest/types'; @@ -18,7 +17,6 @@ export type Options = ShouldInstrumentOptions & Partial<{ isCoreModule: boolean; isInternalModule: boolean; - moduleArguments: Array; }>; // https://stackoverflow.com/a/48216010/1850276 @@ -36,8 +34,7 @@ export type TransformedSource = { }; export type TransformResult = { - script: Script; - scriptContent: string; + code: string; mapCoverage: boolean; sourceMapPath: string | null; };