-
Notifications
You must be signed in to change notification settings - Fork 13
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
- Loading branch information
Benjamin Coe
committed
Dec 27, 2016
1 parent
2d781da
commit 590abf6
Showing
2 changed files
with
11 additions
and
1 deletion.
There are no files selected for viewing
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
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
590abf6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Attrash-Islam can you share the project that is having this error?
590abf6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wyze any thoughts on this one? seems to be similar behavior to this issue:
mishoo/UglifyJS#179
590abf6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Reading the issue from uglify, it seems like this code would be what is causing the error:
@Attrash-Islam, do you have something along those lines in your codebase?
@bcoe, one thing comes to mind, as this is mainly aimed towards React components for their displayName. I could put a guard in to check and see if a parameter of the function matches the variable declaration name, if so, do something like
const area = function _area(something, area) {}
instead.590abf6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've been able to come up with a failing test case that reproduces this though.
590abf6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wyze great, I think this safety would be good 👍 great catch. I didn't know about this
strict
rule -- I believe it's only in certain browsers?590abf6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll get that test case coded up. I can reproduce the error in PhantomJS REPL though:
590abf6
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@wyze awesome; I'd rather not pull phantom in as a dependency, but a test that would fail in a phantom environment would be good.
maybe add an additional
npm script
that executes in phantom, but that we don't run in CI?