diff --git a/packages/react-reconciler/src/ReactUpdateQueue.js b/packages/react-reconciler/src/ReactUpdateQueue.js index d8965347259db..31ee34c86942e 100644 --- a/packages/react-reconciler/src/ReactUpdateQueue.js +++ b/packages/react-reconciler/src/ReactUpdateQueue.js @@ -240,24 +240,81 @@ export function enqueueUpdate(fiber: Fiber, update: Update) { export function enqueueCapturedUpdate( workInProgress: Fiber, - update: Update, + capturedUpdate: Update, ) { + // Captured updates are updates that are thrown by a child during the render + // phase. They should be discarded if the render is aborted. Therefore, + // we should only put them on the work-in-progress queue, not the current one. + let queue: UpdateQueue = (workInProgress.updateQueue: any); + + // Check if the work-in-progress queue is a clone. const current = workInProgress.alternate; if (current !== null) { - // Ensure the work-in-progress queue is a clone - cloneUpdateQueue(current, workInProgress); + const currentQueue: UpdateQueue = (current.updateQueue: any); + if (queue === currentQueue) { + // The work-in-progress queue is the same as current. This happens when + // we bail out on a parent fiber that then captures an error thrown by + // a child. Since we want to append the update only to the work-in + // -progress queue, we need to clone the updates. We usually clone during + // processUpdateQueue, but that didn't happen in this case because we + // skipped over the parent when we bailed out. + let newFirst = null; + let lastFirst = null; + const firstBaseUpdate = queue.firstBaseUpdate; + if (firstBaseUpdate !== null) { + // Loop through the updates and clone them. + let update = firstBaseUpdate; + do { + const clone: Update = { + expirationTime: update.expirationTime, + suspenseConfig: update.suspenseConfig, + + tag: update.tag, + payload: update.payload, + callback: update.callback, + + next: null, + }; + if (lastFirst === null) { + newFirst = lastFirst = clone; + } else { + lastFirst.next = clone; + lastFirst = clone; + } + update = update.next; + } while (update !== null); + + // Append the captured update the end of the cloned list. + if (lastFirst === null) { + newFirst = lastFirst = capturedUpdate; + } else { + lastFirst.next = capturedUpdate; + lastFirst = capturedUpdate; + } + } else { + // There are no base updates. + newFirst = lastFirst = capturedUpdate; + } + queue = { + baseState: currentQueue.baseState, + firstBaseUpdate: newFirst, + lastBaseUpdate: lastFirst, + shared: currentQueue.shared, + effects: currentQueue.effects, + }; + workInProgress.updateQueue = queue; + return; + } } - // Captured updates go only on the work-in-progress queue. - const queue: UpdateQueue = (workInProgress.updateQueue: any); // Append the update to the end of the list. - const last = queue.lastBaseUpdate; - if (last === null) { - queue.firstBaseUpdate = update; + const lastBaseUpdate = queue.lastBaseUpdate; + if (lastBaseUpdate === null) { + queue.firstBaseUpdate = capturedUpdate; } else { - last.next = update; + lastBaseUpdate.next = capturedUpdate; } - queue.lastBaseUpdate = update; + queue.lastBaseUpdate = capturedUpdate; } function getStateFromUpdate( diff --git a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js index a94fe3c857c49..e064861a03501 100644 --- a/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js @@ -1754,6 +1754,51 @@ describe('ReactIncrementalErrorHandling', () => { expect(root).toMatchRenderedOutput('Everything is fine.'); }); + it('uncaught errors are discarded if the render is aborted, case 2', async () => { + const {useState} = React; + const root = ReactNoop.createRoot(); + + let setShouldThrow; + function Oops() { + const [shouldThrow, _setShouldThrow] = useState(false); + setShouldThrow = _setShouldThrow; + if (shouldThrow) { + throw Error('Oops'); + } + return null; + } + + function AllGood() { + Scheduler.unstable_yieldValue('Everything is fine.'); + return 'Everything is fine.'; + } + + await ReactNoop.act(async () => { + root.render(); + }); + + await ReactNoop.act(async () => { + // Schedule a high pri and a low pri update on the root. + ReactNoop.discreteUpdates(() => { + root.render(); + }); + root.render(); + // Render through just the high pri update. The low pri update remains on + // the queue. + expect(Scheduler).toFlushAndYieldThrough(['Everything is fine.']); + + // Schedule a high pri update on a child that triggers an error. + // The root should capture this error. But since there's still a pending + // update on the root, the error should be suppressed. + ReactNoop.discreteUpdates(() => { + setShouldThrow(true); + }); + }); + // Should render the final state without throwing the error. + expect(Scheduler).toHaveYielded(['Everything is fine.']); + expect(root).toMatchRenderedOutput('Everything is fine.'); + }); + if (global.__PERSISTENT__) { it('regression test: should fatal if error is thrown at the root', () => { const root = ReactNoop.createRoot();