Improve source map handling when instrumenting transformed code (#9811)

jestjs · Apr 23, 2020 · 2e45410 · 2e45410
1 parent 70bf93c
commit 2e45410
Show file tree

Hide file tree

Showing 18 changed files with 249 additions and 86 deletions.
diff --git 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
@@ -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.<anonymous> (__tests__/fails.ts:10:3)
+`;
diff --git a/e2e/__tests__/stackTraceSourceMapsWithCoverage.test.ts 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
@@ -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
@@ -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
@@ -0,0 +1,13 @@
+{
+  "jest": {
+    "rootDir": "./",
+    "transform": {
+      "^.+\\.(ts)$": "<rootDir>/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
@@ -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
@@ -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
@@ -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
@@ -18,7 +18,6 @@ export type CoverageWorkerResult =
   | {
       kind: 'BabelCoverage';
       coverage: FileCoverage;
-      sourceMapPath?: string | null;
     }
   | {
       kind: 'V8Coverage';
@@ -66,17 +65,18 @@ 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 */
     if (extracted) {
       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
@@ -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
@@ -140,7 +140,6 @@ class Runtime {
   private _isolatedModuleRegistry: ModuleRegistry | null;
   private _moduleRegistry: ModuleRegistry;
   private _esmoduleRegistry: Map<string, Promise<VMModule>>;
-  private _needsCoverageMapped: Set<string>;
   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<string>): Record<string, string> {
-    return Object.keys(this._sourceMapRegistry).reduce<Record<string, string>>(
-      (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<string>): Record<string, string> {
+    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
@@ -104,6 +104,7 @@ export type TestResult = {
     unmatched: number;
     updated: number;
   };
+  // TODO - Remove in Jest 26
   sourceMaps?: {
     [sourcePath: string]: string;
   };