fix(eslint): fix react-hooks/set-state-in-effect issues #7614
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This one I think is an interesting one, React doesn't recommend setting state directly inside effects (for a good reason), instead suggesting either to re-evalute whether extra state is even needed or, if unavoidable, call
setStateright inside the render method. We also link to a relevant "You might not need an effect" article from the Compass Development Guide docs, so it's nice to see a rule that would force us to think about this more.In some cases in this PR it was pretty straightforward to adjust the logic and get rid of set state where it was a clear case for derived state for example, but in other cases it's more convoluted and potentially tied to a lot of other logic in the plugins. Instead of changing every place independently to use setState in render, I added a new hook
useSyncStateOnPropChangethat does exactly what the documentation recommends. I added links to the docs to the hook and marked it as@deprecatedso that when used it highlights that this is probably an antipattern.