eslint-plugin-react icon indicating copy to clipboard operation
eslint-plugin-react copied to clipboard

Published 20 hours ago • verified publisher icon jsx-eslint

Group static lifecycle methods under lifecycle

Open metreniuk opened this issue 6 years ago • 5 comments

Since the lifecycle group is a special group that deals with state/props changes, grouping static lifecycle methods under lifecycle group helps to have this logic together. This looks like a good default behavior that follows the docs. This PR enforces static lifecycle methods to be grouped under lifecycle group as one of possible solutions suggested by @ljharb .

metreniuk avatar Aug 29 '18 14:08 metreniuk

@alexzherdev what do you think about this? Is it accurate to do so or this is a breaking change that isn't worth doing?

metreniuk avatar Sep 13 '18 08:09 metreniuk

It's definitely a breaking change; it'd be ideal to ship this now under an option, and then we can flip the option in the next major if desired.

ljharb avatar Sep 17 '18 22:09 ljharb

Since the current behavior was intended to allow the same method to be part of multiple groups I am willing to keep this as it is. At the same time there could be different opinions about where a method should be (ex: getDerivedStateFromProps before/after state declaration). What do you think about naming the option preferLifecycle that will be implemented under the behavior from this PR?

metreniuk avatar Sep 18 '18 19:09 metreniuk

@metreniuk sorry, I don't have a lot of experience with this rule. From looking at this PR, I would expect more changes in tests though? Am I missing something?

alexzherdev avatar Sep 18 '18 20:09 alexzherdev

@alexzherdev sure. I've added the new functionality under the preferLifecycle option (false by default) and added more tests.

metreniuk avatar Sep 18 '18 22:09 metreniuk