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

Convert all SourceMapConsumers to TraceMaps #14253

Merged
merged 5 commits into from Feb 10, 2022

Conversation

jridgewell
Copy link
Member

@jridgewell jridgewell commented Feb 9, 2022

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

This accomplishes 2 things:

  1. Moves us off a 4 year old dependency that is no longer maintained (in a non-wasm form)
  2. Speeds up construction and tracing by ~10x.

This accomplishes 2 goals:
- Gets us off the ancient `source-map` v0.5.7
- Speeds up construction and tracing by ~10x.
@jridgewell jridgewell force-pushed the fixture-test-runner-trace-mapping branch from 37a3b17 to 7b0a467 Compare February 9, 2022 05:03
@babel-bot
Copy link
Collaborator

babel-bot commented Feb 9, 2022

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

@nicolo-ribaudo nicolo-ribaudo added the PR: Internal 🏠 A type of pull request used for our changelog categories label Feb 9, 2022
"@types/convert-source-map": "^1.5.1",
"@types/debug": "^4.1.0",
"@types/resolve": "^1.3.2",
"@types/semver": "^5.4.0",
"@types/source-map": "^0.5.0",
"source-map": "0.6.1"
"@types/source-map": "^0.5.0"
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we still require the typings?

Copy link
Member Author

Choose a reason for hiding this comment

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

Removed.

@@ -26,6 +26,7 @@
"devDependencies": {
"@babel/helper-fixtures": "workspace:^",
"@babel/parser": "workspace:^",
"@jridgewell/trace-mapping": "^0.3.4",
"@types/jsesc": "^2.5.0",
"@types/source-map": "^0.5.0",
Copy link
Contributor

Choose a reason for hiding this comment

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

ditto.

Copy link
Member Author

Choose a reason for hiding this comment

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

generator does, because it uses SourceMapGenerator. trace-mapping only offers a Consumer API.

@JLHwung
Copy link
Contributor

JLHwung commented Feb 10, 2022

Merging as-is. The e2e error is unrelated to this PR.

@JLHwung JLHwung merged commit d610f83 into babel:main Feb 10, 2022
@jridgewell jridgewell deleted the fixture-test-runner-trace-mapping branch February 10, 2022 22:20
@github-actions github-actions bot added the outdated A closed issue/PR that is archived due to age. Recommended to make a new issue label May 13, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators May 13, 2022
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue PR: Internal 🏠 A type of pull request used for our changelog categories
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants