Skip to content

perf: replace source-map package with @jridgewell/trace-mapping#489

Merged
alexander-akait merged 1 commit into
webpack:masterfrom
onigoetz:master
Jun 1, 2022
Merged

perf: replace source-map package with @jridgewell/trace-mapping#489
alexander-akait merged 1 commit into
webpack:masterfrom
onigoetz:master

Conversation

@onigoetz

@onigoetz onigoetz commented Jun 1, 2022

Copy link
Copy Markdown
Contributor

This PR contains a:

  • bugfix
  • new feature
  • code refactor
  • test update
  • typo fix
  • metadata update

Motivation / Use-Case

The trace-mapping module is faster than source-map in every benchmark
https://github.com/jridgewell/trace-mapping#benchmarks

It's also being adopted by many packages across the ecosystem (Jest, Babel, Terser at least have included it already)
And it doesn't need to be async like source-map 0.7.*

A previous PR to upgrade for source-map 0.7 was refused in the past : #280

Breaking Changes

None

Additional Info

@alexander-akait

Copy link
Copy Markdown
Member

@onigoetz I think we need to change it in more places, we don't think it will improve somthing, because we use source maps here only for error reporting

@codecov

codecov Bot commented Jun 1, 2022

Copy link
Copy Markdown

Codecov Report

Merging #489 (0e2ce1b) into master (66fdec8) will not change coverage.
The diff coverage is 60.00%.

@@           Coverage Diff           @@
##           master     #489   +/-   ##
=======================================
  Coverage   97.11%   97.11%           
=======================================
  Files           3        3           
  Lines         312      312           
  Branches      114      114           
=======================================
  Hits          303      303           
  Misses          9        9           
Impacted Files Coverage Δ
src/utils.js 95.00% <ø> (ø)
src/index.js 97.34% <60.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 66fdec8...0e2ce1b. Read the comment docs.

@onigoetz

onigoetz commented Jun 1, 2022

Copy link
Copy Markdown
Contributor Author

I understand that in this library's case it won't impact performance nor package size greatly

However there seems to be a big interest to move away from source-map and source-map-js towards these packages and I think that every package adopting this newer alternative is a move towards deprecating an unmaintained package and at some point remove it from our dependency trees entirely

@alexander-akait

Copy link
Copy Markdown
Member

@onigoetz I agree, let's migrate, I think we should do the same in css-minimizer-webpack-plugin and maybe somewhere else? 🤔

@alexander-akait alexander-akait merged commit 6020a94 into webpack:master Jun 1, 2022
@alexander-akait

Copy link
Copy Markdown
Member

Thanks

@onigoetz

onigoetz commented Jun 1, 2022

Copy link
Copy Markdown
Contributor Author

I personally did a yarn why source-map on my main project and am trying to create PRs here and there :)

@alexander-akait

Copy link
Copy Markdown
Member

Feel free to send PRs in other repos 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants