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

Bug of compile (1, eval) #1793

Closed
rwu823 opened this issue Dec 14, 2017 · 8 comments
Closed

Bug of compile (1, eval) #1793

rwu823 opened this issue Dec 14, 2017 · 8 comments

Comments

@rwu823
Copy link

rwu823 commented Dec 14, 2017

input:

(1, eval)('this')

output:

(eval)('this')
@rwu823
Copy link
Author

rwu823 commented Dec 29, 2017

Any updates?

@adrianheine
Copy link
Contributor

Could you explain the issue a bit further? Since the comma operator only returns the last item and 1 is side-effect free, it seems correct to me to drop that part of the expression?

@rwu823
Copy link
Author

rwu823 commented Dec 29, 2017

@adrianheine you can create one JS file and then try both look what is difference

@lukastaegert
Copy link
Member

This looks like a transformation used by a transpiler like Babel. We had similar issues with (0, x.y)() having the wrong this context for which we added a very specific fix. We could solve this by making the fix more general but it would be really interesting to know what library/framework produces such code. If it is handwritten code you could work around it by writing something like (1 && eval)('this') instead

@adrianheine
Copy link
Contributor

@rwu823 Now I understand. I only checked in the nodejs REPL where both behave the same.

@guybedford
Copy link
Contributor

I wonder if this case could be added as a special exception, as it is a known pattern?

@lukastaegert
Copy link
Member

I agree. The question is only how specific this fix should be as eval does not necessarily need to be global.eval while at the same time, global.eval might have been assigned to another variable.

I have been considering for some time to create a more general solution for these kinds of issues as they will become more prevalent once we start simplifying resolvable conditional expressions.
The same issue is encountered when simplifying

(true ? eval : ()=>{})('this');
(true && eval)('this')

and of course any chain of these constructs. With #1949, I plan on introducing a new context-specific render parameter that could be extended to signal a node that it is inside a call expression where it was previously wrapped in some context-shielding construct. Then if a member expression or an identifier (to solve this issue) encounters this flag, it could automatically wrap itself into e.g. a (0, <expression>) for rendering.

@guybedford
Copy link
Contributor

Indirect call cases are all properly handled now thanks to some careful work by @lukastaegert.

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

No branches or pull requests

4 participants