Skip to content

Commit 8c6a8e2

Browse files
ljharbljharb
authored
Merge pull request #2089 from jomasti/feature/support-react-forwardref-memo
Support detecting React.forwardRef/React.memo
2 parents 14451d4 + 3ce2078 commit 8c6a8e2

File tree

4 files changed

+272
-21
lines changed

4 files changed

+272
-21
lines changed

‎lib/rules/void-dom-elements-no-children.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,7 @@ module.exports = {
9999
return;
100100
}
101101

102-
if (!utils.isReactCreateElement(node)) {
102+
if (!utils.isCreateElement(node)) {
103103
return;
104104
}
105105

‎lib/util/Components.js

Lines changed: 42 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@
66

77
const util = require('util');
88
const doctrine = require('doctrine');
9+
const arrayIncludes = require('array-includes');
10+
911
const variableUtil = require('./variable');
1012
const pragmaUtil = require('./pragma');
1113
const astUtil = require('./ast');
@@ -253,34 +255,33 @@ function componentRule(rule, context) {
253255
},
254256

255257
/**
256-
* Check if createElement is destructured from React import
258+
* Check if variable is destructured from pragma import
257259
*
258-
* @returns {Boolean} True if createElement is destructured from React
260+
* @param {variable} String The variable name to check
261+
* @returns {Boolean} True if createElement is destructured from the pragma
259262
*/
260-
hasDestructuredReactCreateElement: function() {
263+
isDestructuredFromPragmaImport: function(variable) {
261264
const variables = variableUtil.variablesInScope(context);
262-
const variable = variableUtil.getVariable(variables, 'createElement');
263-
if (variable) {
264-
const map = variable.scope.set;
265-
if (map.has('React')) {
266-
return true;
267-
}
265+
const variableInScope = variableUtil.getVariable(variables, variable);
266+
if (variableInScope) {
267+
const map = variableInScope.scope.set;
268+
return map.has(pragma);
268269
}
269270
return false;
270271
},
271272

272273
/**
273-
* Checks to see if node is called within React.createElement
274+
* Checks to see if node is called within createElement from pragma
274275
*
275276
* @param {ASTNode} node The AST node being checked.
276-
* @returns {Boolean} True if React.createElement called
277+
* @returns {Boolean} True if createElement called from pragma
277278
*/
278-
isReactCreateElement: function(node) {
279-
const calledOnReact = (
279+
isCreateElement: function(node) {
280+
const calledOnPragma = (
280281
node &&
281282
node.callee &&
282283
node.callee.object &&
283-
node.callee.object.name === 'React' &&
284+
node.callee.object.name === pragma &&
284285
node.callee.property &&
285286
node.callee.property.name === 'createElement'
286287
);
@@ -291,10 +292,10 @@ function componentRule(rule, context) {
291292
node.callee.name === 'createElement'
292293
);
293294

294-
if (this.hasDestructuredReactCreateElement()) {
295-
return calledDirectly || calledOnReact;
295+
if (this.isDestructuredFromPragmaImport('createElement')) {
296+
return calledDirectly || calledOnPragma;
296297
}
297-
return calledOnReact;
298+
return calledOnPragma;
298299
},
299300

300301
getReturnPropertyAndNode(ASTnode) {
@@ -356,12 +357,12 @@ function componentRule(rule, context) {
356357
node[property] &&
357358
jsxUtil.isJSX(node[property])
358359
;
359-
const returnsReactCreateElement = this.isReactCreateElement(node[property]);
360+
const returnsPragmaCreateElement = this.isCreateElement(node[property]);
360361

361362
return Boolean(
362363
returnsConditionalJSX ||
363364
returnsJSX ||
364-
returnsReactCreateElement
365+
returnsPragmaCreateElement
365366
);
366367
},
367368

@@ -394,6 +395,18 @@ function componentRule(rule, context) {
394395
return utils.isReturningJSX(ASTNode, strict) || utils.isReturningNull(ASTNode);
395396
},
396397

398+
isPragmaComponentWrapper(node) {
399+
if (node.type !== 'CallExpression') {
400+
return false;
401+
}
402+
const propertyNames = ['forwardRef', 'memo'];
403+
const calleeObject = node.callee.object;
404+
if (calleeObject) {
405+
return arrayIncludes(propertyNames, node.callee.property.name) && node.callee.object.name === pragma;
406+
}
407+
return arrayIncludes(propertyNames, node.callee.name) && this.isDestructuredFromPragmaImport(node.callee.name);
408+
},
409+
397410
/**
398411
* Find a return statment in the current node
399412
*
@@ -466,6 +479,9 @@ function componentRule(rule, context) {
466479
const isArgument = node.parent && node.parent.type === 'CallExpression'; // Arguments (callback, etc.)
467480
// Attribute Expressions inside JSX Elements (<button onClick={() => props.handleClick()}></button>)
468481
const isJSXExpressionContainer = node.parent && node.parent.type === 'JSXExpressionContainer';
482+
if (node.parent && this.isPragmaComponentWrapper(node.parent)) {
483+
return node.parent;
484+
}
469485
// Stop moving up if we reach a class or an argument (like a callback)
470486
if (isClass || isArgument) {
471487
return null;
@@ -600,6 +616,13 @@ function componentRule(rule, context) {
600616

601617
// Component detection instructions
602618
const detectionInstructions = {
619+
CallExpression: function(node) {
620+
if (!utils.isPragmaComponentWrapper(node)) {
621+
return;
622+
}
623+
components.add(node, 2);
624+
},
625+
603626
ClassExpression: function(node) {
604627
if (!utils.isES6Component(node)) {
605628
return;

‎lib/util/usedPropTypes.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,7 +429,7 @@ module.exports = function usedPropTypesInstructions(context, components, utils)
429429
*/
430430
function markDestructuredFunctionArgumentsAsUsed(node) {
431431
const destructuring = node.params && node.params[0] && node.params[0].type === 'ObjectPattern';
432-
if (destructuring && components.get(node)) {
432+
if (destructuring && (components.get(node) || components.get(node.parent))) {
433433
markPropTypesAsUsed(node);
434434
}
435435
}

‎tests/lib/rules/prop-types.js

Lines changed: 228 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2066,6 +2066,110 @@ ruleTester.run('prop-types', rule, {
20662066
};
20672067
`,
20682068
settings: {react: {version: '16.3.0'}}
2069+
},
2070+
{
2071+
code: `
2072+
const HeaderBalance = React.memo(({ cryptoCurrency }) => (
2073+
<div className="header-balance">
2074+
<div className="header-balance__balance">
2075+
BTC
2076+
{cryptoCurrency}
2077+
</div>
2078+
</div>
2079+
));
2080+
HeaderBalance.propTypes = {
2081+
cryptoCurrency: PropTypes.string
2082+
};
2083+
`
2084+
},
2085+
{
2086+
code: `
2087+
import React, { memo } from 'react';
2088+
const HeaderBalance = memo(({ cryptoCurrency }) => (
2089+
<div className="header-balance">
2090+
<div className="header-balance__balance">
2091+
BTC
2092+
{cryptoCurrency}
2093+
</div>
2094+
</div>
2095+
));
2096+
HeaderBalance.propTypes = {
2097+
cryptoCurrency: PropTypes.string
2098+
};
2099+
`
2100+
},
2101+
{
2102+
code: `
2103+
import Foo, { memo } from 'foo';
2104+
const HeaderBalance = memo(({ cryptoCurrency }) => (
2105+
<div className="header-balance">
2106+
<div className="header-balance__balance">
2107+
BTC
2108+
{cryptoCurrency}
2109+
</div>
2110+
</div>
2111+
));
2112+
HeaderBalance.propTypes = {
2113+
cryptoCurrency: PropTypes.string
2114+
};
2115+
`,
2116+
settings: {
2117+
react: {
2118+
pragma: 'Foo'
2119+
}
2120+
}
2121+
},
2122+
{
2123+
code: `
2124+
const Label = React.forwardRef(({ text }, ref) => {
2125+
return <div ref={ref}>{text}</div>;
2126+
});
2127+
Label.propTypes = {
2128+
text: PropTypes.string,
2129+
};
2130+
`
2131+
},
2132+
{
2133+
code: `
2134+
const Label = Foo.forwardRef(({ text }, ref) => {
2135+
return <div ref={ref}>{text}</div>;
2136+
});
2137+
Label.propTypes = {
2138+
text: PropTypes.string,
2139+
};
2140+
`,
2141+
settings: {
2142+
react: {
2143+
pragma: 'Foo'
2144+
}
2145+
}
2146+
},
2147+
{
2148+
code: `
2149+
import React, { forwardRef } from 'react';
2150+
const Label = forwardRef(({ text }, ref) => {
2151+
return <div ref={ref}>{text}</div>;
2152+
});
2153+
Label.propTypes = {
2154+
text: PropTypes.string,
2155+
};
2156+
`
2157+
},
2158+
{
2159+
code: `
2160+
import Foo, { forwardRef } from 'foo';
2161+
const Label = forwardRef(({ text }, ref) => {
2162+
return <div ref={ref}>{text}</div>;
2163+
});
2164+
Label.propTypes = {
2165+
text: PropTypes.string,
2166+
};
2167+
`,
2168+
settings: {
2169+
react: {
2170+
pragma: 'Foo'
2171+
}
2172+
}
20692173
}
20702174
],
20712175

@@ -3947,6 +4051,130 @@ ruleTester.run('prop-types', rule, {
39474051
errors: [{
39484052
message: '\'page\' is missing in props validation'
39494053
}]
4054+
},
4055+
{
4056+
code: `
4057+
const HeaderBalance = React.memo(({ cryptoCurrency }) => (
4058+
<div className="header-balance">
4059+
<div className="header-balance__balance">
4060+
BTC
4061+
{cryptoCurrency}
4062+
</div>
4063+
</div>
4064+
));
4065+
`,
4066+
errors: [{
4067+
message: '\'cryptoCurrency\' is missing in props validation'
4068+
}]
4069+
},
4070+
{
4071+
code: `
4072+
import React, { memo } from 'react';
4073+
const HeaderBalance = memo(({ cryptoCurrency }) => (
4074+
<div className="header-balance">
4075+
<div className="header-balance__balance">
4076+
BTC
4077+
{cryptoCurrency}
4078+
</div>
4079+
</div>
4080+
));
4081+
`,
4082+
errors: [{
4083+
message: '\'cryptoCurrency\' is missing in props validation'
4084+
}]
4085+
},
4086+
{
4087+
code: `
4088+
const HeaderBalance = Foo.memo(({ cryptoCurrency }) => (
4089+
<div className="header-balance">
4090+
<div className="header-balance__balance">
4091+
BTC
4092+
{cryptoCurrency}
4093+
</div>
4094+
</div>
4095+
));
4096+
`,
4097+
settings: {
4098+
react: {
4099+
pragma: 'Foo'
4100+
}
4101+
},
4102+
errors: [{
4103+
message: '\'cryptoCurrency\' is missing in props validation'
4104+
}]
4105+
},
4106+
{
4107+
code: `
4108+
import Foo, { memo } from 'foo';
4109+
const HeaderBalance = memo(({ cryptoCurrency }) => (
4110+
<div className="header-balance">
4111+
<div className="header-balance__balance">
4112+
BTC
4113+
{cryptoCurrency}
4114+
</div>
4115+
</div>
4116+
));
4117+
`,
4118+
settings: {
4119+
react: {
4120+
pragma: 'Foo'
4121+
}
4122+
},
4123+
errors: [{
4124+
message: '\'cryptoCurrency\' is missing in props validation'
4125+
}]
4126+
},
4127+
{
4128+
code: `
4129+
const Label = React.forwardRef(({ text }, ref) => {
4130+
return <div ref={ref}>{text}</div>;
4131+
});
4132+
`,
4133+
errors: [{
4134+
message: '\'text\' is missing in props validation'
4135+
}]
4136+
},
4137+
{
4138+
code: `
4139+
import React, { forwardRef } from 'react';
4140+
const Label = forwardRef(({ text }, ref) => {
4141+
return <div ref={ref}>{text}</div>;
4142+
});
4143+
`,
4144+
errors: [{
4145+
message: '\'text\' is missing in props validation'
4146+
}]
4147+
},
4148+
{
4149+
code: `
4150+
const Label = Foo.forwardRef(({ text }, ref) => {
4151+
return <div ref={ref}>{text}</div>;
4152+
});
4153+
`,
4154+
settings: {
4155+
react: {
4156+
pragma: 'Foo'
4157+
}
4158+
},
4159+
errors: [{
4160+
message: '\'text\' is missing in props validation'
4161+
}]
4162+
},
4163+
{
4164+
code: `
4165+
import Foo, { forwardRef } from 'foo';
4166+
const Label = forwardRef(({ text }, ref) => {
4167+
return <div ref={ref}>{text}</div>;
4168+
});
4169+
`,
4170+
settings: {
4171+
react: {
4172+
pragma: 'Foo'
4173+
}
4174+
},
4175+
errors: [{
4176+
message: '\'text\' is missing in props validation'
4177+
}]
39504178
}
39514179
]
39524180
});

0 commit comments

Comments
 (0)