Skip to content
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 private getter/setter mangling #1060

Merged
merged 7 commits into from Sep 5, 2021

Conversation

jridgewell
Copy link
Collaborator

@jridgewell jridgewell commented Aug 31, 2021

This fixes an error with private getter/setters being mangled as if they were regular public properties:

export class Foo {
  get #aaaaaa() {}
  constructor() {
    this.#aaaaaa
  }
}

In this, get #aaaaaa {} is a AST_PrivateGetter, which isn't a subclass of AST_ClassPrivateProperty nor AST_PrivateMethod. But it is a subclass of AST_ObjectProperty. So our property mangler was mangling the key as if it were a public property, and not consulting the private_cache mapping. So the mangler renames the getter to #o and the access to #a:

class Foo {
    get #o() {}
    constructor() {
        this.#a;
    }
}

This also enables private property mangling by default whenever we're doing a normal mangle pass. This is as safe as mangling regular const variable_name = 1, because private properties are completely analyzable in the lexical scope.

Re: #1012

@jridgewell jridgewell changed the title Mangle private properties by default when mangling Fix private property mangling Aug 31, 2021
@jridgewell jridgewell changed the title Fix private property mangling Fix private getter/setter mangling Aug 31, 2021
@fabiosantoscode
Copy link
Collaborator

This seems to close #1012

@jridgewell
Copy link
Collaborator Author

Possibly, I don't know if you want to use #1012 to mention that we do private property mangling as a default mangle step. But yes, this does move the mangling into the default step as requested in #1012.

@fabiosantoscode
Copy link
Collaborator

Ok, I checked it more thoroughly, it looks great!

@fabiosantoscode fabiosantoscode merged commit e6312f4 into terser:master Sep 5, 2021
@fabiosantoscode
Copy link
Collaborator

Thank you! ❤️

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants