Skip to content

Commit

Permalink
fix(metro-config): Replace source-map snapshots for metro-transformer…
Browse files Browse the repository at this point in the history
…-worker with mappings tests (expo#25872)

# Why

- Closes ENG-10873
- This reverts the lock on `@babel/generator@~7.20.5` to
`@babel/generator@^7.20.5` to allow upgrades past `7.21.0`
- This was previously put in place in expo#25833 and this is a follow-up PR
- `@babel/generator@7.21.0` shipped a fix that added names of “friendly
call frames” to its source maps
(babel/babel#15022). This caused the source maps
to change to include internal names for in-scope identifiers such as
`globalThis` and `_interopRequireDefault`
- This was a completely backwards-compatible change for us, but this was
obscured by source map output being a little hard to read

When applied, this PR replaces our source map snapshots with tests that
check exact identifiers in the input and output against their mappings
and names.
This should make it easier to identify changes to the source maps and
allow us to debug them more easily in the future.

A note has been added to any assertions in the tests that break when
downgrading below `7.21.0`, but for now, I haven't added any additional
tests that account for this, since I'm not expecting us to downgrade to
the lower version again, and to instead keep all `@babel/*` packages in
`@expo/metro-config` upgraded in lockstep.

# How

- All source-map snapshots for `metro-transformer-worker` have been
deleted
- `@jridgewell/trace-mapping` has been added to parse the worker’s
source maps (with a small converter function)
- This is an alternative to the ubiquitous `source-map` package, and
it’s also used in Babel. It avoids parsing source maps in wasm.
- When adding `source-map` instead, we’d have to work around our virtual
FS mock
- All source-map snapshots have been replaced with checks for both input
and output identifiers against the actual source-maps
- Additional tests have been added to check for internal globals, which
previously caused the source maps to break

# Test Plan

**Note:** When testing this, placing the pinned version back to
`@babel/generator@~7.20.5` causes some `name` properties to be missing
in the source-map output for injected globals, as expected.

# Checklist

- [ ] Documentation is up to date to reflect these changes (eg:
https://docs.expo.dev and README.md).
- [ ] Conforms with the [Documentation Writing Style
Guide](https://github.com/expo/expo/blob/main/guides/Expo%20Documentation%20Writing%20Style%20Guide.md)
- [ ] This diff will work correctly for `npx expo prebuild` & EAS Build
(eg: updated a module plugin).
  • Loading branch information
kitten committed Dec 14, 2023
1 parent 9a237bf commit 199ade3
Show file tree
Hide file tree
Showing 31 changed files with 226 additions and 300 deletions.
2 changes: 2 additions & 0 deletions packages/@expo/metro-config/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@

### 🐛 Bug fixes

- Unpin minor upgrades for `@babel/generator` to `^7.20.5`. ([#25872](https://github.com/expo/expo/pull/25872) by [@kitten](https://github.com/kitten))

### 💡 Others

## 0.16.1 — 2023-12-13
Expand Down
2 changes: 1 addition & 1 deletion packages/@expo/metro-config/build/ExpoMetroConfig.js.map

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/@expo/metro-config/build/babel-transformer.js.map

Large diffs are not rendered by default.

2 changes: 1 addition & 1 deletion packages/@expo/metro-config/build/customizeFrame.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

2 changes: 1 addition & 1 deletion packages/@expo/metro-config/build/env.js.map

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 199ade3

Please sign in to comment.