Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Handle "simple" (missing origin) source mappings in minifier output #928

Closed
wants to merge 1 commit into from

Conversation

robhogan
Copy link
Contributor

Summary:
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 merges source maps to serialize minified output, 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.

Differential Revision: D43351987

@facebook-github-bot facebook-github-bot added CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported labels Feb 16, 2023
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D43351987

…acebook#928)

Summary:
Pull Request resolved: facebook#928

Fixes facebook#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: 706cf2e90cb33ebc8223b449b2c280156512c9a4
@facebook-github-bot
Copy link
Contributor

This pull request was exported from Phabricator. Differential Revision: D43351987

@facebook-github-bot
Copy link
Contributor

This pull request has been merged in 1a9c719.

@robhogan robhogan deleted the export-D43351987 branch February 17, 2023 00:29
robhogan added a commit that referenced this pull request Feb 18, 2023
…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
robhogan added a commit that referenced this pull request Feb 19, 2023
…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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. fb-exported Merged
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Metro may produce invalid source maps with Terser minification enabled.
2 participants