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: Impossible order of getDerivedStateFromProps, state and constructor #1839

Closed
hornta opened this issue Jun 21, 2018 · 6 comments
Closed
Labels

Comments

@hornta
Copy link

hornta commented Jun 21, 2018

    static getDerivedStateFromProps() {}
    constructor(props) {}
    state = {};

Above code reports

getDerivedStateFromProps should be placed after state

So I change the code

    constructor(props) {}
    state = {};
    static getDerivedStateFromProps() {}

getDerivedStateFromProps should be placed before constructor

Also, the state property must be placed after the constructor. Something doesn't make sense here.

          'error',
          {
            order: ['static-methods', 'lifecycle', 'everything-else', 'render'],
            groups: {
              lifecycle: [
                'displayName',
                'propTypes',
                'contextTypes',
                'childContextTypes',
                'mixins',
                'statics',
                'defaultProps',
                'constructor',
                'getDefaultProps',
                'state',
                'getInitialState',
                'getChildContext',
                'getDerivedStateFromProps',
                'componentWillMount',
                'UNSAFE_componentWillMount',
                'componentDidMount',
                'componentWillReceiveProps',
                'UNSAFE_componentWillReceiveProps',
                'shouldComponentUpdate',
                'componentWillUpdate',
                'UNSAFE_componentWillUpdate',
                'getSnapshotBeforeUpdate',
                'componentDidUpdate',
                'componentDidCatch',
                'componentWillUnmount'
              ]
            }
          }
        ]```

@ljharb
Copy link
Member

ljharb commented Jun 21, 2018

"lifecycle" only handles instance methods, since gDSFP is the first such static method (the others are static properties, or instance methods).

@metreniuk
Copy link
Contributor

@ljharb It is not quite clear what is the expected behavior. The docs says (lifecycle default group) that getDerivedStateFromProps should be after the constructor. Should getDerivedStateFromProps be removed from lifecycle group or the rule should be fixed to report when getDerivedStateFromProps is before constructor?
First, I found GH-1795 that fixed that behavior. After I found GH-1856 that reverted the fix and removed the test case for the issue described here.

@ljharb
Copy link
Member

ljharb commented Aug 28, 2018

I think gDSFP should either be removed from the lifecycle list, or, it’s presence in the lifecycle list should override its inclusion in “static-methods”.

@metreniuk
Copy link
Contributor

@ljharb I made a PR (GH-1962) that enforces static lifecycles to be grouped under the lifecycle group. This seems the most reasonable behavior to me while working with lifecycle methods. This also seems to be the first guess behavior described in the docs, because getDerivedStateFromProps is mentioned explicitly in the lifecycle group vs implicitly in the static group.
The issue that I encountered is that having getDerivedStateFromProps in statics and lifecycle groups leads to a discrepancy in the same codebase.

@metreniuk
Copy link
Contributor

By the way the impossible order of getDerivedStateFromProps issue seems to be fixed in GH-1858.

@hornta
Copy link
Author

hornta commented Nov 29, 2018

I can confirm this works in the latest release 7.11.0

@ljharb ljharb closed this as completed Nov 29, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

No branches or pull requests

3 participants