Skip to content
This repository has been archived by the owner on Apr 9, 2020. It is now read-only.

Improve the logic for determining if class is a React component #6

Closed
gajus opened this issue Sep 3, 2015 · 5 comments
Closed

Improve the logic for determining if class is a React component #6

gajus opened this issue Sep 3, 2015 · 5 comments

Comments

@gajus
Copy link

gajus commented Sep 3, 2015

I have been looking at the code and isCompomentishClass function caught my eye as very loose:

/**
* Does this class have a render function?
*/
function isComponentishClass(cls) {
    return cls.body.body.filter(isRenderMethod).length > 0;
} 

The only logic that is used to determine whether class declaration represents a React component is if it defines render method.

Off top of my head, I do not have suggestions how to improve this beyond:

  • Check if class extends Component.
  • Check if class extends React.Component.

I still think thats pretty loose.

I am opening this issue to create a medium for discussion. Maybe someone will have better ideas.

@gajus
Copy link
Author

gajus commented Sep 3, 2015

This is how I would implement isComponentClass:

import _ from 'lodash';

let isComponentClass,
    isComponentIdentifier,
    isReactComponentMemberExpression,
    isReactIdentifier,
    isRenderMethod;

/**
 * @param {Object} node
 * @return {Boolean}
 */
isRenderMethod = (node) => {
    return _.every([
        node.kind === 'method',
        node.key.name === 'render'
    ]);
};

/**
 * @param {Object} node
 * @return {Boolean}
 */
isReactIdentifier = (node) => {
    return _.every([
        node.type === 'Identifier',
        node.name === 'React'
    ]);
};

/**
 * @param {Object} node
 * @return {Boolean}
 */
isComponentIdentifier = (node) => {
    return _.every([
        node.type === 'Identifier',
        node.name === 'Component'
    ]);
}

/**
 * Checks if class extends React.Component.
 *
 * @param {Object} node
 * @return {Boolean}
 */
isReactComponentMemberExpression = (node) => {
    return _.every([
        node.type === 'MemberExpression',
        isReactIdentifier(node.object),
        isComponentIdentifier(node.property)
    ]);
};

/**
 * Checks if class has render method and class extends React.Component.
 * 
 * @param {Object} node
 * @return {Boolean}
 */
isComponentClass = (node) => {
    let hasRenderMethod,
        extendsReactComponent;

    hasRenderMethod = Boolean(_.find(node.body.body, isRenderMethod));
    extendsReactComponent = isReactComponentMemberExpression(node.superClass);

    return _.every([
        hasRenderMethod,
        extendsReactComponent
    ]);
};

However, this assumes that class extends React.Component. I know that some people use Component alias (which I think is too generic for checking). However, it is easy to extend this implementation to detect if class extends React.Component or class extends Component.

@gaearon
Copy link
Owner

gaearon commented Sep 3, 2015

Check if class extends Component.
Check if class extends React.Component.

React classes don't have to extend Component. You can check: it works just fine without it. Moreover, people might have custom base classes. (Yes it's a bad idea, but people do that occasionally.)

@gaearon
Copy link
Owner

gaearon commented Sep 3, 2015

On the other hand, people often add extends Component to appease Flow, so we may as well demand the same. This will also discourage people from inheritance which I guess is a net win. Finally, we can make this configurable like #3 describes, with baseClasses: ['React.Component', 'Component'] being default.

@gajus
Copy link
Author

gajus commented Sep 3, 2015

Sounds like a good idea. I am still getting my head around how to write Babel plugins. I am in the midst of writing one myself babel-react-plugin-css-modules, which is an experiment to automate react-css-modules even further.

I am happy to own this issue and send a PR in a window of a few days.

@gaearon
Copy link
Owner

gaearon commented Sep 3, 2015

Let's discuss in #8.

@gaearon gaearon closed this as completed Sep 3, 2015
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants