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

Move from source-map to source-map-js #12805

Closed
wants to merge 1 commit into from

Conversation

7rulnik
Copy link

@7rulnik 7rulnik commented Feb 14, 2021

Q                       A
Fixed Issues? Fixes #7486
Patch: Bug Fix? -
Major: Breaking Change? -
Minor: New Feature?
Tests Added + Pass? Yes
Documentation PR Link
Any Dependency Changes? Replaced source-map with source-map-sync
License MIT

I made the fork of source-map that introduces sync API, but still uses WASM.
You can compare changes to original versions here

@codesandbox-ci
Copy link

codesandbox-ci bot commented Feb 14, 2021

This pull request is automatically built and testable in CodeSandbox.

To see build info of the built libraries, click here or the icon next to each commit SHA.

Latest deployment of this branch, based on commit fdc8931:

Sandbox Source
babel-repl-custom-plugin Configuration
babel-plugin-multi-config Configuration

@babel-bot
Copy link
Collaborator

babel-bot commented Feb 14, 2021

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

@7rulnik 7rulnik force-pushed the source-map-sync branch 2 times, most recently from 51a9733 to 4e888d5 Compare February 14, 2021 23:38
@7rulnik
Copy link
Author

7rulnik commented Feb 14, 2021

Node 6 failed but it's expected because WASM support was introduced in Node 8

@7rulnik 7rulnik marked this pull request as ready for review February 15, 2021 00:35
@nicolo-ribaudo
Copy link
Member

nicolo-ribaudo commented Feb 15, 2021

Unfortunately we cannot drop Node.js 6 until Babel 8 😕

@7rulnik
Copy link
Author

7rulnik commented Feb 15, 2021

@nicolo-ribaudo yep, I know.

I also maintain source-map-js which is the fork of source-map, but with algorithmic optimizations. It's 4x faster than source-map@6, but still 2x slower than source-map-sync.

So I think we need to make a choice :)

@TrySound
Copy link
Contributor

@7rulnik Does source-map-js supports node 6? It makes sense to migrate to it first and the see if other improvements are necessary.

@7rulnik
Copy link
Author

7rulnik commented May 17, 2021

@TrySound yeah, it should. Will update this PR in few days

@7rulnik 7rulnik force-pushed the source-map-sync branch 2 times, most recently from 8975b3c to ec0f0cf Compare May 30, 2021 21:21
@7rulnik 7rulnik changed the title Move from source-map to source-map-sync Move from source-map to source-map-js May 30, 2021
@onigoetz
Copy link

onigoetz commented Jan 9, 2022

Hi @nicolo-ribaudo are you interested in merging this ?

@7rulnik can you update the dependency to ^1.0.0 ?

@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. Moving to source-map-js would eliminate the issue altogether.

@jridgewell
Copy link
Member

All of the SourceMapConsumers have switched over to @ampproject/remapping and @jridgewell/trace-mapping. There are still some SourceMapGenerators that could be switched to source-map-js.

@7rulnik
Copy link
Author

7rulnik commented Feb 24, 2022

@jridgewell yeah. I can update PR if it makes sense and we will merge it

@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 29, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Jul 29, 2022
@nicolo-ribaudo nicolo-ribaudo added this to the v8.0.0 milestone Aug 8, 2023
@JLHwung JLHwung removed the babel 8 label Aug 9, 2023
@JLHwung JLHwung removed this from the v8.0.0 milestone Aug 9, 2023
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: Dependency ⬆️ PR: New Dependency
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Please upgrade source-map to 0.7.2 to get speed improvements
8 participants