From 6ef9ffa5b6edd230cc9cbea4568a58b2c6451758 Mon Sep 17 00:00:00 2001 From: Daniel Tschinder Date: Fri, 25 Oct 2019 22:08:27 +0200 Subject: [PATCH] fix: be more strict about detecting react class components Previously a class just needed to have a `render` method. Now it in addition requires to have a superclass as well. --- .../__tests__/componentMethodsHandler-test.js | 4 +- .../findAllComponentDefinitions-test.js | 6 +- .../__tests__/isReactComponentClass-test.js | 56 +++++++++++++++++-- src/utils/isReactComponentClass.js | 30 ++++++---- 4 files changed, 76 insertions(+), 20 deletions(-) diff --git a/src/handlers/__tests__/componentMethodsHandler-test.js b/src/handlers/__tests__/componentMethodsHandler-test.js index 9ff40ff7a6c..3e913aa3863 100644 --- a/src/handlers/__tests__/componentMethodsHandler-test.js +++ b/src/handlers/__tests__/componentMethodsHandler-test.js @@ -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 */ @@ -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 */ diff --git a/src/resolver/__tests__/findAllComponentDefinitions-test.js b/src/resolver/__tests__/findAllComponentDefinitions-test.js index ec6aca16611..9a14a415978 100644 --- a/src/resolver/__tests__/findAllComponentDefinitions-test.js +++ b/src/resolver/__tests__/findAllComponentDefinitions-test.js @@ -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 {} `; diff --git a/src/utils/__tests__/isReactComponentClass-test.js b/src/utils/__tests__/isReactComponentClass-test.js index 46b2581f12b..844ad034634 100644 --- a/src/utils/__tests__/isReactComponentClass-test.js +++ b/src/utils/__tests__/isReactComponentClass-test.js @@ -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', () => { @@ -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); + }); + }); }); diff --git a/src/utils/isReactComponentClass.js b/src/utils/isReactComponentClass.js index 201524ac0d0..79c7ae059d8 100644 --- a/src/utils/isReactComponentClass.js +++ b/src/utils/isReactComponentClass.js @@ -28,7 +28,7 @@ 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; @@ -36,6 +36,23 @@ export default function isReactComponentClass(path: NodePath): boolean { 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; @@ -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; }