-
-
Notifications
You must be signed in to change notification settings - Fork 1.6k
Improve approximation of coarse sourcemap segments #5985
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
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
Thank you for your contribution! ❤️You can try out this pull request locally by installing Rollup via npm install rollup/rollup#fix/tweak-sourcemaps-merge Notice: Ensure you have installed the latest stable Rust toolchain. If you haven't installed it yet, please see https://www.rust-lang.org/tools/install to learn how to download Rustup and install Rust. or load it into the REPL: |
Performance report
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## master #5985 +/- ##
=======================================
Coverage 98.78% 98.78%
=======================================
Files 270 270
Lines 8735 8745 +10
Branches 1510 1512 +2
=======================================
+ Hits 8629 8639 +10
Misses 73 73
Partials 33 33 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
8870b91
to
e77df78
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not 100% sure there might be some performance impact but it looks reasonable to me to go with it.
This PR has been released as part of rollup@4.44.0. You can test it via |
We did notice a performance impact, I opened #6056 about it. I linked a repro repo with some measurements; in our real world case the build time increased from about 1 to about 2 minutes. This is caused by a huge 3rd party library that we bundle with our code. During our build, the We will work around this by excluding the lib from sourcemap generation. I intend to open a PR to add a rollup option to support this (a bit like is done in this gist ); in the meantime we can work around the issue with |
Thank you very much for reporting this performance regression — we didn’t expect it to have such a big impact. I’m thinking there might still be room for optimization in the fallback logic. If it really can’t be improved, I think we could consider reverting this commit or going with your suggestion to exclude minified JS files. |
This PR contains:
Are tests included?
Breaking Changes?
List any relevant issue numbers:
resolves #5955
Description
Relevant comment: #5961 (comment)
Currently, esbuild is generating some coarse segments, though it may produce more precise ones in the future evanw/esbuild#4189. Since we already have a fallback logic in place, we only need to extend it to cover the esbuild case.