Terminate module source maps with a null mapping

Summary:
Changelog:
* **[Fix]:** Guard against invalid symbolication in Chrome DevTools by terminating each module's source map with a null mapping.

## The bug
Metro's bundles include a bit of top-level generated code at the end, which today has no source mappings whatsoever. In Chrome DevTools, this results in stack traces that look corrupt if they include any frames in that region of the bundle: this is because Chrome falls back to the nearest preceding mapping, which happens to come from a different module entirely.

## The fix
Here, we add code to `metro-transform-worker` to ensure that we always emit an extra mapping (with no embedded source location) to mark the end of the last line of each module. When concatenated into a bundle, this causes Chrome DevTools to return this null mapping for all generated locations in the bottom of the bundle. This is strictly better than returning a random incorrect source file, regardless of what the "ideal" way to represent that code might be.

NOTE:  The Metro server's source map infra is hacky and outdated, preventing us from using safe abstractions like `BundleBuilder` that have this type of automatic termination built-in (as well as from emitting any actual mappings for the trailing code in the bundle). We'll likely want to follow up with a deeper refactor there.

Reviewed By: GijsWeterings

Differential Revision: D55066677

fbshipit-source-id: 879524b0b49cfb1c137daac2083aeafe24f0e193

packages/metro-transform-worker/src/__tests__/__snapshots__/index-test.js.snap

@@ -123,6 +123,10 @@ Array [
     4,
     23,
   ],
+  Array [
+    9,
+    3,
+  ],
 ]
 `;
 
@@ -175,6 +179,10 @@ Array [
     1,
     15,
   ],
+  Array [
+    3,
+    3,
+  ],
 ]
 `;
 
@@ -227,6 +235,10 @@ Array [
     1,
     25,
   ],
+  Array [
+    3,
+    140,
+  ],
 ]
 `;
 
@@ -315,6 +327,10 @@ Array [
     1,
     20,
   ],
+  Array [
+    5,
+    3,
+  ],
 ]
 `;
 

packages/metro-transform-worker/src/__tests__/index-test.js

@@ -192,7 +192,7 @@ it('transforms an es module with asyncToGenerator', async () => {
 
   expect(result.output[0].type).toBe('js/module');
   expect(result.output[0].data.code).toMatchSnapshot();
-  expect(result.output[0].data.map).toHaveLength(13);
+  expect(result.output[0].data.map).toHaveLength(14);
   expect(result.output[0].data.functionMap).toMatchSnapshot();
   expect(result.dependencies).toEqual([
     {

packages/metro-transform-worker/src/index.js

@@ -44,7 +44,6 @@ const {
   toSegmentTuple,
 } = require('metro-source-map');
 const metroTransformPlugins = require('metro-transform-plugins');
-const countLines = require('metro/src/lib/countLines');
 const collectDependencies = require('metro/src/ModuleGraph/worker/collectDependencies');
 const {
   InvalidRequireCallError: InternalInvalidRequireCallError,
@@ -441,11 +440,14 @@ async function transformJS(
     ));
   }
 
+  let lineCount;
+  ({lineCount, map} = countLinesAndTerminateMap(code, map));
+
   const output: Array<JsOutput> = [
     {
       data: {
         code,
-        lineCount: countLines(code),
+        lineCount,
         map,
         functionMap: file.functionMap,
       },
@@ -552,9 +554,11 @@ async function transformJSON(
     jsType = 'js/module';
   }
 
+  let lineCount;
+  ({lineCount, map} = countLinesAndTerminateMap(code, map));
   const output: Array<JsOutput> = [
     {
-      data: {code, lineCount: countLines(code), map, functionMap: null},
+      data: {code, lineCount, map, functionMap: null},
       type: jsType,
     },
   ];
@@ -652,6 +656,7 @@ module.exports = {
     const {babelTransformerPath, minifierPath, ...remainingConfig} = config;
 
     const filesKey = getCacheKey([
+      __filename,
       require.resolve(babelTransformerPath),
       require.resolve(minifierPath),
       require.resolve('./utils/getMinifier'),
@@ -670,3 +675,42 @@ module.exports = {
     ].join('$');
   },
 };
+
+function countLinesAndTerminateMap(
+  code: string,
+  map: $ReadOnlyArray<MetroSourceMapSegmentTuple>,
+): {
+  lineCount: number,
+  map: Array<MetroSourceMapSegmentTuple>,
+} {
+  const NEWLINE = /\r\n?|\n|\u2028|\u2029/g;
+  let lineCount = 1;
+  let lastLineStart = 0;
+
+  // Count lines and keep track of where the last line starts
+  for (const match of code.matchAll(NEWLINE)) {
+    lineCount++;
+    lastLineStart = match.index + match[0].length;
+  }
+  const lastLineLength = code.length - lastLineStart;
+  const lastLineIndex1Based = lineCount;
+  const lastLineNextColumn0Based = lastLineLength;
+
+  // If there isn't a mapping at one-past-the-last column of the last line,
+  // add one that maps to nothing. This ensures out-of-bounds lookups hit the
+  // null mapping rather than aliasing to whichever mapping happens to be last.
+  // ASSUMPTION: Mappings are generated in order of increasing line and column.
+  const lastMapping = map[map.length - 1];
+  const terminatingMapping = [lastLineIndex1Based, lastLineNextColumn0Based];
+  if (
+    !lastMapping ||
+    lastMapping[0] !== terminatingMapping[0] ||
+    lastMapping[1] !== terminatingMapping[1]
+  ) {
+    return {
+      lineCount,
+      map: map.concat([terminatingMapping]),
+    };
+  }
+  return {lineCount, map: [...map]};
+}

packages/metro/src/integration_tests/__tests__/__snapshots__/buildGraph-test.js.snap

@@ -389,6 +389,10 @@ Array [
           20,
           40,
         ],
+        Array [
+          24,
+          3,
+        ],
       ],
     },
     "type": "js/module",