-
Notifications
You must be signed in to change notification settings - Fork 47k
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: Some transition updates haven't been rendered #24350
Comments
I can reproduce this if I type very very very fast. It's worth noting that the input is uncontrolled, so there's nothing wrong with the |
I have debugged the react source code for a long time. What I can confirm is that all |
I think we want to see if that’s the case of an update getting “dropped” from the update queue somehow, or if it’s the case of “forgetting” to commit it. |
All updates have executed The code of my print log is as follows: function enqueueUpdate<S, A>(
fiber: Fiber,
queue: UpdateQueue<S, A>,
update: Update<S, A>,
lane: Lane,
) {
if (isInterleavedUpdate(fiber, lane)) {
+ console.log('isInterleavedUpdate')
const interleaved = queue.interleaved;
if (interleaved === null) {
// This is the first update. Create a circular list.
update.next = update;
// At the end of the current render, this queue's interleaved updates will
// be transferred to the pending queue.
pushInterleavedQueue(queue);
} else {
update.next = interleaved.next;
interleaved.next = update;
}
queue.interleaved = update;
} else {
+ console.log('pending')
const pending = queue.pending;
if (pending === null) {
// This is the first update. Create a circular list.
update.next = update;
} else {
update.next = pending.next;
pending.next = update;
}
queue.pending = update;
}
} The printed log is shown in the figure below: I hope this is helpful in troubleshooting problems. |
My hunch is that it's a bug with the "interleaved updates" check causing the updates to be enqueued in the wrong order |
I suspect if you add a function enqueueUpdate<S, A>(
fiber: Fiber,
queue: UpdateQueue<S, A>,
update: Update<S, A>,
lane: Lane,
) {
if (isInterleavedUpdate(fiber, lane)) {
const interleaved = queue.interleaved;
if (interleaved === null) {
// This is the first update. Create a circular list.
update.next = update;
// At the end of the current render, this queue's interleaved updates will
// be transferred to the pending queue.
pushInterleavedQueue(queue);
} else {
update.next = interleaved.next;
interleaved.next = update;
}
queue.interleaved = update;
} else {
+ enqueueInterleavedUpdates();
const pending = queue.pending;
if (pending === null) {
// This is the first update. Create a circular list.
update.next = update;
} else {
update.next = pending.next;
pending.next = update;
}
queue.pending = update;
}
} |
^ that's not an appropriate fix, btw, because it has other consequences, but if it works that tells us what the issue is |
Demonstrates the bug reported in facebook#24350.
Demonstrates the bug reported in facebook#24350.
I believe this will fix it. Going to a meeting but will check after: #24353 |
Confirmed that #24353 fixes this: https://codesandbox.io/s/react18-starttransition-forked-dzyyzb?file=/package.json Thanks so much for the bug report! |
…dates (#24353) * Regression test: Interleaved update race condition Demonstrates the bug reported in #24350. * Bugfix: Last update wins, even if interleaved "Interleaved" updates are updates that are scheduled while a render is already in progress. We put these on a special queue so that they don't get processed during the current render. Then we transfer them to the "real" queue after the render has finished. There was a race condition where an update is received after the render has finished but before the interleaved update queue had been transferred, causing the updates to be queued in the wrong order. The fix I chose is to check if the interleaved updates queue is empty before adding any update to the real queue. If it's not empty, then the new update must also be treated as interleaved.
…dates (#24353) * Regression test: Interleaved update race condition Demonstrates the bug reported in #24350. * Bugfix: Last update wins, even if interleaved "Interleaved" updates are updates that are scheduled while a render is already in progress. We put these on a special queue so that they don't get processed during the current render. Then we transfer them to the "real" queue after the render has finished. There was a race condition where an update is received after the render has finished but before the interleaved update queue had been transferred, causing the updates to be queued in the wrong order. The fix I chose is to check if the interleaved updates queue is empty before adding any update to the real queue. If it's not empty, then the new update must also be treated as interleaved.
…dates (#24353) * Regression test: Interleaved update race condition Demonstrates the bug reported in #24350. * Bugfix: Last update wins, even if interleaved "Interleaved" updates are updates that are scheduled while a render is already in progress. We put these on a special queue so that they don't get processed during the current render. Then we transfer them to the "real" queue after the render has finished. There was a race condition where an update is received after the render has finished but before the interleaved update queue had been transferred, causing the updates to be queued in the wrong order. The fix I chose is to check if the interleaved updates queue is empty before adding any update to the real queue. If it's not empty, then the new update must also be treated as interleaved.
…dates (facebook#24353) * Regression test: Interleaved update race condition Demonstrates the bug reported in facebook#24350. * Bugfix: Last update wins, even if interleaved "Interleaved" updates are updates that are scheduled while a render is already in progress. We put these on a special queue so that they don't get processed during the current render. Then we transfer them to the "real" queue after the render has finished. There was a race condition where an update is received after the render has finished but before the interleaved update queue had been transferred, causing the updates to be queued in the wrong order. The fix I chose is to check if the interleaved updates queue is empty before adding any update to the real queue. If it's not empty, then the new update must also be treated as interleaved.
…dates (#24353) * Regression test: Interleaved update race condition Demonstrates the bug reported in #24350. * Bugfix: Last update wins, even if interleaved "Interleaved" updates are updates that are scheduled while a render is already in progress. We put these on a special queue so that they don't get processed during the current render. Then we transfer them to the "real" queue after the render has finished. There was a race condition where an update is received after the render has finished but before the interleaved update queue had been transferred, causing the updates to be queued in the wrong order. The fix I chose is to check if the interleaved updates queue is empty before adding any update to the real queue. If it's not empty, then the new update must also be treated as interleaved.
Fix out in 18.1.0. Thanks again for a great report. |
React version: 18.0.0
Steps To Reproduce
<input />
at a very fast speed.216948261894
, only216948
is rendered, and the following characters are not rendered.Link to code example:
https://codesandbox.io/s/react18-starttransition-5u1en2?file=/src/App.js
The current behavior
As shown in the figure below, all the contents I input for the first time have been rendered. But the second time I input
216948261894
, only216948
is rendered, and the following characters are not rendered.The expected behavior
Everything I input should be rendered.
The text was updated successfully, but these errors were encountered: