Skip to content

Commit

Permalink
[Fix] no-typos: prevent crash on styled components and forwardRefs
Browse files Browse the repository at this point in the history
Fixes #3036.
  • Loading branch information
ljharb committed Aug 9, 2021
1 parent 23a9e84 commit 30ae98b
Show file tree
Hide file tree
Showing 7 changed files with 59 additions and 17 deletions.
2 changes: 2 additions & 0 deletions CHANGELOG.md
Expand Up @@ -14,13 +14,15 @@ This change log adheres to standards from [Keep a CHANGELOG](http://keepachangel
* [`destructuring-assignment`], [`no-multi-comp`], [`no-unstable-nested-components`], component detection: improve component detection ([#3001][] @vedadeepta)
* [`no-deprecated`]: fix crash on rest elements ([#3016][] @ljharb)
* [`destructuring-assignment`]: get the contextName correctly ([#3025][] @ohhoney1)
* [`no-typos`]: prevent crash on styled components and forwardRefs ([#3036][] @ljharb)

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

[#3036]: https://github.com/yannickcr/eslint-plugin-react/issues/3036
[#3026]: https://github.com/yannickcr/eslint-plugin-react/pull/3026
[#3025]: https://github.com/yannickcr/eslint-plugin-react/pull/3025
[#3018]: https://github.com/yannickcr/eslint-plugin-react/pull/3018
Expand Down
4 changes: 2 additions & 2 deletions lib/util/Components.js
Expand Up @@ -410,11 +410,11 @@ function componentRule(rule, context) {
},

isReturningJSX(ASTNode, strict) {
return jsxUtil.isReturningJSX(this.isCreateElement.bind(this), ASTNode, strict, true);
return jsxUtil.isReturningJSX(this.isCreateElement.bind(this), ASTNode, context, strict, true);
},

isReturningJSXOrNull(ASTNode, strict) {
return jsxUtil.isReturningJSX(this.isCreateElement.bind(this), ASTNode, strict);
return jsxUtil.isReturningJSX(this.isCreateElement.bind(this), ASTNode, context, strict);
},

getPragmaComponentWrapper(node) {
Expand Down
33 changes: 27 additions & 6 deletions lib/util/ast.js
Expand Up @@ -5,6 +5,7 @@
'use strict';

const estraverse = require('estraverse');
// const pragmaUtil = require('./pragma');

/**
* Wrapper for estraverse.traverse
Expand Down Expand Up @@ -65,10 +66,11 @@ function findReturnStatement(node) {
* returned expression in the case of an arrow function) of a function
*
* @param {ASTNode} ASTNode The AST node being checked
* @param {Context} context The context of `ASTNode`.
* @param {function} enterFunc Function to execute for each returnStatement found
* @returns {undefined}
*/
function traverseReturns(ASTNode, enterFunc) {
function traverseReturns(ASTNode, context, enterFunc) {
const nodeType = ASTNode.type;

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

if (nodeType !== 'FunctionExpression'
&& nodeType !== 'FunctionDeclaration'
&& nodeType !== 'ArrowFunctionExpression'
&& nodeType !== 'MethodDefinition'
/* TODO: properly warn on React.forwardRefs having typo properties
if (nodeType === 'CallExpression') {
const callee = ASTNode.callee;
const pragma = pragmaUtil.getFromContext(context);
if (
callee.type === 'MemberExpression'
&& callee.object.type === 'Identifier'
&& callee.object.name === pragma
&& callee.property.type === 'Identifier'
&& callee.property.name === 'forwardRef'
&& ASTNode.arguments.length > 0
) {
return enterFunc(ASTNode.arguments[0]);
}
return;
}
*/

if (
nodeType !== 'FunctionExpression'
&& nodeType !== 'FunctionDeclaration'
&& nodeType !== 'ArrowFunctionExpression'
&& nodeType !== 'MethodDefinition'
) {
throw new TypeError('only function nodes are expected');
return;
}

traverse(ASTNode.body, {
Expand Down
5 changes: 3 additions & 2 deletions lib/util/jsx.js
Expand Up @@ -88,13 +88,14 @@ function isWhiteSpaces(value) {
* @param {Function} isCreateElement Function to determine if a CallExpresion is
* a createElement one
* @param {ASTNode} ASTnode The AST node being checked
* @param {Context} context The context of `ASTNode`.
* @param {Boolean} [strict] If true, in a ternary condition the node must return JSX in both cases
* @param {Boolean} [ignoreNull] If true, null return values will be ignored
* @returns {Boolean} True if the node is returning JSX or null, false if not
*/
function isReturningJSX(isCreateElement, ASTnode, strict, ignoreNull) {
function isReturningJSX(isCreateElement, ASTnode, context, strict, ignoreNull) {
let found = false;
astUtil.traverseReturns(ASTnode, (node) => {
astUtil.traverseReturns(ASTnode, context, (node) => {
// Traverse return statement
astUtil.traverse(node, {
enter(childNode) {
Expand Down
14 changes: 14 additions & 0 deletions tests/lib/rules/no-typos.js
Expand Up @@ -642,6 +642,20 @@ ruleTester.run('no-typos', rule, {
}
`,
parserOptions
}, {
code: `
const MyComponent = React.forwardRef((props, ref) => <div />);
MyComponent.defaultProps = { value: "" };
`,
parserOptions
}, {
code: `
import styled from "styled-components";
const MyComponent = styled.div;
MyComponent.defaultProps = { value: "" };
`,
parserOptions
}),

invalid: [].concat({
Expand Down
12 changes: 7 additions & 5 deletions tests/util/ast.js
Expand Up @@ -18,6 +18,8 @@ const parseCode = (code) => {
return ASTnode.body[0];
};

const mockContext = {};

describe('ast', () => {
describe('traverseReturnStatements', () => {
it('Correctly traverses function declarations', () => {
Expand All @@ -26,7 +28,7 @@ describe('ast', () => {
function foo({prop}) {
return;
}
`), spy);
`), mockContext, spy);

assert(spy.calledOnce);
});
Expand All @@ -37,7 +39,7 @@ describe('ast', () => {
const foo = function({prop}) {
return;
}
`).declarations[0].init, spy);
`).declarations[0].init, mockContext, spy);

assert(spy.calledOnce);
});
Expand All @@ -48,15 +50,15 @@ describe('ast', () => {
({prop}) => {
return;
}
`).expression, spy);
`).expression, mockContext, spy);

assert(spy.calledOnce);

spy.resetHistory();

traverseReturns(parseCode(`
({prop}) => 'someething'
`).expression, spy);
`).expression, mockContext, spy);

assert(spy.calledOnce);
});
Expand Down Expand Up @@ -88,7 +90,7 @@ describe('ast', () => {
const foo = () => 'not valid';
}
`), spy);
`), mockContext, spy);

const enterCalls = spy.getCalls();

Expand Down
6 changes: 4 additions & 2 deletions tests/util/jsx.js
Expand Up @@ -20,13 +20,15 @@ const parseCode = (code) => {
return ASTnode.body[0];
};

const mockContext = {};

describe('jsxUtil', () => {
describe('isReturningJSX', () => {
const assertValid = (codeStr) => assert(
isReturningJSX(() => false, parseCode(codeStr))
isReturningJSX(() => false, parseCode(codeStr), mockContext)
);
const assertInValid = (codeStr) => assert(
!!isReturningJSX(() => false, parseCode(codeStr))
!!isReturningJSX(() => false, parseCode(codeStr), mockContext)
);
it('Works when returning JSX', () => {
assertValid(`
Expand Down

0 comments on commit 30ae98b

Please sign in to comment.