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

fix: batch flushSync execution within a microtask #273

Merged
merged 30 commits into from
Nov 21, 2024

Conversation

vursen
Copy link
Contributor

@vursen vursen commented Nov 14, 2024

Description

Simply wrapping renderers in flushSync doesn't guarantee that the DOM will be updated synchronously. React may still defer it to a micro task if flushSync was called while rendering was already in progress, which is explained in the previously suppressed warning:

Warning: flushSync was called from inside a lifecycle method. React cannot flush when React is already rendering. Consider moving this call to a scheduler task or micro task.

When there are many deferred sync renderers that in turn trigger some async renderers, it can create race conditions that React misinterprets as a possible infinite loop:

Warning: Maximum update depth exceeded. This can happen when a component calls setState inside useEffect, but useEffect either doesn’t have a dependency array, or one of the dependencies changes on every render.

This warning particularly often occurs in AutoGrid where a mix of sync (GridColumn) and async (Select) renderers is used.

This PR updates Grid renderers to batch flushSync execution within a microtask, which resolves the maximum depth warning and is also good for performance. That being said, GridPro's editModeRenderer continues to use unbatched flushSync because its web component currently requires the editor to render synchronously.

Fixes #255

Type of change

  • Bugfix

@vursen vursen force-pushed the refactor/revert-grid-to-async-rendering branch from 9ba8bc6 to 0ca7e1c Compare November 15, 2024 14:28
@vursen vursen marked this pull request as ready for review November 15, 2024 14:31
@vursen vursen changed the title refactor: revert Grid to async rendering fix: revert Grid to async rendering Nov 15, 2024
@vursen vursen force-pushed the refactor/revert-grid-to-async-rendering branch from 1090856 to 33625f3 Compare November 18, 2024 12:57
@vursen vursen force-pushed the refactor/revert-grid-to-async-rendering branch 2 times, most recently from e228dc6 to 768626e Compare November 19, 2024 15:00
@vursen vursen changed the title fix: revert Grid to async rendering fix: batch flushSync execution within a microtask Nov 20, 2024
test/Grid.spec.tsx Outdated Show resolved Hide resolved
@vursen vursen force-pushed the refactor/revert-grid-to-async-rendering branch from 4818b22 to 5a8d15b Compare November 20, 2024 13:30
@vursen vursen force-pushed the refactor/revert-grid-to-async-rendering branch from d1e4617 to d01f67c Compare November 21, 2024 06:36
@vursen vursen force-pushed the refactor/revert-grid-to-async-rendering branch from 84ed982 to d502a0b Compare November 21, 2024 10:39
@vursen vursen force-pushed the refactor/revert-grid-to-async-rendering branch from d502a0b to 9199d2f Compare November 21, 2024 10:40
@vursen vursen removed the request for review from web-padawan November 21, 2024 10:44
@vursen vursen force-pushed the refactor/revert-grid-to-async-rendering branch from 98fe6e7 to 870c524 Compare November 21, 2024 11:37
@vursen vursen merged commit cf60b54 into main Nov 21, 2024
3 checks passed
@vursen vursen deleted the refactor/revert-grid-to-async-rendering branch November 21, 2024 11:42
vursen added a commit that referenced this pull request Nov 21, 2024
vursen added a commit that referenced this pull request Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Maximum update depth exceeded with AutoGrid columnRendering set to lazy
3 participants