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

Support detecting React.forwardRef/React.memo #2089

Merged

Conversation

jomasti
Copy link
Contributor

@jomasti jomasti commented Dec 16, 2018

This updates the component detection to allow for considering components
wrapped in either React.forwardRef or React.memo.

Resolves #1841. Resolves #2059

lib/util/Components.js Outdated Show resolved Hide resolved
This updates the component detection to allow for considering components
wrapped in either React.forwardRef or React.memo.
@jomasti jomasti force-pushed the feature/support-react-forwardref-memo branch from efb7ee5 to 341bb4f Compare December 16, 2018 06:47
if (node.type !== 'CallExpression') {
return false;
}
const propertyNames = ['forwardRef', 'memo'];
Copy link
Member

Choose a reason for hiding this comment

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

Should these kick in only when the React version setting is 16.3 and 16.6 respectively?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Should they? Would this break anything for people on earlier versions?

Copy link
Member

Choose a reason for hiding this comment

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

Probably not, no - but it might be weird to see a warning about a React feature that you can’t use yet.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This wouldn't show any warnings related to the features. It only improves the detection of components using them. So, if they aren't use the features, nothing happens, right? But I could see the case for not running the check on earlier versions.

Copy link
Member

Choose a reason for hiding this comment

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

Yeah that’s a fair point too. I’m not really sure whether it’s better to keep it simple and run the checks always, or to only run the checks when the version dictates.

lib/util/Components.js Outdated Show resolved Hide resolved
lib/util/Components.js Outdated Show resolved Hide resolved
Also, the detection actually works now.
@jomasti jomasti force-pushed the feature/support-react-forwardref-memo branch from 2010245 to 3ce2078 Compare December 17, 2018 03:58
Copy link
Collaborator

@EvHaus EvHaus left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks for the clean up on the lib utils.

@ljharb ljharb merged commit 8c6a8e2 into jsx-eslint:master Dec 27, 2018
This was referenced Jan 4, 2019
@ghost ghost mentioned this pull request Jan 12, 2019
1 task
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Development

Successfully merging this pull request may close these issues.

Add missing prop validation for React.memo jsx/prop-types doesn't trigger with React.forwardRef
3 participants