fix(mangle): handle inferred names for functions #842
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
The t.NOT_LOCAL_BINDING symbol used in plugin-function-name (https://github.com/babel/babel/blob/bd98041321c60737ddcacd1ad7e056ef6de31879/packages/babel-helper-function-name/src/index.js#L206) is causing this issue. This was added in babel/babel#3298 The problem is the binding identifier is NOT registered (skipped) during crawl and the mangler finds the first safe identifier which ultimately collides with this name during runtime. So, we register the binding and don't respect the t.NOT_LOCAL_BINDING. Fix #829
j-f1
suggested changes
May 14, 2018
| function foo() { | ||
| var b = console; | ||
| return { | ||
| a: function d(c) { |
There was a problem hiding this comment.
Shouldn’t this be function a so that its .name is correct?
There was a problem hiding this comment.
Handled by keepFnName option.
There was a problem hiding this comment.
Shouldn’t we strip the name then?
There was a problem hiding this comment.
yes. Correct. Deadcode should have taken care of that (have to debug why it didn't in this case), and we come back to plugin ordering problem - #422
There was a problem hiding this comment.
I guess it's the same issue as this one - we have to add the same thing in DCE (register this binding before dce runs).
| * - https://github.com/babel/minify/issues/829 | ||
| */ | ||
| BindingIdentifier(path) { | ||
| const { scope } = path; |
vigneshshanmugam
approved these changes
May 14, 2018
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The t.NOT_LOCAL_BINDING symbol used in plugin-function-name
(https://github.com/babel/babel/blob/bd98041321c60737ddcacd1ad7e056ef6de31879/packages/babel-helper-function-name/src/index.js#L206)
is causing this issue.
This was added in babel/babel#3298
The problem is the binding identifier is NOT registered (skipped) during
crawl and the mangler finds the first safe identifier which ultimately
collides with this name during runtime.
So, we register the binding and don't respect the t.NOT_LOCAL_BINDING.
Fix #829