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
Fix compressed source-maps have non-terminated segments #1106
Fix compressed source-maps have non-terminated segments #1106
Conversation
@devversion Do you have a link for that source-map-visualization? You should be able to the "Link to this" link and it'll generate a share URL for you. |
@jridgewell Sure, here is the link for the sourcemap from the test. Likely there are some mappings that could be simplified/improved in this test, but that is not of relevance.. Only the generated 1-length segment mappings are new with this change, allowing the |
I've already seen this but haven't said anything because it's going to be super hard to verify this doesn't have the same issues as before. I still do want to advance the version though. |
yeah, that's very reasonable and I'm also not entirely sure on how to proceed here. It's definitely a little unfortunate that the corectness of source-maps previously couldn't be fixed due to a bug in some of the popular bundlers/source-map transformations, but at the same time, breaking the many users by triggering this bug (if it still there?!) transitively is also bad, yeah. I think, if we go with that at some point, then it might make sense to really put that into a major where people would not happen to trigger sourcemap parsing bugs out of nowhere (as reported back in the days). |
Releasing major versions gives me the chills. People are really slow to update, and especially transient dependencies get really stuck in the past. |
Slow updates being ironically the reason why a major would be necessary/optimal in the first place. |
Agreed, worth noting that uglify-js fixed this some time ago as well: mishoo/UglifyJS@46d142c. |
Hey, it seems like this has been a while. Happy to rebase, just let me know (also seeing the new source-map library used, nice! -- seems like this would also help with this change not generating a lot of unnecessary mappings) |
Hello there @devversion! Please do rebase. I'm probably not going to release a new version soon due to pain in my fingers (I'm typing on my phone) but as soon as this is sorted I'll get back here. |
00ad6ac
to
5faa961
Compare
@fabiosantoscode @jridgewell Rebased. I also had to update one of the source-map test fixtures because they changed as part of this change. The test inputs and map seemed very broken though, so I re-created a test fixture using TS -> JS and async await downleveling. Now seems to work as expected when I visualize the input & minified maps. |
This reverts commit 7c1e9cc.
5faa961
to
e3be5c1
Compare
Awesome! Sorry it took so long, but I'm merging and releasing today |
Thanks @fabiosantoscode. No problem at all! I realize this change unveiled some problems in the ecosystem (when we landed it initially), but I'm hoping this time it will work out. |
@fabiosantoscode Out of curiosity: Didn't you want to cut it in a major release (just confirming in case this is an oversight) |
I think it's been long enough that it's most likely not an issue. Webpack is more often updated than Terser, anyway. |
Currently when
terser
compresses a given input file that comeswith a source-map, but the source-map does not have mappings for
all code parts in the input file. e.g. the input file has specific code-parts have been generated (e.g. when async/await ia downleveled). Such. generated code does not originate from any source-file.
In that case if mappings before the "generated code" are collected, and
the original file location for the "generated code" comes after previous
segments for the same line, Terser just ignores the fact that there is no
original location and the previous segment is not terminated. This causes
the "generated code" incorrectly to be mapped to the previous segment/
original source location.
Reverts #381, which is a revert of #342
#342 was reverted to work around a bug that was thought to be caused by source-map,
but surfaced on many projects due to Terser emitting generated-only mappings (which
is valid per source map spec and per
source-map
module).It seems like Webpack or other tooling incorrectly added a source to mappings that
do not have any original source. It also looks like Webpack has changed its source
map logic over time already, so maybe this is no longer an issue? It's difficult to test
that out; but the source map emitted here is correct..
Visualized source map for the test: