Skip to content
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

Fixes a few false positive failures in the no-unused-prop-types rule #792

Merged

Conversation

EvHaus
Copy link
Collaborator

@EvHaus EvHaus commented Aug 29, 2016

Fixes several false positives failures in the no-unused-prop-types rules:

' }',
' constructor (props) {',
' super(props);',
' const {something} = props;',
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we also test all these cases with the destructuring being done inline in the signature? ie, constructor ({ something }) {?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Done!

@EvHaus EvHaus force-pushed the no-unused-prop-types-fix-for-lifecycles branch from 070b489 to 42ce491 Compare August 30, 2016 16:19
@EvHaus EvHaus force-pushed the no-unused-prop-types-fix-for-lifecycles branch from 42ce491 to 3d09075 Compare August 30, 2016 16:20
@ljharb ljharb added the bug label Aug 30, 2016
@ljharb
Copy link
Member

ljharb commented Aug 30, 2016

Test cases LGTM.

@yannickcr
Copy link
Member

Merged, thanks!

@EvHaus EvHaus deleted the no-unused-prop-types-fix-for-lifecycles branch September 13, 2016 18:29
@quanru
Copy link

quanru commented Dec 20, 2016

@EvNaverniouk hi, i wonder know why must i use destructured props:
'Ensures destructured props in component lifecycle methods are properly counted towards used props'

@ljharb
Copy link
Member

ljharb commented Dec 20, 2016

You don't have to; this just ensures it works if you do.

@quanru
Copy link

quanru commented Dec 21, 2016

@ljharb if i don't do it in this way, it goes wrong. is this config correct ?
'no-unused-prop-types': [2, { customValidators: ['nextProps'], skipShapeProps: true }],

@ismay
Copy link

ismay commented Feb 22, 2017

Using nextProps with an object spread operator doesn't prevent the no-unused-proptypes error (even though all props are used), I assume that's by design? (Since it's still only a proposal)

@ljharb
Copy link
Member

ljharb commented Feb 22, 2017

The rule has no way of knowing what props you are using there because it can't know what properties are on it at runtime. Using object spread is dynamic; prefer explicit prop referencing.

@ismay
Copy link

ismay commented Feb 22, 2017

Ah yeah of course. That makes sense.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

5 participants