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

[sort-comp] static getter should be treated as static method over common getter #1808

Closed
kouhin opened this issue Jun 5, 2018 · 3 comments · Fixed by #1858
Closed

[sort-comp] static getter should be treated as static method over common getter #1808

kouhin opened this issue Jun 5, 2018 · 3 comments · Fixed by #1858

Comments

@kouhin
Copy link

kouhin commented Jun 5, 2018

Configuration (airbnb):

{
  rules: {
    'react/sort-comp': ['error', {
      order: [
        'static-methods',
        'instance-variables',
        'lifecycle',
        '/^on.+$/',
        'getters',
        'setters',
        '/^(get|set)(?!(InitialState$|DefaultProps$|ChildContext$)).+$/',
        'instance-methods',
        'everything-else',
        'rendering',
      ],
  }
}

Here is a simple example:

import PropTypes from 'prop-types';
import React from 'react';
import cx from 'classnames';

export class Icon extends React.Component {
  static propTypes = {
    name: PropTypes.string,
  };

  static get defaultProps() { // LINT ERROR
    return {
      name: 'defaultIcon',
    };
  }

  shouldComponentUpdate(nextProps) {
    return this.props.name === nextProps.name;
  }

  render() {
    return (
      <i // eslint-disable-line jsx-a11y/no-static-element-interactions,jsx-a11y/click-events-have-key-events
        className={cx('s', `s-${this.props.name}`)}
      />
    );
  }
}

It causes an error on static get defaultProps() and error message is getter functions should be placed after shouldComponentUpdate [react/sort-comp].

@ljharb
Copy link
Member

ljharb commented Jun 5, 2018

This is correct; also, using static getters like this is a big performance hit. Use Icon.defaultProps = instead.

@kouhin
Copy link
Author

kouhin commented Jun 8, 2018

@ljharb Thank you. Actually, we are using static get defaultProps() in our project in order to avoid memory leak.
However, besides performance problem of this pattern (maybe anti-pattern), I think static getters should be treated as static or static getters rather than common getters.

@ljharb
Copy link
Member

ljharb commented Jun 8, 2018

I'm not sure how there would be a memory leak from an object created once and assigned as a property.

It might make sense to make a separate category for "static getters", but it's absolutely an antipattern to use them in the first place.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging a pull request may close this issue.

2 participants