Skip to content

Commit

Permalink
fix: be more strict about detecting react class components (#397)
Browse files Browse the repository at this point in the history
Previously a class just needed to have a `render` method.
Now it in addition requires to have a superclass as well.
  • Loading branch information
danez committed Oct 25, 2019
1 parent aa54200 commit b3601b6
Show file tree
Hide file tree
Showing 4 changed files with 76 additions and 20 deletions.
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;
}

0 comments on commit b3601b6

Please sign in to comment.