Javascript: react/sort-comp: getDerivedStateFromProps

[brandonburkett]

After updating to v17, I am getting 'getDerivedStateFromProps should be placed after componentDidUpdate`. I want to say previously in v16 the rule was below the constructor... does the new placement make sense?

[ljharb]

Yes - the bug was in eslint-plugin-react (you'd see the change on v16 as well, with a fully updated eslint-plugin-react). Specifically, gDSFP is both a static method and a lifecycle method, and the sorting logic was wrong.

See https://github.com/yannickcr/eslint-plugin-react/pull/1795 / https://github.com/yannickcr/eslint-plugin-react/issues/1793

[brandonburkett]

@ljharb

Apologies! I do not see gDSFP here at all though (assuming this overrides what eslint-plugin-react does?)

https://github.com/airbnb/javascript/blob/master/packages/eslint-config-airbnb/rules/react.js

// Enforce component methods order
    // https://github.com/yannickcr/eslint-plugin-react/blob/843d71a432baf0f01f598d7cf1eea75ad6896e4b/docs/rules/sort-comp.md
    'react/sort-comp': ['error', {
      order: [
        'static-methods',
        'instance-variables',
        'lifecycle',
        '/^on.+$/',
        'getters',
        'setters',
        '/^(get|set)(?!(InitialState$|DefaultProps$|ChildContext$)).+$/',
        'instance-methods',
        'everything-else',
        'rendering',
      ],
      groups: {
        lifecycle: [
          'displayName',
          'propTypes',
          'contextTypes',
          'childContextTypes',
          'mixins',
          'statics',
          'defaultProps',
          'constructor',
          'getDefaultProps',
          'getInitialState',
          'state',
          'getChildContext',
          'componentWillMount',
          'componentDidMount',
          'componentWillReceiveProps',
          'shouldComponentUpdate',
          'componentWillUpdate',
          'componentDidUpdate',
          'componentWillUnmount',
        ],
        rendering: [
          '/^render.+$/',
          'render'
        ],
      },
    }],

[ljharb]

Ooh, good call - we do override the defaults.

I think the change (which would still have shown up in v16) is that gDSFP in "statics" vs "lifecycle" changed.

In other words, gDSFP moved from "everything-else" to "statics" in eslint-plugin-react. We could/should add it to lifecycle, but that would still result in it appearing above the constructor.

[brandonburkett]

Okay cool. I was in refactor mode with my npm updated and was hoping not to have to move em around again too much.

Do you think the order will change at all for 17.x (even if it is just to match the react plugin)?

Internally debating if I delay the class method shuffle or not.

Appreciate all the feedback and thanks for your time on my isssues / questions today!

[ljharb]

I do not expect the ordering to change further (although there may be new React lifecycle methods added to the "lifecycle" group)

[brandonburkett]

To be clear, you do see it moving to statics right? Not below componentDidUpdate (my current error, plugin is updated).

[ljharb]

Since lifecycle is primarily for instance methods, i think it would be in "statics"