Do not emit source map names for identical names - #18005
Conversation
Sorry, something went wrong.
|
|
||
| (1:10-12) Test { <-- (4:2-11) function Test | ||
| ^^ ^^^^^^^^^ | ||
| (1:10-12) Test { <-- (4:2-19) function Test() { |
There was a problem hiding this comment.
Marking as draft until I understand what's going on here.
Sorry, something went wrong.
There was a problem hiding this comment.
Oh, this is actually fine. Before we were mapping all of the locations in function Test() { to after class Test, and we were giving name: Test to Test(.
That's not happening anymore (since it's not renamed), so the source map now has a single mapping-to-nowhere-useful for the whole function Test() {
Sorry, something went wrong.
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/61580 |
Sorry, something went wrong.
|
commit: e678d68 |
Sorry, something went wrong.
| "e", | ||
| "#e", |
There was a problem hiding this comment.
The reason we have both e and #e here is that the first one is for the renamed identifier, while the second one is for the name of the function.
The first one is probably not needed, but it's unrelated to this PR.
Sorry, something went wrong.
Andarist
left a comment
There was a problem hiding this comment.
LGTM 😉
Sorry, something went wrong.
liuxingbaoyu
left a comment
There was a problem hiding this comment.
The CI failed because toMatchTemplate failed and the error message wasn't reported correctly.
I agree that this PR shouldn't be ported to Babel 7 because it changes some behavior of source-map-support.
Sorry, something went wrong.
|
I tested the integration of this PR with Jest, and it doesn't seem to have affected the tests. |
Sorry, something went wrong.
|
Yeah CI error is related, thanks for skipping it for now! It seems like the new stack trace is actually more correct. |
Sorry, something went wrong.
I am working on an implementation of range mappings (https://github.com/tc39/ecma426/blob/main/proposals/range-mappings.md). For that to work well, we need to minimize the number of places where we would need to force a mapping not because code is moved around, but because we need to assign a name in it.
Babel emits a name for each identifier, but it's actually only needed when the new name is different from the original one. This also applies to #14907.
Even though this is PR should be safe, I'd avoid backporting it to Babel 7.
@Andarist as you very recently touched this logic, I'd appreciate if you could do a quick review :)