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 collisions between original property and mangled property #1063
Fix collisions between original property and mangled property #1063
Conversation
@@ -167,9 +167,6 @@ function mangle_properties(ast, options) { | |||
var private_cache = new Map(); | |||
if (options.cache) { | |||
cache = options.cache.props; | |||
cache.forEach(function(mangled_name) { | |||
reserved.add(mangled_name); | |||
}); |
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.
I think we should instead be pushing to unmangleable
. It fulfills the same role as reserved
, but is only checked when nth_identifier
comes up with a new mangled name.
The reason this was added was to stop mangling from creating colliding names, eg. if something mangles to X but there's already an X in a previous file that Terser mangled.
Here's when/why these lines were added, for more context:
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.
Would've added a fancy GH suggestion, but unmangleable
is defined after this so it's not as simple as that.
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.
Good catch. Fixed, and updated the test for this case.
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.
I think just that one thing needs to be changed
This fixes a bug when an original property (`obj.i`) shares the same name as a mangled property (`obj.prop` -> `n.i`), and you're using a name-cache. Because our mangled name collides, we have to ensure all original `.i` properties are renamed to something else (`n.o`). But, because we had a name-cache, we would reserve the mangled property name in the mangler. That prevents us from attempting to rename the original `.i` property to `.o`.
0605bc3
to
7ac3aca
Compare
7ac3aca
to
5a9d757
Compare
Great stuff, thanks 💪 |
This fixes a bug when an original property (
obj.i
) shares the same name as a mangled property (obj.prop
->n.i
), and you're using a name-cache. Because our mangled name collides, we have to ensure all original.i
properties are renamed to something else (n.o
).But, because we had a name-cache, we would reserve the mangled property name in the mangler. That prevents us from attempting to rename the original
.i
property to.o
.Fixes #233.