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

[0.73.x] Release 0.73.8 (Terser source map fixes) #930

Merged
merged 3 commits into from Feb 19, 2023
Merged

Conversation

robhogan
Copy link
Contributor

Release 0.73.8 as a hotfix to address two recent issues, coincidentally both with source maps, revealed as we've switched the default minifier to Terser.

Fixes: #927
Mitigates: terser/terser#1341

Proposed changelog

* **[Fix]**: Source maps may have invalid entries when using Terser minification. (https://github.com/facebook/metro/pull/928)
* **[Fix]**: Mitigate potential source map mismatches with concurrent transformations due to [terser#1341](https://github.com/terser/terser/issues/1341). (https://github.com/facebook/metro/pull/929)

Test plan

Generated with:

git cherry-pick 1a9c71993bce534f60a49b610e5f4b909d206551
git cherry-pick 5e27b3b8464f02a47ff6401c23cda0ec6658c9b1
node ./scripts/updateVersion.js 0.73.8

CI passes

robhogan and others added 3 commits February 18, 2023 20:18
…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
Summary:
Mitigates terser/terser#1341

Terser sets properties inlcuding `source_map` and `_destroy_ast` on the given `options.output` (or `options.format`) object, which can affect subsequent calls where we re-use the same config object.

Here we take a shallow copy of the given configuration, so `terser` doesn't mutate `metro-minify-terser`'s input.

Changelog: [Fix] Mitigate potential source map mismatches with concurrent transformations (Terser [#1341](terser/terser#1341))

Reviewed By: jacdebug, motiz88

Differential Revision: D43362977

fbshipit-source-id: 940e6209273cdd382513aea2f2a8aeca12daa2e3
@facebook-github-bot facebook-github-bot added the CLA Signed This label is managed by the Facebook bot. Authors need to sign the CLA before a PR can be reviewed. label Feb 18, 2023
Copy link
Member

@huntie huntie left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

@robhogan robhogan merged commit 65d801c into 0.73.x Feb 19, 2023
@robhogan robhogan deleted the hotfix/0.73.8 branch February 19, 2023 08:32
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.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants