-
-
Notifications
You must be signed in to change notification settings - Fork 2.8k
Add forbidPatterns to forbid-component-props rule #936
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
base: master
Are you sure you want to change the base?
Add forbidPatterns to forbid-component-props rule #936
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.
I like the idea of forbidPatterns. I very much dislike the idea of feeding the string into RegExp.
Adds a forbidPatterns option that takes an array of safe pattern strings so it's possible to forbid the use of things like "data-*" props. Also adds the ignoreDomNodes option (defaults to true), because sometimes we do want to check forbidden props on DOM nodes i.e. for "data-*" so we can avoid tying implementation to thirdparty libraries that work on the DOM.
098a123
to
3deeb99
Compare
all changes done. |
|
||
### `ignoreComponents` | ||
|
||
Whether to skip checking forbidden props on Components (div, input, span, etc...). Defaults to false. |
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.
these examples are DOM nodes, not Components
var tag = node.parent.name.name; | ||
if (tag && tag[0] !== tag[0].toUpperCase()) { | ||
// This is a DOM node, not a Component, so exit. | ||
var isComponent = tag && tag[0] === tag[0].toUpperCase(); |
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 think your code here is making the false assumption that there's only two kinds of nodes, DOM and Component. nodes can also be strings, numbers, arrays… are we sure this is the right way to check "is dom node" and "is component"? I feel like we have functions for this in the codebase already.
|
||
Whether to check forbidden props on DOM nodes (div, input, span, etc...). Defaults to false. | ||
|
||
### `ignoreComponents` |
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 dont have time to refactor according to #850. if anyone want to do it please do =) |
59af733
to
865ed16
Compare
069314a
to
181c68f
Compare
380e32c
to
51d342b
Compare
Adds a forbidPatterns option that takes an array of regexp strings so it's possible to forbid the use of things like
data-*
props