Group static lifecycle methods under lifecycle #1962
Conversation
|
@alexzherdev what do you think about this? Is it accurate to do so or this is a breaking change that isn't worth doing? |
|
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. |
|
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: |
|
@metreniuk sorry, I don't have a lot of experience with this rule. |
|
@alexzherdev sure. I've added the new functionality under the |
| 'class Hello extends React.Component {', | ||
| ' constructor() {}', | ||
| ' static getDerivedStateFromProps() {}', | ||
| '}' | ||
| ].join('\n') | ||
| ].join('\n'), | ||
| options: [{preferLifecycle: true}] |
There was a problem hiding this comment.
instead of modifying this test, can we duplicate it and modify the duplicate?
ljharb
force-pushed
the
master
branch
6 times, most recently
from
59af733 to
865ed16
Compare
ljharb
force-pushed
the
master
branch
4 times, most recently
from
069314a to
181c68f
Compare
ljharb
force-pushed
the
master
branch
2 times, most recently
from
380e32c to
51d342b
Compare
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
lifecyclegroup as one of possible solutions suggested by @ljharb .