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

Please upgrade source-map to 0.7.2 to get speed improvements #7486

Closed
andreicristianpetcu opened this issue Mar 4, 2018 · 11 comments
Closed
Labels
outdated A closed issue/PR that is archived due to age. Recommended to make a new issue

Comments

@andreicristianpetcu
Copy link

Expected Behavior

Babel should use the latest source-map which is v0.7.2. v0.7.2 got support for WebAssembly and it is 10 times faster than the old one. More info here:

https://github.com/mozilla/source-map/blob/master/CHANGELOG.md#072
http://fitzgeraldnick.com/2018/02/26/speed-without-wizardry.html
https://hacks.mozilla.org/2018/01/oxidizing-source-maps-with-rust-and-webassembly/

Current Behavior

Babel uses an old version of source-map

Context

I want source-map to be newer in debugger.html and I see it uses an old source map package since babel requires it.

@babel-bot
Copy link
Collaborator

Hey @andreicristianpetcu! We really appreciate you taking the time to report an issue. The collaborators
on this project attempt to help as many people as possible, but we're a limited number of volunteers,
so it's possible this won't be addressed swiftly.

If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack
community that typically always has someone willing to help. You can sign-up here
for an invite.

@andreicristianpetcu
Copy link
Author

Here is the downstream issue firefox-devtools/debugger#5598

@andreicristianpetcu
Copy link
Author

I'm personally interested in getting the update for: babel-generator, babel-cli, babel-core and babel-register since this is needed by debugger.html

@danez
Copy link
Member

danez commented Mar 4, 2018

These speed improvements look awesome, but they also come with a price and that is that version >=0.7 requires node 8 at least.
As sad as this is but right now we cannot update to this newer version. We still support node 4 for babel 7 and even if we decide to drop node 4 because it runs out of support soon, then we would still support node 6 for at least a year or more.

@nicolo-ribaudo
Copy link
Member

Can we install version >= 0.7 as an optionalDependency and only use it in node 8?

@realityking
Copy link
Contributor

Babel should probably first update to 0.6.1 as that contains some breaking API changes, otherwise usage of 0.7 is out of the question anyway.

@danez
Copy link
Member

danez commented Mar 4, 2018

I seems also the api changed, so we would need to support different versions in our code

Breaking change: new SourceMapConsumer now returns a Promise object that resolves to the newly constructed SourceMapConsumer instance, rather than returning the new instance immediately.

(ups wrong button :) )

@danez danez closed this as completed Mar 4, 2018
@danez danez reopened this Mar 4, 2018
@loganfsmyth
Copy link
Member

loganfsmyth commented Mar 4, 2018

Oh interesting, if there's no way to create a sourcemap consumer synchronously, I'm not sure there's any way we can use this, since currently Babel exposes primarily synchronous transformation APIs.

We've starting to add infrastructure for Babel's APIs being async, but that will take time and will likely require either dropping or rearchitecting babel-register from scratch, for instance. The cases for consumer for us are:

For babel/core, when merging source-maps in babel/core we have to return the merged map synchronously

For babel/register, require() hooks are synchronous, so without reworking babel-register to compile up front, it seems impossible for us to make use of any APIs in core that are async

node-source-map-support requires remapping stack traces synchronously, so it seems like that would also be a no-go?

@coreyfarrell
Copy link
Contributor

@loganfsmyth Would you accept a patch to upgrade to source-map@0.6.1 for now so babel will use the 'latest possible'? I tested locally with coverage enabled, most files where source-map is imported had very high coverage:

-----------------------------------------------------------------------|----------|----------|----------|----------|-------------------|
File                                                                   |  % Stmts | % Branch |  % Funcs |  % Lines | Uncovered Line #s |
-----------------------------------------------------------------------|----------|----------|----------|----------|-------------------|
 packages/babel-cli/src/babel/file.js                                  |        0 |        0 |        0 |        0 |                   |
 packages/babel-core/src/transformation/file/merge-map.js              |    97.85 |    90.24 |      100 |    98.85 |               302 |
 packages/babel-generator/src/source-map.js                            |      100 |    95.83 |      100 |      100 |                20 |
 packages/babel-helper-transform-fixture-test-runner/src/index.js      |    85.16 |       76 |    89.47 |    87.29 |... 72,275,284,310 |

I'm not sure how to get test coverage to that code in babel-cli. That source uses SourceMapGenerator and SourceMapConsumer. SourceMapConsumer is used with a single parameter so I believe it should be OK. Basing this idea off the git diff between source-map 0.5.7 and 0.6.0 (I couldn't find a changelog detailing breaking changes).

@jordanbtucker
Copy link
Contributor

jordanbtucker commented Feb 24, 2022

This will also fix an annoying issue in VS Code.

source-map@<0.7.1 has an invalid typings field in its package.json which is causing VS Code to report an error that it can't find the type declarations. This is fixed in source-map@>=0.7.1.

@jridgewell
Copy link
Member

Closed with #14253

@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 Jul 27, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 27, 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
Projects
None yet
Development

Successfully merging a pull request may close this issue.

9 participants