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

Do not apply negate_iife optimization to new expression #1074

Merged
merged 2 commits into from
May 16, 2016
Merged

Do not apply negate_iife optimization to new expression #1074

merged 2 commits into from
May 16, 2016

Conversation

kzc
Copy link
Contributor

@kzc kzc commented May 9, 2016

Potential fix for #1073

@rvanvelzen
Copy link
Collaborator

I think just checking for AST_New is enough? The cause here is that AST_New extends from AST_Call, but it shouldn't get the same treatment here.

@kzc
Copy link
Contributor Author

kzc commented May 15, 2016

I think it's a problem that's specific to ECMAscript new syntax. Do you have a counter case involving a javascript call that fails in a major browser or node?

@rvanvelzen
Copy link
Collaborator

Sorry, I may have been a bit unclear. The check can be simplified to just:

if (node instanceof AST_New) {
    return node;
}

... because even though this case only fails when node.expression is a function, new doesn't need this optimization anyway.

as suggested by @rvanvelzen.

Added a test for IIFEs in nested contexts.
@kzc
Copy link
Contributor Author

kzc commented May 15, 2016

@rvanvelzen Good point. I wasn't sure such a fix would allow for nested IIFE's, but it does. PR revised.

@rvanvelzen rvanvelzen merged commit 5f464b4 into mishoo:master May 16, 2016
@rvanvelzen
Copy link
Collaborator

Awesome, thanks!

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