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.

Sign up for GitHub

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

Jump to bottom

Metro may produce invalid source maps with Terser minification enabled. #927

Closed
robhogan opened this issue Feb 16, 2023 · 0 comments
Closed

Metro may produce invalid source maps with Terser minification enabled. #927

robhogan opened this issue Feb 16, 2023 · 0 comments

Comments

@robhogan
Copy link
Contributor

robhogan commented Feb 16, 2023 •
edited
Loading

This was originally reported against react-native-bundle-visualizer by @frankcalise in IjzerenHein/react-native-bundle-visualizer#116 (CC @frankcalise, @hurali97).

What is the current behavior?
When using metro-minify-terser (as is now default), Metro may produce source maps that contain invalid entries. This prevents source-map-explorer from working at all, and is likely to affect other tools.

If the current behavior is a bug, please provide the steps to reproduce and a minimal repository on GitHub that we can yarn install and yarn test.

git clone https://github.com/frankcalise/react-native-vanilla-reanimated
yarn
yarn run visualizer

(Pin react-native-bundle-visualizer to 3.1.2 to reproduce before the workaround (IjzerenHein/react-native-bundle-visualizer@af0289c) was released)

Or

yarn add source-map-explorer
yarn run react-native bundle --platform=ios --dev=false --entry-file=./index.js --bundle-output=terser.bundle --sourcemap-output terser.bundle.map --minify --reset-cache
yarn run source-map-explorer terser.bundle terser.bundle.map

What is the expected behavior?
A source-map derived visualisation of dependencies.

Metro 0.75.0
@robhogan robhogan mentioned this issue Feb 16, 2023
Closed
robhogan added a commit to robhogan/metro that referenced this issue Feb 16, 2023
@robhogan @facebook-github-bot
Handle "simple" (missing origin) source mappings in minifier output
66914bf 
Summary:
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 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

fbshipit-source-id: 5ba5e9edc6913d1feaf7a6b1827ad43e579fbada
robhogan added a commit to robhogan/metro that referenced this issue Feb 16, 2023
@robhogan @facebook-github-bot
Handle "simple" (missing origin) source mappings in minifier output (f…
ed4a813 
…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
robhogan added a commit that referenced this issue Feb 18, 2023
@robhogan
Handle "simple" (missing origin) source mappings in minifier output (#…
9c0229b 
…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 robhogan mentioned this issue Feb 18, 2023
Merged
robhogan added a commit that referenced this issue Feb 19, 2023
@robhogan
Handle "simple" (missing origin) source mappings in minifier output (#…
84947c7 
…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
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Handle "simple" (missing origin) source mappings in minifier output robhogan/metro
1 participant
@robhogan