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 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 jomasti:feature/support-react-forwardref-memo branch from efb7ee5 to 341bb4f Dec 16, 2018
if (node.type !== 'CallExpression') {
return false;
}
const propertyNames = ['forwardRef', 'memo'];

This comment has been minimized.

Copy link
@ljharb

ljharb Dec 16, 2018

Collaborator

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

This comment has been minimized.

Copy link
@jomasti

jomasti Dec 16, 2018

Author Contributor

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

This comment has been minimized.

Copy link
@ljharb

ljharb Dec 16, 2018

Collaborator

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

This comment has been minimized.

Copy link
@jomasti

jomasti Dec 16, 2018

Author Contributor

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.

This comment has been minimized.

Copy link
@ljharb

ljharb Dec 16, 2018

Collaborator

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
@jomasti jomasti force-pushed the jomasti:feature/support-react-forwardref-memo branch from 4704762 to 2010245 Dec 16, 2018
lib/util/Components.js Outdated Show resolved Hide resolved
Also, the detection actually works now.
@jomasti jomasti force-pushed the jomasti:feature/support-react-forwardref-memo branch from 2010245 to 3ce2078 Dec 17, 2018
@ljharb
ljharb approved these changes Dec 27, 2018
@ljharb ljharb requested review from yannickcr and EvHaus Dec 27, 2018
@EvHaus
EvHaus approved these changes Dec 27, 2018
Copy link
Collaborator

EvHaus left a comment

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

@ljharb ljharb merged commit 8c6a8e2 into yannickcr:master Dec 27, 2018
3 checks passed
3 checks passed
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
coverage/coveralls Coverage increased (+0.007%) to 97.481%
Details
@renovate renovate bot mentioned this pull request Dec 28, 2018
0 of 1 task complete
@renovate-bot renovate-bot mentioned this pull request Dec 28, 2018
0 of 1 task complete
This was referenced Jan 4, 2019
@ibf-renovate ibf-renovate mentioned this pull request Jan 12, 2019
0 of 1 task complete
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

3 participants
You can’t perform that action at this time.