Improve source map handling when instrumenting transformed code (jestjs#5739)

Author: Matthew Preble

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,18 +289,14 @@ 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
       // matches that source code. They could have gotten out-of-sync in case
       // two separate processes write concurrently to the same cache files.
       return {
         code,
-        mapCoverage,
+        mapCoverage: false,
         originalCode: content,
         sourceMapPath,
       };
@@ -333,9 +331,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 +344,38 @@ 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 && !!map) || !transform;
+
+      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,7 @@ export default class ScriptTransformer {
 
     return {
       code,
-      mapCoverage,
+      mapCoverage: false,
       originalCode: content,
       sourceMapPath,
     };
@@ -396,7 +409,6 @@ export default class ScriptTransformer {
 
     let code = content;
     let sourceMapPath: string | null = null;
-    let mapCoverage = false;
 
     const willTransform =
       !isInternalModule &&
@@ -415,12 +427,11 @@ export default class ScriptTransformer {
 
         code = transformedSource.code;
         sourceMapPath = transformedSource.sourceMapPath;
-        mapCoverage = transformedSource.mapCoverage;
       }
 
       return {
         code,
-        mapCoverage,
+        mapCoverage: false,
         originalCode: content,
         sourceMapPath,
       };

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`] = `

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);