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

Bug: useSyncExternalStore update not batched within unstable_batchedUpdates #24831

Open
Andarist opened this issue Jun 30, 2022 · 5 comments
Open
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug

Comments

@Andarist
Copy link
Contributor

Steps To Reproduce

  1. schedule two updates using "default priority" scope (eg. from within setTimeout)
  2. let one of the updates be handled through useSyncExternalStore and let the other one be handled through regular setState

Link to code example:
https://codesandbox.io/s/interesting-chatterjee-9ed8hj?file=/src/App.js (without unstable_batchedUpdates)
https://codesandbox.io/s/prod-glitter-51vwm1?file=/src/App.js (with unstable_batchedUpdates)

The current behavior

The useSyncExternalStore update gets flushed and committed before the other one has a chance to be committed to the screen. This doesn't allow me to read the updated state of the committed DOM of the children components from within the Parent's effect

The expected behavior

I would expect to be able to "join" this sync update of the useSyncExternalStore somehow. Ideally, I wouldn't have to wrap both with unstable_batchedUpdates. It would be cool if I could just schedule a single update with the same priority. If I understand correctly multiple updates coming from different useSyncExternalStore updates can be batched together (it doesn't work like flushSync, so it doesn't literally immediately flush the scheduled update) - and I would just like to hop on that train with my second update.

I could live with the solution based on unstable_batchedUpdates but that doesn't seem to work either.

@Andarist Andarist added the Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug label Jun 30, 2022
@markerikson
Copy link
Contributor

Andarist pointed out this is likely the cause of reduxjs/react-redux#1912

@sebmarkbage
Copy link
Collaborator

sebmarkbage commented Sep 5, 2022

unstable_batchedUpdates is a way to deprioritize an update by delaying it. The default priority is even more delayed and more batched than unstable_batchedUpdates. So unstable_batchedUpdates is a noop in React 18.

If you need the consistency you need to compromise - by making your updates less batched and flush earlier than they otherwise would - using flushSync.

That's the compromise of using useSyncExternalStore. To preserve consistency with external mutable store you have to make it less batchable - less delayed, than other forms of updates.

@TkDodo
Copy link

TkDodo commented Oct 31, 2022

So unstable_batchedUpdates is a noop in React 18.

Cool, that's the first time I'm seeing this. Is the same true for unstable_batchedUpdates from react-native @sebmarkbage ?

Copy link

This issue has been automatically marked as stale. If this issue is still affecting you, please leave any comment (for example, "bump"), and we'll keep it open. We are sorry that we haven't been able to prioritize it yet. If you have any new additional information, please include it with your comment!

@github-actions github-actions bot added the Resolution: Stale Automatically closed due to inactivity label Apr 10, 2024
@TkDodo
Copy link

TkDodo commented Apr 10, 2024

bump

@github-actions github-actions bot removed the Resolution: Stale Automatically closed due to inactivity label Apr 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Unconfirmed A potential issue that we haven't yet confirmed as a bug
Projects
None yet
Development

No branches or pull requests

4 participants