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

Be more strict about detecting react class components #397

Merged
merged 1 commit into from Oct 25, 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
4 changes: 2 additions & 2 deletions src/handlers/__tests__/componentMethodsHandler-test.js
Expand Up @@ -94,7 +94,7 @@ describe('componentMethodsHandler', () => {

it('extracts the documentation for a ClassDeclaration', () => {
const src = `
class Test {
class Test extends React.Component {
/**
* The foo method
*/
Expand Down Expand Up @@ -129,7 +129,7 @@ describe('componentMethodsHandler', () => {

it('should handle and ignore computed methods', () => {
const src = `
class Test {
class Test extends React.Component {
/**
* The foo method
*/
Expand Down
6 changes: 3 additions & 3 deletions src/resolver/__tests__/findAllComponentDefinitions-test.js
Expand Up @@ -99,9 +99,9 @@ describe('findAllComponentDefinitions', () => {
const source = `
import React from 'React';
class ComponentA extends React.Component {}
class ComponentB { render() {} }
var ComponentC = class extends React.Component {}
var ComponentD = class { render() {} }
class ComponentB extends Foo { render() {} }
var ComponentC = class extends React.PureComponent {}
var ComponentD = class extends Bar { render() {} }
class NotAComponent {}
`;

Expand Down
56 changes: 52 additions & 4 deletions src/utils/__tests__/isReactComponentClass-test.js
Expand Up @@ -11,14 +11,14 @@ import isReactComponentClass from '../isReactComponentClass';

describe('isReactComponentClass', () => {
describe('render method', () => {
it('accepts class declarations with a render method', () => {
it('ignores class declarations with a render method without superclass', () => {
const def = statement('class Foo { render() {}}');
expect(isReactComponentClass(def)).toBe(true);
expect(isReactComponentClass(def)).toBe(false);
});

it('accepts class expression with a render method', () => {
it('ignores class expression with a render method without superclass', () => {
const def = expression('class { render() {}}');
expect(isReactComponentClass(def)).toBe(true);
expect(isReactComponentClass(def)).toBe(false);
});

it('ignores static render methods', () => {
Expand Down Expand Up @@ -102,4 +102,52 @@ describe('isReactComponentClass', () => {
expect(isReactComponentClass(def)).toBe(true);
});
});

describe('React.PureComponent inheritance', () => {
it('accepts class declarations extending React.PureComponent', () => {
const def = parse(`
var React = require('react');
class Foo extends React.PureComponent {}
`).get('body', 1);

expect(isReactComponentClass(def)).toBe(true);
});

it('accepts class expressions extending React.PureComponent', () => {
const def = parse(`
var React = require('react');
var Foo = class extends React.PureComponent {}
`).get('body', 1, 'declarations', 0, 'init');

expect(isReactComponentClass(def)).toBe(true);
});

it('resolves the super class reference', () => {
const def = parse(`
var {PureComponent} = require('react');
var C = PureComponent;
class Foo extends C {}
`).get('body', 2);

expect(isReactComponentClass(def)).toBe(true);
});

it('does not accept references to other modules', () => {
const def = parse(`
var {PureComponent} = require('FakeReact');
class Foo extends PureComponent {}
`).get('body', 1);

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

it('does not consider super class if render method is present', () => {
const def = parse(`
var {PureComponent} = require('FakeReact');
class Foo extends PureComponent { render() {} }
`).get('body', 1);

expect(isReactComponentClass(def)).toBe(true);
});
});
});
30 changes: 19 additions & 11 deletions src/utils/isReactComponentClass.js
Expand Up @@ -28,14 +28,31 @@ function isRenderMethod(node) {

/**
* Returns `true` of the path represents a class definition which either extends
* `React.Component` or implements a `render()` method.
* `React.Component` or has a superclass and implements a `render()` method.
*/
export default function isReactComponentClass(path: NodePath): boolean {
const node = path.node;
if (!t.ClassDeclaration.check(node) && !t.ClassExpression.check(node)) {
return false;
}

// extends something
if (!node.superClass) {
return false;
}

// React.Component or React.PureComponent
const superClass = resolveToValue(path.get('superClass'));
if (
match(superClass.node, { property: { name: 'Component' } }) ||
match(superClass.node, { property: { name: 'PureComponent' } })
) {
const module = resolveToModule(superClass);
if (module && isReactModuleName(module)) {
return true;
}
}

// render method
if (node.body.body.some(isRenderMethod)) {
return true;
Expand All @@ -60,14 +77,5 @@ export default function isReactComponentClass(path: NodePath): boolean {
}
}

// extends ReactComponent?
if (!node.superClass) {
return false;
}
const superClass = resolveToValue(path.get('superClass'));
if (!match(superClass.node, { property: { name: 'Component' } })) {
return false;
}
const module = resolveToModule(superClass);
return !!module && isReactModuleName(module);
return false;
}