From 9c0229b5c3cc8dc6f8a50c87943d5116b30d326f Mon Sep 17 00:00:00 2001 From: Rob Hogan Date: Thu, 16 Feb 2023 08:41:31 -0800 Subject: [PATCH] Handle "simple" (missing origin) source mappings in minifier output (#928) Summary: Pull Request resolved: https://github.com/facebook/metro/pull/928 Fixes https://github.com/facebook/metro/issues/927 Terser, unlike the previous default minifier UglifyES, produces source maps whose entries may contain "simple mappings" - that is entries with a generated line+col but no original location. When Metro processes minified output ahead of serialization, it doesn't correctly handle these entries. Instead it incorrectly detects them as complete mappings and produces a merged source map with `-1` origin line numbers in place of simple mappings. Some tools will reject the whole source map (which is otherwise correct) based on the appearance of a negative line number. The issue comes down to the conversion of raw mappings to `BabelSourceMapSegments` in `toBabelSegments`, which currently generates entries for simple mappings with `null` `origin` *properties*, such as: ``` { generated: { column: 2, line: 1 }, origin: { column: null, line: null }, // ... } ``` According to the Flow type for `BabelSourceMapSegments`, that's incorrect. `origin` itself is optional and should be missing - the properties of `origin` are not nullable. Based on the presence of the `origin` property in that object, `toSegmentTuple` generates a 4-tuple and we go on to add a "source mapping" rather than a "simple mapping". This fixes `toBabelSegments` to omit `origin` unless `line` and `column` are both populated. The final source map output is then correct. Changelog: [Fix] Source maps may have invalid entries when using Terser minification. Reviewed By: motiz88 Differential Revision: D43351987 fbshipit-source-id: 5cd46fdd001f3c3a3599ada44f525bec467854e1 --- .../__snapshots__/source-map-test.js.snap | 12 ---- packages/metro-source-map/src/source-map.js | 59 +++++++++++++------ 2 files changed, 42 insertions(+), 29 deletions(-) diff --git a/packages/metro-source-map/src/__tests__/__snapshots__/source-map-test.js.snap b/packages/metro-source-map/src/__tests__/__snapshots__/source-map-test.js.snap index f7ad4c3cdb..83d248696a 100644 --- a/packages/metro-source-map/src/__tests__/__snapshots__/source-map-test.js.snap +++ b/packages/metro-source-map/src/__tests__/__snapshots__/source-map-test.js.snap @@ -8,10 +8,6 @@ Array [ "line": 1, }, "name": null, - "original": Object { - "column": null, - "line": null, - }, "source": null, }, Object { @@ -56,10 +52,6 @@ Array [ "line": 12, }, "name": null, - "original": Object { - "column": null, - "line": null, - }, "source": null, }, Object { @@ -80,10 +72,6 @@ Array [ "line": 25, }, "name": null, - "original": Object { - "column": null, - "line": null, - }, "source": null, }, Object { diff --git a/packages/metro-source-map/src/source-map.js b/packages/metro-source-map/src/source-map.js index 29935b247e..890b68dc5b 100644 --- a/packages/metro-source-map/src/source-map.js +++ b/packages/metro-source-map/src/source-map.js @@ -4,7 +4,7 @@ * This source code is licensed under the MIT license found in the * LICENSE file in the root directory of this source tree. * - * @flow + * @flow strict-local * @format * @oncall react_native */ @@ -21,6 +21,7 @@ const Consumer = require('./Consumer'); const normalizeSourcePath = require('./Consumer/normalizeSourcePath'); const {generateFunctionMap} = require('./generateFunctionMap'); const Generator = require('./Generator'); +// $FlowFixMe[untyped-import] - source-map const SourceMap = require('source-map'); export type {IConsumer}; @@ -85,6 +86,15 @@ export type IndexMap = { export type MixedSourceMap = IndexMap | BasicSourceMap; +type SourceMapConsumerMapping = { + generatedLine: number, + generatedColumn: number, + originalLine: ?number, + originalColumn: ?number, + source: ?string, + name: ?string, +}; + function fromRawMappingsImpl( isBlocking: boolean, onDone: Generator => void, @@ -205,20 +215,33 @@ function toBabelSegments( ): Array { const rawMappings: Array = []; - new SourceMap.SourceMapConsumer(sourceMap).eachMapping(map => { - rawMappings.push({ - generated: { - line: map.generatedLine, - column: map.generatedColumn, - }, - original: { - line: map.originalLine, - column: map.originalColumn, - }, - source: map.source, - name: map.name, - }); - }); + new SourceMap.SourceMapConsumer(sourceMap).eachMapping( + (map: SourceMapConsumerMapping) => { + rawMappings.push( + map.originalLine == null || map.originalColumn == null + ? { + generated: { + line: map.generatedLine, + column: map.generatedColumn, + }, + source: map.source, + name: map.name, + } + : { + generated: { + line: map.generatedLine, + column: map.generatedColumn, + }, + original: { + line: map.originalLine, + column: map.originalColumn, + }, + source: map.source, + name: map.name, + }, + ); + }, + ); return rawMappings; } @@ -274,11 +297,13 @@ function addMapping( if (n === 2) { generator.addSimpleMapping(line, column); } else if (n === 4) { - const sourceMap: SourceMapping = (mapping: any); + // $FlowIssue[invalid-tuple-arity] Arity is ensured by conidition on length + const sourceMap: SourceMapping = mapping; generator.addSourceMapping(line, column, sourceMap[2], sourceMap[3]); } else if (n === 5) { - const sourceMap: SourceMappingWithName = (mapping: any); + // $FlowIssue[invalid-tuple-arity] Arity is ensured by conidition on length + const sourceMap: SourceMappingWithName = mapping; generator.addNamedSourceMapping( line,