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 compress/eval of optional chains #1062

Merged
merged 6 commits into from Sep 19, 2021

Conversation

jridgewell
Copy link
Collaborator

This implements the recursion necessary to determine if a property access/call is part of an optional chain that has short-circuited.

Fixes #1040

Copy link
Collaborator

@fabiosantoscode fabiosantoscode left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This one is looking great in terms of high level structure, but does need a few things changed.

jridgewell and others added 3 commits September 12, 2021 21:32
This implements the recursion necessary to determine if a property access/call is part of an optional chain that has short-circuited.
Co-authored-by: Fábio Santos <fabiosantosart@gmail.com>
@@ -2436,7 +2432,7 @@ def_optimize(AST_Call, function(self, compressor) {
}

var ev = self.evaluate(compressor);
if (ev !== self) {
if (ev !== self && ev !== nullish) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can remove these checks, by making evaluate return this when it sees nullish.

https://github.com/terser/terser/pull/1062/files#diff-3e5692bda6529f2cdc21e76c6bc0d34e209304ad796f83acb5905d4b84931d8eR93-R102

There's already a bit of a redirection, so it's probably good to take advantage of it.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

For some reason, the link doesn't show me where you're pointing. I'm assuming it's the AST_Node.DEFMETHOD("evaluate", …) definition, so I added a check there.

@fabiosantoscode
Copy link
Collaborator

I asked for just one more little change. This is otherwise looking great, love those tests!

@fabiosantoscode fabiosantoscode merged commit 8dea529 into terser:master Sep 19, 2021
@fabiosantoscode
Copy link
Collaborator

This is great, thank you 💪

Will release on Monday

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.

terser mangled my optional chaining operator
2 participants