Switch to `@ampproject/remapping` to merge source maps #14209

jridgewell · 2022-01-27T04:53:19Z

Q	A
Fixed Issues?	`Fixes #1, Fixes #2`
Patch: Bug Fix?
Major: Breaking Change?
Minor: New Feature?
Tests Added + Pass?	Yes
Documentation PR Link
Any Dependency Changes?	Yes
License	MIT

This switches us from the custom-build source map merging to @ampproject/remapping. This should result in a considerably faster merges with reduced memory usage. Eg, angular/angular-cli#21779 found that using the old method caused OOM errors when the input/output maps were large enough.

babel-bot · 2022-01-27T04:56:39Z

Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/50972/

nicolo-ribaudo · 2022-01-27T14:57:19Z

packages/babel-core/src/transformation/file/merge-map.ts

-    // but eachMapping returns mappings with absolute paths. To avoid that
-    // incompatibility, we leave the sourceRoot out here and add it to the
-    // final map at the end instead.
+    // This is a bit hack. Remapping will create absolute sources in our


Is it true for @ampproject/remapping too? In the readme, the helloworld.js doesn't seem to be transformed to absolute.

Unfortunately so: https://github.com/ampproject/remapping/blob/fcc9f442f43249389dc946cbe8bb3736a8033d6a/src/source-map-tree.ts#L115.

In the readme, the helloworld.js doesn't seem to be transformed to absolute.

The readme example doesn't have a sourceRoot. If one were included, say src, then the output would partially match { sourceRoot: undefined, sources: ['src/helloworld.js'] }.

nicolo-ribaudo · 2022-01-27T15:02:06Z

Could you verify if the failing tests are correct?

jridgewell · 2022-01-28T01:51:02Z

I actually get 0 failures on my local, so I'm still trying to debug what's happening.

jridgewell

The changed sourcemaps here all refer to the same var foo = () => 4; input, which only has 2 mappings:

var foo = () => 4;
^^^^^^^^^
         ^^^^^^^^^

The old code would create new mappings based on the transformed map, but these don't actually point to a sane spot in the original file. See how when you hover over foo on the left, it hightlights var foo = on the right? This is because the foo mapping is pointing to line 1 column 0, not column 4 (where foo exists in the original code). If it did, it would correctly highlight the foo identifier on the right side.

The new output doesn't generate a useless mapping for foo. But, if the input sourcemap were higher fidelity (with mappings at identifiers, syntax, etc), it would.

jridgewell · 2022-01-28T04:22:47Z

packages/babel-core/test/api.js

