diff --git a/CHANGELOG.md b/CHANGELOG.md index 9be830431972..c318022f32db 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - `[jest-runtime]` Support importing CJS from ESM using `import` statements ([#9850](https://github.com/facebook/jest/pull/9850)) - `[jest-runtime]` Support importing parallel dynamic `import`s ([#9858](https://github.com/facebook/jest/pull/9858)) +- `[jest-transform]` Improve source map handling when instrumenting transformed code ([#9811](https://github.com/facebook/jest/pull/9811)) ### Chore & Maintenance diff --git a/e2e/__tests__/__snapshots__/stackTraceSourceMapsWithCoverage.test.ts.snap b/e2e/__tests__/__snapshots__/stackTraceSourceMapsWithCoverage.test.ts.snap new file mode 100644 index 000000000000..fa3a2f7871ab --- /dev/null +++ b/e2e/__tests__/__snapshots__/stackTraceSourceMapsWithCoverage.test.ts.snap @@ -0,0 +1,20 @@ +// Jest Snapshot v1, https://goo.gl/fbAQLP + +exports[`processes stack traces and code frames with source maps with coverage 1`] = ` +FAIL __tests__/fails.ts + ✕ fails + + ● fails + + This did not work! + + 11 | + 12 | export function error() { + > 13 | throw new Error('This did not work!'); + | ^ + 14 | } + 15 | + + at Object.error (lib.ts:13:9) + at Object. (__tests__/fails.ts:10:3) +`; diff --git a/e2e/__tests__/stackTraceSourceMapsWithCoverage.test.ts b/e2e/__tests__/stackTraceSourceMapsWithCoverage.test.ts new file mode 100644 index 000000000000..4ac65f38e890 --- /dev/null +++ b/e2e/__tests__/stackTraceSourceMapsWithCoverage.test.ts @@ -0,0 +1,22 @@ +/** + * 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 * as path from 'path'; +import wrap from 'jest-snapshot-serializer-raw'; +import {extractSummary, run} from '../Utils'; +import runJest from '../runJest'; + +it('processes stack traces and code frames with source maps with coverage', () => { + const dir = path.resolve( + __dirname, + '../stack-trace-source-maps-with-coverage', + ); + run('yarn', dir); + const {stderr} = runJest(dir, ['--no-cache', '--coverage']); + + // Should report an error at source line 13 in lib.ts at line 10 of the test + expect(wrap(extractSummary(stderr).rest)).toMatchSnapshot(); +}); diff --git a/e2e/stack-trace-source-maps-with-coverage/__tests__/fails.ts b/e2e/stack-trace-source-maps-with-coverage/__tests__/fails.ts new file mode 100644 index 000000000000..583175c89ed7 --- /dev/null +++ b/e2e/stack-trace-source-maps-with-coverage/__tests__/fails.ts @@ -0,0 +1,11 @@ +/** + * 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 {error} from '../lib'; + +it('fails', () => { + error(); +}); diff --git a/e2e/stack-trace-source-maps-with-coverage/lib.ts b/e2e/stack-trace-source-maps-with-coverage/lib.ts new file mode 100644 index 000000000000..9fffd6978ab1 --- /dev/null +++ b/e2e/stack-trace-source-maps-with-coverage/lib.ts @@ -0,0 +1,14 @@ +/** + * 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. + */ +interface NotUsedButTakesUpLines { + a: number; + b: string; +} + +export function error() { + throw new Error('This did not work!'); +} diff --git a/e2e/stack-trace-source-maps-with-coverage/package.json b/e2e/stack-trace-source-maps-with-coverage/package.json new file mode 100644 index 000000000000..f8fbad2d8bca --- /dev/null +++ b/e2e/stack-trace-source-maps-with-coverage/package.json @@ -0,0 +1,13 @@ +{ + "jest": { + "rootDir": "./", + "transform": { + "^.+\\.(ts)$": "/preprocessor.js" + }, + "testEnvironment": "node", + "testRegex": "fails" + }, + "dependencies": { + "typescript": "^3.7.4" + } +} diff --git a/e2e/stack-trace-source-maps-with-coverage/preprocessor.js b/e2e/stack-trace-source-maps-with-coverage/preprocessor.js new file mode 100644 index 000000000000..133d42ec44a2 --- /dev/null +++ b/e2e/stack-trace-source-maps-with-coverage/preprocessor.js @@ -0,0 +1,21 @@ +/** + * 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. + */ + +const tsc = require('typescript'); + +module.exports = { + process(src, path) { + return tsc.transpileModule(src, { + compilerOptions: { + inlineSourceMap: true, + module: tsc.ModuleKind.CommonJS, + target: 'es5', + }, + fileName: path, + }).outputText; + }, +}; diff --git a/e2e/stack-trace-source-maps-with-coverage/yarn.lock b/e2e/stack-trace-source-maps-with-coverage/yarn.lock new file mode 100644 index 000000000000..e9f19f98faf0 --- /dev/null +++ b/e2e/stack-trace-source-maps-with-coverage/yarn.lock @@ -0,0 +1,8 @@ +# THIS IS AN AUTOGENERATED FILE. DO NOT EDIT THIS FILE DIRECTLY. +# yarn lockfile v1 + + +typescript@^3.7.4: + version "3.8.3" + resolved "https://registry.yarnpkg.com/typescript/-/typescript-3.8.3.tgz#409eb8544ea0335711205869ec458ab109ee1061" + integrity sha512-MYlEfn5VrLNsgudQTVJeNaQFUAI7DkhnOjdpAp4T+ku1TfQClewlbSuTVHiA+8skNBgaf02TL/kLOvig4y3G8w== diff --git a/packages/jest-reporters/src/coverage_reporter.ts b/packages/jest-reporters/src/coverage_reporter.ts index b769743e2ff6..4ec8d75961ac 100644 --- a/packages/jest-reporters/src/coverage_reporter.ts +++ b/packages/jest-reporters/src/coverage_reporter.ts @@ -71,28 +71,6 @@ export default class CoverageReporter extends BaseReporter { if (testResult.coverage) { this._coverageMap.merge(testResult.coverage); } - - const sourceMaps = testResult.sourceMaps; - if (sourceMaps) { - Object.keys(sourceMaps).forEach(sourcePath => { - let inputSourceMap: RawSourceMap | undefined; - try { - const coverage: istanbulCoverage.FileCoverage = this._coverageMap.fileCoverageFor( - sourcePath, - ); - inputSourceMap = (coverage.toJSON() as any).inputSourceMap; - } finally { - if (inputSourceMap) { - this._sourceMapStore.registerMap(sourcePath, inputSourceMap); - } else { - this._sourceMapStore.registerURL( - sourcePath, - sourceMaps[sourcePath], - ); - } - } - }); - } } async onRunComplete( @@ -215,13 +193,6 @@ export default class CoverageReporter extends BaseReporter { ]); } else { this._coverageMap.addFileCoverage(result.coverage); - - if (result.sourceMapPath) { - this._sourceMapStore.registerURL( - filename, - result.sourceMapPath, - ); - } } } } catch (error) { diff --git a/packages/jest-reporters/src/generateEmptyCoverage.ts b/packages/jest-reporters/src/generateEmptyCoverage.ts index 62ab4c023f2c..b6c270060fdd 100644 --- a/packages/jest-reporters/src/generateEmptyCoverage.ts +++ b/packages/jest-reporters/src/generateEmptyCoverage.ts @@ -18,7 +18,6 @@ export type CoverageWorkerResult = | { kind: 'BabelCoverage'; coverage: FileCoverage; - sourceMapPath?: string | null; } | { kind: 'V8Coverage'; @@ -66,9 +65,11 @@ export default function ( } // Transform file with instrumentation to make sure initial coverage data is well mapped to original code. - const {code, mapCoverage, sourceMapPath} = new ScriptTransformer( - config, - ).transformSource(filename, source, true); + const {code} = new ScriptTransformer(config).transformSource( + filename, + source, + true, + ); // TODO: consider passing AST const extracted = readInitialCoverage(code); // Check extracted initial coverage is not null, this can happen when using /* istanbul ignore file */ @@ -76,7 +77,6 @@ export default function ( coverageWorkerResult = { coverage: createFileCoverage(extracted.coverageData), kind: 'BabelCoverage', - sourceMapPath: mapCoverage ? sourceMapPath : null, }; } } diff --git a/packages/jest-runner/src/runTest.ts b/packages/jest-runner/src/runTest.ts index 36754d3f086e..fb3e93ea0eda 100644 --- a/packages/jest-runner/src/runTest.ts +++ b/packages/jest-runner/src/runTest.ts @@ -275,7 +275,6 @@ async function runTestInternal( const coverageKeys = Object.keys(coverage); if (coverageKeys.length) { result.coverage = coverage; - result.sourceMaps = runtime.getSourceMapInfo(new Set(coverageKeys)); } } diff --git a/packages/jest-runtime/src/index.ts b/packages/jest-runtime/src/index.ts index 8df6447e8b00..363c18ba846f 100644 --- a/packages/jest-runtime/src/index.ts +++ b/packages/jest-runtime/src/index.ts @@ -140,7 +140,6 @@ class Runtime { private _isolatedModuleRegistry: ModuleRegistry | null; private _moduleRegistry: ModuleRegistry; private _esmoduleRegistry: Map>; - private _needsCoverageMapped: Set; private _resolver: Resolver; private _shouldAutoMock: boolean; private _shouldMockModuleCache: BooleanObject; @@ -186,7 +185,6 @@ class Runtime { this._isolatedMockRegistry = null; this._moduleRegistry = new Map(); this._esmoduleRegistry = new Map(); - this._needsCoverageMapped = new Set(); this._resolver = resolver; this._scriptTransformer = new ScriptTransformer(config); this._shouldAutoMock = config.automock; @@ -794,20 +792,9 @@ class Runtime { }); } - getSourceMapInfo(coveredFiles: Set): Record { - return Object.keys(this._sourceMapRegistry).reduce>( - (result, sourcePath) => { - if ( - coveredFiles.has(sourcePath) && - this._needsCoverageMapped.has(sourcePath) && - fs.existsSync(this._sourceMapRegistry[sourcePath]) - ) { - result[sourcePath] = this._sourceMapRegistry[sourcePath]; - } - return result; - }, - {}, - ); + // TODO - remove in Jest 26 + getSourceMapInfo(_coveredFiles: Set): Record { + return {}; } getSourceMaps(): SourceMapRegistry { @@ -1056,9 +1043,6 @@ class Runtime { if (transformedFile.sourceMapPath) { this._sourceMapRegistry[filename] = transformedFile.sourceMapPath; - if (transformedFile.mapCoverage) { - this._needsCoverageMapped.add(filename); - } } return transformedFile; } diff --git a/packages/jest-test-result/src/types.ts b/packages/jest-test-result/src/types.ts index 304b358c143b..ffd9c23735b6 100644 --- a/packages/jest-test-result/src/types.ts +++ b/packages/jest-test-result/src/types.ts @@ -104,6 +104,7 @@ export type TestResult = { unmatched: number; updated: number; }; + // TODO - Remove in Jest 26 sourceMaps?: { [sourcePath: string]: string; }; diff --git a/packages/jest-transform/src/ScriptTransformer.ts b/packages/jest-transform/src/ScriptTransformer.ts index 1ad4109cd83a..46ae3afb049f 100644 --- a/packages/jest-transform/src/ScriptTransformer.ts +++ b/packages/jest-transform/src/ScriptTransformer.ts @@ -205,11 +205,15 @@ export default class ScriptTransformer { private _instrumentFile( filename: Config.Path, - content: string, + input: TransformedSource, supportsDynamicImport: boolean, supportsStaticESM: boolean, - ): string { - const result = babelTransform(content, { + canMapToInput: boolean, + ): TransformedSource { + const inputCode = typeof input === 'string' ? input : input.code; + const inputMap = typeof input === 'string' ? null : input.map; + + const result = babelTransform(inputCode, { auxiliaryCommentBefore: ' istanbul ignore next ', babelrc: false, caller: { @@ -228,21 +232,19 @@ export default class ScriptTransformer { cwd: this._config.rootDir, exclude: [], extension: false, + inputSourceMap: inputMap, useInlineSourceMaps: false, }, ], ], + sourceMaps: canMapToInput ? 'both' : false, }); - if (result) { - const {code} = result; - - if (code) { - return code; - } + if (result && result.code) { + return result as TransformResult; } - return content; + return input; } private _getRealPath(filepath: Config.Path): Config.Path { @@ -287,10 +289,6 @@ export default class ScriptTransformer { const transformWillInstrument = shouldCallTransform && transform && transform.canInstrument; - // If we handle the coverage instrumentation, we should try to map code - // coverage against original source with any provided source map - const mapCoverage = instrument && !transformWillInstrument; - if (code) { // This is broken: we return the code, and a path for the source map // directly from the cache. But, nothing ensures the source map actually @@ -298,7 +296,6 @@ export default class ScriptTransformer { // two separate processes write concurrently to the same cache files. return { code, - mapCoverage, originalCode: content, sourceMapPath, }; @@ -333,9 +330,8 @@ export default class ScriptTransformer { //Could be a potential freeze here. //See: https://github.com/facebook/jest/pull/5177#discussion_r158883570 const inlineSourceMap = sourcemapFromSource(transformed.code); - if (inlineSourceMap) { - transformed.map = inlineSourceMap.toJSON(); + transformed.map = inlineSourceMap.toObject(); } } catch (e) { const transformPath = this._getTransformPath(filename); @@ -347,22 +343,39 @@ export default class ScriptTransformer { } } + // Apply instrumentation to the code if necessary, keeping the instrumented code and new map + let map = transformed.map; if (!transformWillInstrument && instrument) { - code = this._instrumentFile( + /** + * We can map the original source code to the instrumented code ONLY if + * - the process of transforming the code produced a source map e.g. ts-jest + * - we did not transform the source code + * + * Otherwise we cannot make any statements about how the instrumented code corresponds to the original code, + * and we should NOT emit any source maps + * + */ + const shouldEmitSourceMaps = + (transform != null && map != null) || transform == null; + + const instrumented = this._instrumentFile( filename, - transformed.code, + transformed, supportsDynamicImport, supportsStaticESM, + shouldEmitSourceMaps, ); + + code = + typeof instrumented === 'string' ? instrumented : instrumented.code; + map = typeof instrumented === 'string' ? null : instrumented.map; } else { code = transformed.code; } - if (transformed.map) { + if (map) { const sourceMapContent = - typeof transformed.map === 'string' - ? transformed.map - : JSON.stringify(transformed.map); + typeof map === 'string' ? map : JSON.stringify(map); writeCacheFile(sourceMapPath, sourceMapContent); } else { sourceMapPath = null; @@ -372,7 +385,6 @@ export default class ScriptTransformer { return { code, - mapCoverage, originalCode: content, sourceMapPath, }; @@ -396,7 +408,6 @@ export default class ScriptTransformer { let code = content; let sourceMapPath: string | null = null; - let mapCoverage = false; const willTransform = !isInternalModule && @@ -415,12 +426,10 @@ export default class ScriptTransformer { code = transformedSource.code; sourceMapPath = transformedSource.sourceMapPath; - mapCoverage = transformedSource.mapCoverage; } return { code, - mapCoverage, originalCode: content, sourceMapPath, }; 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 19c563f8e376..13584d3f6693 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 @@ -80,7 +80,7 @@ exports[`ScriptTransformer transforms a file properly 1`] = ` /* istanbul ignore next */ function cov_25u22311x4() { var path = "/fruits/banana.js"; - var hash = "4be0f6184160be573fc43f7c2a5877c28b7ce249"; + var hash = "3f8e915bed83285455a8a16aa04dc0cf5242d755"; var global = new Function("return this")(); var gcv = "__coverage__"; var coverageData = { @@ -104,8 +104,9 @@ function cov_25u22311x4() { }, f: {}, b: {}, + inputSourceMap: null, _coverageSchema: "1a1c01bbd47fc00a2c39e90264f33305004495a9", - hash: "4be0f6184160be573fc43f7c2a5877c28b7ce249" + hash: "3f8e915bed83285455a8a16aa04dc0cf5242d755" }; var coverage = global[gcv] || (global[gcv] = {}); @@ -125,13 +126,14 @@ function cov_25u22311x4() { cov_25u22311x4(); cov_25u22311x4().s[0]++; module.exports = "banana"; +//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbImJhbmFuYS5qcyJdLCJuYW1lcyI6WyJtb2R1bGUiLCJleHBvcnRzIl0sIm1hcHBpbmdzIjoiOzs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7QUFBQUEsTUFBTSxDQUFDQyxPQUFQLEdBQWlCLFFBQWpCIiwic291cmNlc0NvbnRlbnQiOlsibW9kdWxlLmV4cG9ydHMgPSBcImJhbmFuYVwiOyJdfQ== `; exports[`ScriptTransformer transforms a file properly 2`] = ` /* istanbul ignore next */ function cov_23yvu8etmu() { var path = "/fruits/kiwi.js"; - var hash = "7705dd5fcfbc884dcea7062944cfb8cc5d141d1a"; + var hash = "8b5afd38d79008f13ebc229b89ef82b12ee9447a"; var global = new Function("return this")(); var gcv = "__coverage__"; var coverageData = { @@ -193,8 +195,9 @@ function cov_23yvu8etmu() { "0": 0 }, b: {}, + inputSourceMap: null, _coverageSchema: "1a1c01bbd47fc00a2c39e90264f33305004495a9", - hash: "7705dd5fcfbc884dcea7062944cfb8cc5d141d1a" + hash: "8b5afd38d79008f13ebc229b89ef82b12ee9447a" }; var coverage = global[gcv] || (global[gcv] = {}); @@ -220,6 +223,7 @@ module.exports = () => { cov_23yvu8etmu().s[1]++; return "kiwi"; }; +//# sourceMappingURL=data:application/json;charset=utf-8;base64,eyJ2ZXJzaW9uIjozLCJzb3VyY2VzIjpbImtpd2kuanMiXSwibmFtZXMiOlsibW9kdWxlIiwiZXhwb3J0cyJdLCJtYXBwaW5ncyI6Ijs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7Ozs7O0FBQUFBLE1BQU0sQ0FBQ0MsT0FBUCxHQUFpQixNQUFNO0FBQUE7QUFBQTtBQUFBO0FBQUE7QUFBTSxDQUE3QiIsInNvdXJjZXNDb250ZW50IjpbIm1vZHVsZS5leHBvcnRzID0gKCkgPT4gXCJraXdpXCI7Il19 `; exports[`ScriptTransformer uses multiple preprocessors 1`] = ` diff --git a/packages/jest-transform/src/__tests__/script_transformer.test.js b/packages/jest-transform/src/__tests__/script_transformer.test.js index 9069861c2420..b6570ee1c404 100644 --- a/packages/jest-transform/src/__tests__/script_transformer.test.js +++ b/packages/jest-transform/src/__tests__/script_transformer.test.js @@ -535,6 +535,90 @@ describe('ScriptTransformer', () => { expect(writeFileAtomic.sync).toHaveBeenCalledTimes(1); }); + it('should write a source map for the instrumented file when transformed', () => { + const transformerConfig = { + ...config, + transform: [['^.+\\.js$', 'preprocessor-with-sourcemaps']], + }; + const scriptTransformer = new ScriptTransformer(transformerConfig); + + const map = { + mappings: ';AAAA', + version: 3, + }; + + // A map from the original source to the instrumented output + /* eslint-disable sort-keys */ + const instrumentedCodeMap = { + version: 3, + sources: ['banana.js'], + names: ['content'], + mappings: ';;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;AAAAA,OAAO', + sourcesContent: ['content'], + }; + /* eslint-enable */ + + require('preprocessor-with-sourcemaps').process.mockReturnValue({ + code: 'content', + map, + }); + + const result = scriptTransformer.transform( + '/fruits/banana.js', + makeGlobalConfig({ + collectCoverage: true, + }), + ); + expect(result.sourceMapPath).toEqual(expect.any(String)); + expect(writeFileAtomic.sync).toBeCalledTimes(2); + expect(writeFileAtomic.sync).toBeCalledWith( + result.sourceMapPath, + JSON.stringify(instrumentedCodeMap), + expect.anything(), + ); + + // Inline source map allows debugging of original source when running instrumented code + expect(result.code).toContain('//# sourceMappingURL'); + }); + + it('should write a source map for the instrumented file when not transformed', () => { + const scriptTransformer = new ScriptTransformer(config); + + // A map from the original source to the instrumented output + /* eslint-disable sort-keys */ + const instrumentedCodeMap = { + version: 3, + sources: ['banana.js'], + names: ['module', 'exports'], + mappings: + ';;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;AAAAA,MAAM,CAACC,OAAP,GAAiB,QAAjB', + sourcesContent: ['module.exports = "banana";'], + }; + /* eslint-enable */ + + require('preprocessor-with-sourcemaps').process.mockReturnValue({ + code: 'content', + map: null, + }); + + const result = scriptTransformer.transform( + '/fruits/banana.js', + makeGlobalConfig({ + collectCoverage: true, + }), + ); + expect(result.sourceMapPath).toEqual(expect.any(String)); + expect(writeFileAtomic.sync).toBeCalledTimes(2); + expect(writeFileAtomic.sync).toBeCalledWith( + result.sourceMapPath, + JSON.stringify(instrumentedCodeMap), + expect.anything(), + ); + + // Inline source map allows debugging of original source when running instrumented code + expect(result.code).toContain('//# sourceMappingURL'); + }); + it('passes expected transform options to getCacheKey', () => { config = {...config, transform: [['^.+\\.js$', 'test_preprocessor']]}; const scriptTransformer = new ScriptTransformer(config); diff --git a/packages/jest-transform/src/types.ts b/packages/jest-transform/src/types.ts index 30075298bc26..138bab7f5406 100644 --- a/packages/jest-transform/src/types.ts +++ b/packages/jest-transform/src/types.ts @@ -34,6 +34,7 @@ interface FixedRawSourceMap extends SourceMapWithVersion { version: number; } +// TODO: For Jest 26 normalize this (always structured data, never a string) export type TransformedSource = | {code: string; map?: FixedRawSourceMap | string | null} | string; diff --git a/packages/jest-types/src/Transform.ts b/packages/jest-types/src/Transform.ts index 9c59e7b34489..2e60af1e489d 100644 --- 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; + mapCoverage?: boolean; // TODO - Remove in Jest 26 sourceMapPath: string | null; };