Skip to content

composeSourceMaps: Keep only the last mapping at a repeated column - #2095

Open
robhogan wants to merge 1 commit into
mainfrom
pr2095
Open

robhogan wants to merge 1 commit into
mainfrom
pr2095

Conversation

@robhogan

@robhogan robhogan commented Oct 23, 2026 •

Copy link
Copy Markdown
Collaborator

Undoes a behaviour change from #1981 that, while not a technical regression, in practice reduced effectiveness of symbolication.

hermesc emits several mappings at some bytecode offsets, eg at a function's entry: its declaration, then its first instruction. #1981 made composeSourceMaps keep all of them, but:

  • Only the last covers any code, since a mapping's segment runs to the next one. The others can't be the answer to a lookup by position.
  • Consumers that take the first mapping at a column, including metro-symbolicate and trace-mapping, now resolve these offsets to the wrong place, or to nothing.

For example, a frame in return {run: cb => cb()}; in RN's console.js polyfill:

$ metro-symbolicate composed.map < trace.txt
    at anonymous (address at …/js-polyfills/console.js:576:run)  # 0.87.1
    at anonymous (address at null:null:null)                     # main

On an RN 0.88 Android release build, 2,060 of 137,616 lookups at function entries change, 426 of them to null.

This keeps only the last mapping at each position, as Metro's Consumer resolves it, and keeps #1981's speedup. #1981 is unreleased, so its changelog line should come out of the 0.87.2 notes.

Changelog: Internal

Test plan:

@meta-cla meta-cla 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 Oct 23, 2026
@robhogan
robhogan marked this pull request as ready for review October 23, 2026 20:02
`hermesc` emits several mappings at some bytecode offsets, eg a function's declaration, then its first instruction. #1981 keeps all of them, but:

- Only the last covers any code. The others can't answer a lookup by position.
- Consumers that take the first, like `metro-symbolicate` and `trace-mapping`, resolve these to the wrong place, or nowhere:

```
$ metro-symbolicate composed.map < trace.txt
    at anonymous (address at …/js-polyfills/console.js:576:run)  # 0.87.1
    at anonymous (address at null:null:null)                     # main
```

On an RN 0.88 Android release build, 2,060 of 137,616 lookups at function entries change, 426 to `null`.

This keeps only the last mapping, as Metro's `Consumer` resolves it, and keeps #1981's speedup. #1981's changelog line should come out of the 0.87.2 notes.

Changelog: Internal

Test plan:
- The test from #1981 now expects only the last mapping.
- RN 0.88 release maps composed with their `hermesc -O` maps (Android, minified iOS) are byte-identical to 0.87.1.
- Timing against `main` is within noise.

This branch has not been deployed

No deployments
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.

1 participant