Skip to content

Commit

Permalink
Handle "simple" (missing origin) source mappings in minifier output (#…
Browse files Browse the repository at this point in the history
…928)

Summary:
Pull Request resolved: #928

Fixes #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
  • Loading branch information
robhogan committed Feb 19, 2023
1 parent fd11374 commit 84947c7
Show file tree
Hide file tree
Showing 2 changed files with 42 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -8,10 +8,6 @@ Array [
"line": 1,
},
"name": null,
"original": Object {
"column": null,
"line": null,
},
"source": null,
},
Object {
Expand Down Expand Up @@ -56,10 +52,6 @@ Array [
"line": 12,
},
"name": null,
"original": Object {
"column": null,
"line": null,
},
"source": null,
},
Object {
Expand All @@ -80,10 +72,6 @@ Array [
"line": 25,
},
"name": null,
"original": Object {
"column": null,
"line": null,
},
"source": null,
},
Object {
Expand Down
59 changes: 42 additions & 17 deletions packages/metro-source-map/src/source-map.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
*/
Expand All @@ -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};
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -205,20 +215,33 @@ function toBabelSegments(
): Array<BabelSourceMapSegment> {
const rawMappings: Array<BabelSourceMapSegment> = [];

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;
}
Expand Down Expand Up @@ -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,
Expand Down

0 comments on commit 84947c7

Please sign in to comment.