Skip to content

[Proposal] Batch bg state updates in Engine service #15189

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

stas-kravchuk
Copy link

Description

Related issues

Fixes:

Manual testing steps

  1. Go to this page...

Screenshots/Recordings

Before

After

Pre-merge author checklist

Pre-merge reviewer checklist

  • I've manually tested the PR (e.g. pull and build branch, run the app, test code being changed).
  • I confirm that this PR addresses all acceptance criteria described in the ticket it closes and includes the necessary testing evidence such as recordings and or screenshots.

Copy link
Contributor

github-actions bot commented May 6, 2025

CLA Signature Action:

Thank you for your submission, we really appreciate it. We ask that you read and sign our Contributor License Agreement before we can accept your contribution. You can sign the CLA by just by adding a comment to this pull request with this exact sentence:

I have read the CLA Document and I hereby sign the CLA

By commenting with the above message you are agreeing to the terms of the CLA. Your account will be recorded as agreeing to our CLA so you don't need to sign it again for future contributions to this repository.

0 out of 1 committers have signed the CLA.
@stas-kravchuk

@gambinish gambinish requested a review from Cal-L May 6, 2025 18:55
Copy link
Contributor

@Cal-L Cal-L left a comment

Choose a reason for hiding this comment

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

Nice Stas! We already have a solution for this using lodash's debounce here - #14927. Any thoughts between the two solutions?

@stas-kravchuk
Copy link
Author

@Cal-L, the two main differences in our approaches are:

  1. usage of unstable_batchedUpdates - state updates triggered outside of React event handlers - such as within a setTimeout callback (debounce uses setTimeout under the hood) - are not automatically batched, so each dispatch would normally trigger its own rerender, so usage of unstable_batchedUpdates ensures that multiple redux actions flush result in a single render pass
  2. setTimeout vs debounce - it was already mentioned in PR discussion that if events keep coming in faster than the delay (constant background updates), a debounced flush may never occur, and that results in no UI updates. The setTimeout approach ensured a flush at least once per interval. Additionally, the setTimeout approach avoids pulling in an external dependency.

also this pr introduces fewer changes, and the Batcher class might be reused in other places to batch updates

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.

3 participants