disallowIdentifierNames: fix errors with using object properties as a id...#1204
disallowIdentifierNames: fix errors with using object properties as a id...#1204hzoo wants to merge 1 commit intojscs-dev:masterfrom hzoo:disallowIdentifierNames-fix
Conversation
|
Lgtm |
|
Changed to check with |
|
LGTM |
There was a problem hiding this comment.
Actually this will cause a problem if a hasOwnProperty key exists on the object. You want Object.prototype.hasOwnProperty.call(disallowedIdentifiers, node.name)
|
@hzoo any chance of fixing my comment? I'd love to see this merged and released asap :-) |
… identifier Fixes #1203
|
@ljharb Made the change. Should I write a test for this? (configure with |
|
@hzoo that's up to @mrjoelkemp and the collaborators. I don't typically write a test for this case, because one would also have to test for @mrjoelkemp any chance this could be merged and published soon? :-) |
|
@ljharb yeah, I can land some PR's and release a patch version tonight. |
|
@mikesherov hooray, thanks! I've got lots of repos I want to bump |
|
@ljharb got delayed in doing this last night. Sorry about that, will hopefully get to this tonight! |
|
@mikesherov np, any chance for today? |
|
@mikesherov :-) ping again? (please stop me if this form of a reminder isn't helpful, not trying to be a bother) |
|
@mikesherov Can I help you with |
|
The pinging isn't necessary :) We don't usually forget about PRs, we just do a bunch of merges in one sitting. @zxqfox, since it's been approved by two maintainers, it should be fine to initiate the fast-forward merge. We'd still have to cut a patch release, which won't happen today. However, @ljharb, after merge, you can reference the master branch of jscs if you need immediate access to the fix. |
|
@ljharb Landed, thanks for patience 😼 |
|
Thank you!!! I'll wait quietly for the publish :-) |
...entifier
Fixes #1203