-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
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
Support propWrapperFunctions for boolean-prop-naming #1478
Conversation
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.
We definitely don't want to just support any function call. Take a look at propWrapperFunctions
for how this can be configured - we should probably add a new setting that defaults to ['Object.assign']
, such that users could define _.assign
or merge
etc themselves. It could be named propObjectAssignFunctions
perhaps?
Any progress on this? |
I think this is good based on the previous feedback, but @ljharb will have to review again. |
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.
This LGTM; although it occurs to me that with object spread, there’s no need for a merge/assign function anyways - is there a reason you aren’t using object spread?
@ljharb last I remembered ESLint supports node 4 which does not support object spread. I've had the same problems in the past. We still have Travis running builds on node 4 so I suppose we still have to support it. |
@jseminck I’m not asking about the rule implementation, I’m asking about users in their own codebase - also, object spread is trivially transpileable with Babel to Object.assign, or to any other package name via transform-replace-object-assign. |
@ljharb Now that I think about it, this change should just use the current |
@jomasti it could; but However, this rule deals only with "exact prop wrapper functions" - |
@ljharb You may be thinking about my other PR for Also, avoiding adding this should make it easy to change the schema of |
@jomasti oh lol i got confused, yes, i was thinking about the other one. Yes, this rule should just use propWrapperFunctions, good call :-D |
This adds support for checking prop types in
boolean-prop-naming
when using thepropWrapperFunctions
setting.Closes #1452.