Change the reporting for no-unused-prop-types #2292
Conversation
… accurate node Fixes jsx-eslint#2287
| @@ -104,7 +104,7 @@ module.exports = { | |||
|
|
|||
| if (prop.node && !isPropUsed(component, prop)) { | |||
| context.report({ | |||
| node: prop.node.value || prop.node, | |||
| node: prop.node.key || prop.node, | |||
There was a problem hiding this comment.
I wonder if the fallback to prop.node here is actually needed? If the fallback is used then it will report like this:
There was a problem hiding this comment.
Maybe it is for flow object type? It seems that babel-eslint does not provide key in ObjectTypeProperty nodes demo.
|
@jseminck it'd be great to add a test case that fails when the |
|
@jseminck are you no longer interested in completing this PR? |
There are already 34+ test cases that will fail when prop.node fallback is removed. The prop.node fallback is needed for propTypes written with flow types. This pr is ready to go after a rebase and fixing a test case from master. |
ljharb
force-pushed
the
change-reporting-for-no-unused-prop-types
branch
2 times, most recently
from
e2f4c2c
to
b833535
Compare
Fixes #2287
This change seems rather trivial, which made me wonder if there was ever a reason why it was not done this way? 🤔
The code used to report only on
prop.nodevalue, then it would report like this:However, this logic was changed in this commit to report only on the
node.valueinstead. But this commit doesn't really explain why and seems mostly about another rule instead. I'm thinking that it may have been done by accident?2e4f91a
So now I question whether we should report back to the full
nodeor change it tonode.key.Otherwise, I tried it out on a local project and seemed to work fine, all tests also continue to pass once the column context values were adjusted.
Before:
After:
