facebook / react

The library for web and native user interfaces.
https://react.dev
MIT License
227.36k stars 46.37k forks source link

Bug: SetState Calls not being batched inside Promise calls/micro Task #30605

Open Biki-das opened 1 month ago

Biki-das commented 1 month ago

setState calls are generally batched in React. However, in a case where setState calls occur inside a native event handler that also includes a microTask (Promise), the batching behaviour changes. When a setState is executed first in the synchronous part of the handler, followed by a microTask operation which includes a state update, and then another setState is executed after the microTask, these setState calls are not batched together as one might expect. the stateUpdate inside the Promise call does not get batched.

React version: 18.3.1

Steps To Reproduce

I am attaching a code Sandbox below to reproduce the same, where you can just run the code and see the Count Value rendered which does not resemble with the Way react should behave.

Link to code example:

https://stackblitz.com/edit/vitejs-vite-5lvqjx?file=src%2FCounter.tsx

The current behaviour

if we look closely, currently the count is getting set to 2, which proves the point, that state updates inside the Promise call are not being batched.

https://github.com/user-attachments/assets/4a686d8d-5815-4c6c-bc5a-b7b638c85bd8

The expected behaviour

the state update call inside the Promise should have been batched which would have made the count value to be 1 on the increment click.

Biki-das commented 1 month ago

cc @eps1lon @josephsavona

surenpoghosian commented 1 month ago

@Biki-das Hi there,

The behavior you’re seeing is due to the way React batches state updates, especially when dealing with both synchronous and asynchronous updates. Here’s a detailed look at the situation:

Synchronous Updates: When you call setCount(countRef.current + 1) multiple times in the synchronous part of the event handler, React batches these updates. However, if the updates are followed by an asynchronous operation (like a Promise), the batching of these synchronous updates might not align with the asynchronous update.

Asynchronous Updates: When setCount is called inside a Promise, React treats it as a separate update. The Promise-based state update is not batched with the synchronous updates that occurred before it, which can lead to the observed behavior where the count increments more than expected.

surenpoghosian commented 1 month ago

I've fully investigated your code and here is some feedback...

The point is that the setCount calls you make outside the then callback are both executed in the same synchronous execution flow. The then callback will be only be executed when the call stack is empty. And that isn't the case as long as there is code to execute in the currently running click handler.

So this is the sequence of what happens in your code:

  1. Execute the click handler
  2. Print "Click handler start"
  3. Read countRef.current, which is 0
  4. Call setCount the first time, with argument 0: the update is batched.
  5. Print "After first setCount"
  6. Create a resolved promise and call its then method. The callback that is given to it is not yet executed. That callback is queued as a microtask.
  7. Read countRef.current, which is still 0 because the earlier update is batched.
  8. Call setCount the second time, again with argument 0: also this update is batched.
  9. Print "After last setCount"

The click handler returns and the call stack becomes empty. Now the JS engine finds the react microtask that will perform the batched updates asynchronously. This results in countRef.current to be updated to 1.

No rendering cycle gets to execute yet, because there is our own promise-related microtask that now gets its turn to execute:

  1. Read countRef.current, which is now 1 because the earlier batched updates have already executed.
  2. Call setCount the third time, now with argument 1: this update is batched.
  3. Print "inside Promise"

The then callback returns and the call stack becomes empty. The JS engine finds another react microtask that will perform the newly batched update (only one this time). This results in countRef.current to be updated to 2.

The call stack is empty again, and eventually a paint job is executed, rendering that 2. Note that you'll never see the 1 displayed, because this is the first paint job that runs after the click handler got executed.

The batching and rendering would not have behaved differently if you would have moved some code like this:

 useLayoutEffect(() => {
    countRef.current = count;
    if (buttonRef.current) {
      buttonRef.current.onclick = () => {
        console.log('Click handler start');
        setCount(countRef.current + 1);
        console.log('After first setCount');

        setCount(countRef.current + 1);       // Moved this block here
        console.log('After last setCount');

        Promise.resolve().then(() => {        // ...and only then this block
          setCount(countRef.current + 1);
          console.log('Inside Promise');
        });

      };
    }
  });

Whether you place the then call at the start, the middle, or the end of the click handler body, it doesn't influence the order of executing those setCount calls, nor the rendering.

You wrote:

The state update call inside the Promise should be batched, resulting in the count value being 1 after the increment click.

It is batched, but after react has already cleared the batch that was collected by the synchronous part of the click handler. Realise that both calls of setCount that are made in that click handler were executed as part of the same synchronous flow. When the click handler returns, the JS engine gets to execute the react job that will process the batched updates. That happens before your own then callback is dequeued and executed.

Perseman commented 3 weeks ago

useLayoutEffect(() => { .... }, [])