Do not emit source map names for identical names#18005
Conversation
|
|
||
| (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.
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() {
|
Build successful! You can test your changes in the REPL here: https://babeljs.io/repl/build/61580 |
c016155 to
6ab723d
Compare
|
commit: |
| "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.
cec24c0 to
c046ecc
Compare
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.
bf66b11 to
e678d68
Compare
|
I tested the integration of this PR with Jest, and it doesn't seem to have affected the tests. |
|
Yeah CI error is related, thanks for skipping it for now! It seems like the new stack trace is actually more correct. |
Fixes #1, Fixes #2I 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
namein it.Babel emits a
namefor 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 :)