fix #884 #914
Closed
fix #884 #914
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yannickcr
requested changes
Nov 1, 2016
| @@ -1277,6 +1277,9 @@ ruleTester.run('prop-types', rule, { | |||
| ' return names.map((name) => {', | |||
| ' return <div>{name}</div>;', | |||
| ' });', | |||
| '}', | |||
| 'Hello.propTypes = {', | |||
| ' names: React.PropTypes.string', | |||
There was a problem hiding this comment.
Here Hello is not a component (it returns an Array, so no propTypes should be specified)
|
any update here? would be happy to help out if needed. |
|
Sorry, I never found the time to address the change requests and to see this through to merging. And I probably won't any time soon. If you want to completely take this over and see it through to merging, feel free! |
|
sure! |
|
Is there something I can do to let you "take over" this PR? Or maybe should I close this one and let you open another one? |
|
yeah, that makes the most sense to me. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
The most straightforward way of fixing #884 would be to add a third condition to
isPropTypesUsagethat checks if it's anextPropsorprevPropsin a lifecycle method. If I did this, then I'd also have to addnextPropsandprevPropstoDIRECT_PROPS_REGEX. I didn't want to do that because it duplicates logic betweenDIRECT_PROPS_REGEXandisPropAttributeName.So this PR removes the need for
DIRECT_PROPS_REGEX.The main idea is that before this PR, the
MemberExpressioncallback caught direct access (props.foo) and ThisExpression access (this.props.foo) at different "levels" in the AST. This meant thatgetPropertyNamehad to have some logic to fix them up to be on the same level. I added aIdentifercallback that catches direct access at the same level asMemberExpressioncatches ThisExpression access so thatgetPropertyNameno longer needs that logic.This change uncovered something that I think is a bug in Components.js that made one of the existing no-unused-prop-types tests start failing, so I fixed that too.