Skip to content

Conversation

@gribnoysup
Copy link
Collaborator

@gribnoysup gribnoysup commented Nov 26, 2025

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 setState right 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 useSyncStateOnPropChange that does exactly what the documentation recommends. I added links to the docs to the hook and marked it as @deprecated so that when used it highlights that this is probably an antipattern.

@gribnoysup gribnoysup requested a review from a team as a code owner November 26, 2025 17:31
@gribnoysup gribnoysup added no release notes Fix or feature not for release notes no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion) labels Nov 26, 2025
@gribnoysup gribnoysup requested a review from Anemy November 26, 2025 17:31
@github-actions github-actions bot added the fix label Nov 26, 2025
Copy link
Member

@Anemy Anemy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm! Left one comment (non-blocker, feel free to merge without).

@gribnoysup gribnoysup merged commit 2b70205 into main Dec 1, 2025
81 of 84 checks passed
@gribnoysup gribnoysup deleted the fix-react-hooks-set-state-in-effect branch December 1, 2025 10:56
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix no release notes Fix or feature not for release notes no-title-validation Skips validation of PR titles (conventional commit adherence + JIRA ticket inclusion)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants