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

fix: bail when function returns are recursive instead of stack overflowing #407

Merged
merged 1 commit into from Dec 11, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
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 @@ -35,7 +35,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 @@ -47,17 +54,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 @@ -71,7 +78,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 @@ -112,7 +119,7 @@ function resolvesToJSXElementOrReactCall(path) {

if (
!resolvedMemberExpression ||
returnsJSXElementOrReactCall(resolvedMemberExpression)
returnsJSXElementOrReactCall(resolvedMemberExpression, seen)
) {
return true;
}
Expand All @@ -122,14 +129,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 @@ -145,7 +152,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