@@ -540,7 +540,7 @@ describe("api", function () {
        column: 4,
      }),
    ).toEqual({
-      name: null,
+      name: "Foo",


The "Foo" name doesn't exist in the input sourcemap, but does in the transformed sourcemap. Remapping will use the input's if it existed, but falls back to the transformed one if not.

nicolo-ribaudo

Thanks!

What is happing here is that when using the linker on `@angular/material` the sourcemap will contain multiple sources example. Babel now uses [remapping](https://github.com/ampproject/remapping/) to merge sourcemaps, see: babel/babel#14209. This doesn't support multiple sources which the linker produduces. Previously, Babel failed silently and didn't generate a sourcemap angular/angular#42769 at all. Linker produced sourcemap ```js [ '/packages/web-app-edit/Users/cli-reproductions/monorepo-new/node_modules/@angular/material/fesm2020/card.mjs', '/packages/web-app-edit/Users/src/material/card/card.html', '/packages/web-app-edit/Users/src/material/card/card-header.html', '/packages/web-app-edit/Users/src/material/card/card-title-group.html' ] ``` Will cause the below error during merging ``` Transformation map 0 must have exactly one source file. ``` Related to - angular/angular#42769 - angular/angular#44972 Addresses CI failure - https://app.circleci.com/pipelines/github/angular/angular/42281/workflows/e060088b-5963-43b0-b6fc-b4ddd8855bee/jobs/1116551

When using the linker on `@angular/material` or packages which contain external sourcemap. The sourcemap will contain multiple sources example. Babel now uses [remapping](https://github.com/ampproject/remapping/) to merge sourcemaps, see: babel/babel#14209. This doesn't support multiple sources which the linker produduces. Previously, Babel failed silently and didn't generate a sourcemap angular/angular#42769 at all. Linker produced sourcemap ```js [ '/packages/web-app-edit/Users/cli-reproductions/monorepo-new/node_modules/@angular/material/fesm2020/card.mjs', '/packages/web-app-edit/Users/src/material/card/card.html', '/packages/web-app-edit/Users/src/material/card/card-header.html', '/packages/web-app-edit/Users/src/material/card/card-title-group.html' ] ``` Will cause the below error during merging ``` Transformation map 0 must have exactly one source file. ``` Related to - angular/angular#42769 - angular/angular#44972 Addresses CI failure - https://app.circleci.com/pipelines/github/angular/angular/42281/workflows/e060088b-5963-43b0-b6fc-b4ddd8855bee/jobs/1116551

Surprisingly, Babel allows a transformer to mark the source file of a node to allow it to be sourced from any file. When this happens, the output sourcemap will contain multiple `sources`. I didn't realize this when I created babel#14209, and this `remapping` will throw an error if the output map has multiple sources. This can be fixed by using `remapping`'s graph building API (don't pass an array). This allows us to return an input map for _any_ source file, and we just need some special handling to figure out which source is our transformed file. Fixes ampproject/remapping#159.

Surprisingly, Babel allows a transformer to mark the source file of a node to allow it to be sourced from any file. When this happens, the output sourcemap will contain multiple `sources`. I didn't realize this when I created babel#14209, and this `remapping` will throw an error if the output map has multiple sources. This can be fixed by using `remapping`'s graph building API (don't pass an array). This allows us to return an input map for _any_ source file, and we just need some special handling to figure out which source is our transformed file. This actually adds a new feature, allowing us to remap these multi-source outputs. Previously, the merging would silently fail and generate a blank (no `mappings`) sourcemap. That's not great. The new behavior will properly merge the maps, provided we can figure out which source is the transformed file (which should always work, I can't think of a case it wouldn't). Fixes ampproject/remapping#159.

Surprisingly, Babel allows a transformer to mark the source file of a node to allow it to be sourced from any file. When this happens, the output sourcemap will contain multiple `sources`. I didn't realize this when I created #14209, and this `remapping` will throw an error if the output map has multiple sources. This can be fixed by using `remapping`'s graph building API (don't pass an array). This allows us to return an input map for _any_ source file, and we just need some special handling to figure out which source is our transformed file. This actually adds a new feature, allowing us to remap these multi-source outputs. Previously, the merging would silently fail and generate a blank (no `mappings`) sourcemap. That's not great. The new behavior will properly merge the maps, provided we can figure out which source is the transformed file (which should always work, I can't think of a case it wouldn't). Fixes ampproject/remapping#159.

…ty and performance of babel sourcemaps With this change we remove the workaround for fidelity and performance of babel sourcemaps as this is no longer needed as Babel now uses `@ampproject/remapping` to merge sourcemaps. See babel/babel#14209 for more context.

Switch to @ampproject/remapping to merge source maps

74cac9c

yarn dedupe

a200470

nicolo-ribaudo reviewed Jan 27, 2022

View reviewed changes

Update sourcemaps

5ce885d

jridgewell commented Jan 28, 2022

View reviewed changes

nicolo-ribaudo approved these changes Jan 29, 2022

View reviewed changes

nicolo-ribaudo added the PR: New Dependency label Jan 29, 2022

JLHwung added the PR: Performance 🏃‍♀️ A type of pull request used for our changelog categories label Jan 29, 2022

JLHwung approved these changes Jan 29, 2022

View reviewed changes

nicolo-ribaudo changed the title ~~Switch to @ampproject/remapping to merge source maps~~ Switch to @ampproject/remapping to merge source maps Jan 29, 2022

nicolo-ribaudo merged commit 7d32f49 into babel:main Jan 29, 2022

jridgewell deleted the remapping branch January 29, 2022 21:34

jridgewell mentioned this pull request Jan 31, 2022

wg-performance Status Update 2022-01-31 ampproject/wg-performance#104

Closed

alan-agius4 mentioned this pull request Feb 4, 2022

refactor(@angular-devkit/build-angular): remove workaround for fidelity and performance of babel sourcemaps angular/angular-cli#22646

Merged

pmdartus mentioned this pull request Feb 4, 2022

don't know how to turn this value into a node at valueToNode (node_modules/@babel/types/lib/converters/valueToNode.js:98:9) salesforce/sfdx-lwc-jest#263

Closed

alan-agius4 mentioned this pull request Feb 4, 2022

fix(bazel): disable angular linker sourcemaps angular/dev-infra#385

Merged

JoostK mentioned this pull request Feb 5, 2022

Remapping when there's multiple sources ampproject/remapping#159

Closed

jridgewell mentioned this pull request Feb 6, 2022

Merge multi-source output sourcemaps #14246

Merged

jridgewell mentioned this pull request Feb 16, 2022

wg-performance Status Update 2022-02-14 ampproject/wg-performance#105

Closed

github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label May 1, 2022

github-actions bot locked as resolved and limited conversation to collaborators May 1, 2022

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Switch to `@ampproject/remapping` to merge source maps #14209

Switch to `@ampproject/remapping` to merge source maps #14209

jridgewell commented Jan 27, 2022 •

edited by gitpod-io bot

babel-bot commented Jan 27, 2022 •

edited

nicolo-ribaudo Jan 27, 2022

jridgewell Jan 28, 2022

nicolo-ribaudo commented Jan 27, 2022 •

edited

jridgewell commented Jan 28, 2022

jridgewell left a comment •

edited

jridgewell Jan 28, 2022

nicolo-ribaudo left a comment

Switch to @ampproject/remapping to merge source maps #14209

Switch to @ampproject/remapping to merge source maps #14209

Conversation

jridgewell commented Jan 27, 2022 • edited by gitpod-io bot

babel-bot commented Jan 27, 2022 • edited

nicolo-ribaudo Jan 27, 2022

Choose a reason for hiding this comment

jridgewell Jan 28, 2022

Choose a reason for hiding this comment

nicolo-ribaudo commented Jan 27, 2022 • edited

jridgewell commented Jan 28, 2022

jridgewell left a comment • edited

Choose a reason for hiding this comment

jridgewell Jan 28, 2022

Choose a reason for hiding this comment

nicolo-ribaudo left a comment

Choose a reason for hiding this comment

Switch to `@ampproject/remapping` to merge source maps #14209

Switch to `@ampproject/remapping` to merge source maps #14209

jridgewell commented Jan 27, 2022 •

edited by gitpod-io bot

babel-bot commented Jan 27, 2022 •

edited

nicolo-ribaudo commented Jan 27, 2022 •

edited

jridgewell left a comment •

edited