Add forbidDefaultForRequired option #1532
Conversation
New option for require-default-props rule that forbids defining default props for props that are required.
| ... | ||
| ``` | ||
|
|
||
| * `enabled`: for enabling the rule. 0=off, 1=warn, 2=error. Defaults to 0. |
There was a problem hiding this comment.
This is basic eslint documentation; individual rule docs don't tend to explain this.
There was a problem hiding this comment.
I just modified this block from another rule. Should I remove it?
There was a problem hiding this comment.
Not a blocker then, altho i don’t think any rules should have this.
|
cc @sharmilajesupaul; a review would be great :-) |
There was a problem hiding this comment.
just have a question about how this will behave with object spreads, otherwise lgtm pending @ljharb's comments
| ].join('\n'), | ||
| options: [{forbidDefaultForRequired: true}], | ||
| errors: [{ | ||
| message: 'propType "foo" is required and should not have a defaultProp declaration.' |
There was a problem hiding this comment.
Will this also warn on the case where a spread property has an object literal referenced in the same file?
eg:
const defaults = {
foo: ''
}
class Hello extends React.Component {
static propTypes = {
foo: PropTypes.string.isRequired
}
static defaultProps = { ...defaults } // <---
render () {
return <div>Hello {this.props.foo}</div>;
}
}There was a problem hiding this comment.
nvm read your pr discription
|
I created #1535 for tracking the spread property enhancement. |
|
Will merge this tomorrow if no objections. |
New option for require-default-props rule that forbids defining default
props for props that are required.
This doesn't handle spread properties since the work needed should also work for the default state of the rule. If that were to happen, it could cause reports for previously valid code, and that seems more suitable for a major version bump. Therefore, I think that should be in a separate PR.
Closes #1524