Skip to content

Commit

Permalink
fix: bail when function returns are recursive instead of stack overfl…
Browse files Browse the repository at this point in the history
…owing
  • Loading branch information
jquense authored and danez committed Dec 11, 2019
1 parent a2978e8 commit 13a8de9
Show file tree
Hide file tree
Showing 2 changed files with 28 additions and 11 deletions.
12 changes: 11 additions & 1 deletion src/utils/__tests__/isStatelessComponent-test.js
Expand Up @@ -6,7 +6,7 @@
*
*/

import { statement, parse } from '../../../tests/utils';
import { parse, statement } from '../../../tests/utils';
import isStatelessComponent from '../isStatelessComponent';

describe('isStatelessComponent', () => {
Expand Down Expand Up @@ -233,6 +233,16 @@ describe('isStatelessComponent', () => {
expect(isStatelessComponent(def)).toBe(true);
});

it('handles recursive function calls', () => {
const def = statement(`
function Foo (props) {
return props && Foo(props);
}
`);

expect(isStatelessComponent(def)).toBe(false);
});

test(
'handles simple resolves',
`
Expand Down
27 changes: 17 additions & 10 deletions src/utils/isStatelessComponent.js
Expand Up @@ -33,7 +33,14 @@ function isJSXElementOrReactCall(path) {
);
}

function resolvesToJSXElementOrReactCall(path) {
function resolvesToJSXElementOrReactCall(path, seen) {
// avoid returns with recursive function calls
if (seen.has(path)) {
return false;
}

seen.add(path);

// Is the path is already a JSX element or a call to one of the React.* functions
if (isJSXElementOrReactCall(path)) {
return true;
Expand All @@ -45,17 +52,17 @@ function resolvesToJSXElementOrReactCall(path) {
// the two possible paths
if (resolvedPath.node.type === 'ConditionalExpression') {
return (
resolvesToJSXElementOrReactCall(resolvedPath.get('consequent')) ||
resolvesToJSXElementOrReactCall(resolvedPath.get('alternate'))
resolvesToJSXElementOrReactCall(resolvedPath.get('consequent'), seen) ||
resolvesToJSXElementOrReactCall(resolvedPath.get('alternate'), seen)
);
}

// If the path points to a logical expression (AND, OR, ...), then we need to look only at
// the two possible paths
if (resolvedPath.node.type === 'LogicalExpression') {
return (
resolvesToJSXElementOrReactCall(resolvedPath.get('left')) ||
resolvesToJSXElementOrReactCall(resolvedPath.get('right'))
resolvesToJSXElementOrReactCall(resolvedPath.get('left'), seen) ||
resolvesToJSXElementOrReactCall(resolvedPath.get('right'), seen)
);
}

Expand All @@ -69,7 +76,7 @@ function resolvesToJSXElementOrReactCall(path) {
if (resolvedPath.node.type === 'CallExpression') {
let calleeValue = resolveToValue(resolvedPath.get('callee'));

if (returnsJSXElementOrReactCall(calleeValue)) {
if (returnsJSXElementOrReactCall(calleeValue, seen)) {
return true;
}

Expand Down Expand Up @@ -110,7 +117,7 @@ function resolvesToJSXElementOrReactCall(path) {

if (
!resolvedMemberExpression ||
returnsJSXElementOrReactCall(resolvedMemberExpression)
returnsJSXElementOrReactCall(resolvedMemberExpression, seen)
) {
return true;
}
Expand All @@ -120,14 +127,14 @@ function resolvesToJSXElementOrReactCall(path) {
return false;
}

function returnsJSXElementOrReactCall(path) {
function returnsJSXElementOrReactCall(path, seen = new WeakSet()) {
let visited = false;

// early exit for ArrowFunctionExpressions
if (
path.node.type === 'ArrowFunctionExpression' &&
path.get('body').node.type !== 'BlockStatement' &&
resolvesToJSXElementOrReactCall(path.get('body'))
resolvesToJSXElementOrReactCall(path.get('body'), seen)
) {
return true;
}
Expand All @@ -143,7 +150,7 @@ function returnsJSXElementOrReactCall(path) {
// Only check return statements which are part of the checked function scope
if (returnPath.scope !== scope) return false;

if (resolvesToJSXElementOrReactCall(returnPath.get('argument'))) {
if (resolvesToJSXElementOrReactCall(returnPath.get('argument'), seen)) {
visited = true;
return false;
}
Expand Down

0 comments on commit 13a8de9

Please sign in to comment.