Skip to content

Commit 30ae98b

Browse files
committed
[Fix] no-typos: prevent crash on styled components and forwardRefs
Fixes #3036.
1 parent 23a9e84 commit 30ae98b

File tree

7 files changed

+59
-17
lines changed

7 files changed

+59
-17
lines changed

CHANGELOG.md

+2
Original file line numberDiff line numberDiff line change
@@ -14,13 +14,15 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
1414
* [`destructuring-assignment`], [`no-multi-comp`], [`no-unstable-nested-components`], component detection: improve component detection ([#3001][] @vedadeepta)
1515
* [`no-deprecated`]: fix crash on rest elements ([#3016][] @ljharb)
1616
* [`destructuring-assignment`]: get the contextName correctly ([#3025][] @ohhoney1)
17+
* [`no-typos`]: prevent crash on styled components and forwardRefs ([#3036][] @ljharb)
1718

1819
### Changed
1920
* [Docs] [`jsx-no-bind`]: updates discussion of refs ([#2998][] @dimitropoulos)
2021
* [Refactor] `utils/Components`: correct spelling and delete unused code ([#3026][] @ohhoney1)
2122
* [Docs] [`jsx-uses-react`], [`react-in-jsx-scope`]: document [`react/jsx-runtime`] config ([#3018][] @pkuczynski @ljharb)
2223
* [Docs] [`require-default-props`]: fix small typo ([#2994][] @evsasse)
2324

25+
[#3036]: https://github.com/yannickcr/eslint-plugin-react/issues/3036
2426
[#3026]: https://github.com/yannickcr/eslint-plugin-react/pull/3026
2527
[#3025]: https://github.com/yannickcr/eslint-plugin-react/pull/3025
2628
[#3018]: https://github.com/yannickcr/eslint-plugin-react/pull/3018

lib/util/Components.js

+2-2
Original file line numberDiff line numberDiff line change
@@ -410,11 +410,11 @@ function componentRule(rule, context) {
410410
},
411411

412412
isReturningJSX(ASTNode, strict) {
413-
return jsxUtil.isReturningJSX(this.isCreateElement.bind(this), ASTNode, strict, true);
413+
return jsxUtil.isReturningJSX(this.isCreateElement.bind(this), ASTNode, context, strict, true);
414414
},
415415

416416
isReturningJSXOrNull(ASTNode, strict) {
417-
return jsxUtil.isReturningJSX(this.isCreateElement.bind(this), ASTNode, strict);
417+
return jsxUtil.isReturningJSX(this.isCreateElement.bind(this), ASTNode, context, strict);
418418
},
419419

420420
getPragmaComponentWrapper(node) {

lib/util/ast.js

+27-6
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@
55
'use strict';
66

77
const estraverse = require('estraverse');
8+
// const pragmaUtil = require('./pragma');
89

910
/**
1011
* Wrapper for estraverse.traverse
@@ -65,10 +66,11 @@ function findReturnStatement(node) {
6566
* returned expression in the case of an arrow function) of a function
6667
*
6768
* @param {ASTNode} ASTNode The AST node being checked
69+
* @param {Context} context The context of `ASTNode`.
6870
* @param {function} enterFunc Function to execute for each returnStatement found
6971
* @returns {undefined}
7072
*/
71-
function traverseReturns(ASTNode, enterFunc) {
73+
function traverseReturns(ASTNode, context, enterFunc) {
7274
const nodeType = ASTNode.type;
7375

7476
if (nodeType === 'ReturnStatement') {
@@ -79,12 +81,31 @@ function traverseReturns(ASTNode, enterFunc) {
7981
return enterFunc(ASTNode.body);
8082
}
8183

82-
if (nodeType !== 'FunctionExpression'
83-
&& nodeType !== 'FunctionDeclaration'
84-
&& nodeType !== 'ArrowFunctionExpression'
85-
&& nodeType !== 'MethodDefinition'
84+
/* TODO: properly warn on React.forwardRefs having typo properties
85+
if (nodeType === 'CallExpression') {
86+
const callee = ASTNode.callee;
87+
const pragma = pragmaUtil.getFromContext(context);
88+
if (
89+
callee.type === 'MemberExpression'
90+
&& callee.object.type === 'Identifier'
91+
&& callee.object.name === pragma
92+
&& callee.property.type === 'Identifier'
93+
&& callee.property.name === 'forwardRef'
94+
&& ASTNode.arguments.length > 0
95+
) {
96+
return enterFunc(ASTNode.arguments[0]);
97+
}
98+
return;
99+
}
100+
*/
101+
102+
if (
103+
nodeType !== 'FunctionExpression'
104+
&& nodeType !== 'FunctionDeclaration'
105+
&& nodeType !== 'ArrowFunctionExpression'
106+
&& nodeType !== 'MethodDefinition'
86107
) {
87-
throw new TypeError('only function nodes are expected');
108+
return;
88109
}
89110

90111
traverse(ASTNode.body, {

lib/util/jsx.js

+3-2
Original file line numberDiff line numberDiff line change
@@ -88,13 +88,14 @@ function isWhiteSpaces(value) {
8888
* @param {Function} isCreateElement Function to determine if a CallExpresion is
8989
* a createElement one
9090
* @param {ASTNode} ASTnode The AST node being checked
91+
* @param {Context} context The context of `ASTNode`.
9192
* @param {Boolean} [strict] If true, in a ternary condition the node must return JSX in both cases
9293
* @param {Boolean} [ignoreNull] If true, null return values will be ignored
9394
* @returns {Boolean} True if the node is returning JSX or null, false if not
9495
*/
95-
function isReturningJSX(isCreateElement, ASTnode, strict, ignoreNull) {
96+
function isReturningJSX(isCreateElement, ASTnode, context, strict, ignoreNull) {
9697
let found = false;
97-
astUtil.traverseReturns(ASTnode, (node) => {
98+
astUtil.traverseReturns(ASTnode, context, (node) => {
9899
// Traverse return statement
99100
astUtil.traverse(node, {
100101
enter(childNode) {

tests/lib/rules/no-typos.js

+14
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,20 @@ ruleTester.run('no-typos', rule, {
642642
}
643643
`,
644644
parserOptions
645+
}, {
646+
code: `
647+
const MyComponent = React.forwardRef((props, ref) => <div />);
648+
MyComponent.defaultProps = { value: "" };
649+
`,
650+
parserOptions
651+
}, {
652+
code: `
653+
import styled from "styled-components";
654+
655+
const MyComponent = styled.div;
656+
MyComponent.defaultProps = { value: "" };
657+
`,
658+
parserOptions
645659
}),
646660

647661
invalid: [].concat({

tests/util/ast.js

+7-5
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@ const parseCode = (code) => {
1818
return ASTnode.body[0];
1919
};
2020

21+
const mockContext = {};
22+
2123
describe('ast', () => {
2224
describe('traverseReturnStatements', () => {
2325
it('Correctly traverses function declarations', () => {
@@ -26,7 +28,7 @@ describe('ast', () => {
2628
function foo({prop}) {
2729
return;
2830
}
29-
`), spy);
31+
`), mockContext, spy);
3032

3133
assert(spy.calledOnce);
3234
});
@@ -37,7 +39,7 @@ describe('ast', () => {
3739
const foo = function({prop}) {
3840
return;
3941
}
40-
`).declarations[0].init, spy);
42+
`).declarations[0].init, mockContext, spy);
4143

4244
assert(spy.calledOnce);
4345
});
@@ -48,15 +50,15 @@ describe('ast', () => {
4850
({prop}) => {
4951
return;
5052
}
51-
`).expression, spy);
53+
`).expression, mockContext, spy);
5254

5355
assert(spy.calledOnce);
5456

5557
spy.resetHistory();
5658

5759
traverseReturns(parseCode(`
5860
({prop}) => 'someething'
59-
`).expression, spy);
61+
`).expression, mockContext, spy);
6062

6163
assert(spy.calledOnce);
6264
});
@@ -88,7 +90,7 @@ describe('ast', () => {
8890
8991
const foo = () => 'not valid';
9092
}
91-
`), spy);
93+
`), mockContext, spy);
9294

9395
const enterCalls = spy.getCalls();
9496

tests/util/jsx.js

+4-2
Original file line numberDiff line numberDiff line change
@@ -20,13 +20,15 @@ const parseCode = (code) => {
2020
return ASTnode.body[0];
2121
};
2222

23+
const mockContext = {};
24+
2325
describe('jsxUtil', () => {
2426
describe('isReturningJSX', () => {
2527
const assertValid = (codeStr) => assert(
26-
isReturningJSX(() => false, parseCode(codeStr))
28+
isReturningJSX(() => false, parseCode(codeStr), mockContext)
2729
);
2830
const assertInValid = (codeStr) => assert(
29-
!!isReturningJSX(() => false, parseCode(codeStr))
31+
!!isReturningJSX(() => false, parseCode(codeStr), mockContext)
3032
);
3133
it('Works when returning JSX', () => {
3234
assertValid(`

0 commit comments

Comments
 (0)