Support lifecycle methods with nextProps/prevProps in no-unused-prop-types #1232
Conversation
The valid tests have two props, one is used in the render() function and the other in nextProps using the dot operator in the folllowing lifecycle methods: * shouldComponentUpdate * componentWillUpdate * componentDidUpdate (uses prevProps instead of nextProps). The invalid tests are the same as the valid tests but without the render() method. In here there should be an error that one of the props is unused, but there currently is no error.
* Extending the list of lifecycle methods * Checking also for the `prevProps` parameter name Currently for this rule to work, the user has to name the parameter either: * nextProps * prevProps * props
lib/rules/no-unused-prop-types.js
Outdated
| var scope = context.getScope(); | ||
| while (scope) { | ||
| if ( | ||
| scope.block && scope.block.parent && | ||
| scope.block.parent.key && scope.block.parent.key.name === 'componentWillReceiveProps' | ||
| scope.block.parent.key && | ||
| LIFE_CYCLE_METHODS.includes(scope.block.parent.key.name) |
There was a problem hiding this comment.
arrays don't have .includes until node 6; we need to use indexOf or https://npmjs.com/array-includes here.
|
Used I also fixed the invalid test cases, as the lines and column values were incorrect. Hopefully CI should be green now. Sorry for the problems! https://npmjs.com/array-includes would be nice to use, actually. There's a lot of places that use |
|
Adding a new dep is fine imo, but it should definitely be in a separate PR :-) |
|
Is there any change needed in the docs for this? It's for example important that the parameter names are correct (only works with |
|
Sure, it's worth making an additional PR for docs. |
#1213 #1142 (comment)
This makes the no-unused-prop-types work when a prop is used in React lifecycle components other than
componentWillReceivePropsusing the "dot-operator". E.g:There is one thing to keep in mind: under the current implementation the rule will only work if the parameter is named
nextProps,prevPropsorprops. This logic already existed and I did not want to touch it, though I thought it was kind of odd. Do we want to keep it that way?Perhaps we could introduce a rule that validates the parameter names in lifecycle methods - sometimes it's confusing anyway whether
shouldComponentUpdatehasprevPropsornextPropsas the parameter, and what is the order ofnextPropsornextState. Anyway, this would be for another issue.