Skip to content

Commit

Permalink
Ensure base queue is a clone
Browse files Browse the repository at this point in the history
When an error boundary captures an error, we append the error update
to the work-in-progress queue only so that if the render is aborted,
the error update is dropped.

Before appending to the queue, we need to make sure the queue is a
work-in-progress copy. Usually we clone the queue during
`processUpdateQueue`; however, if the base queue has lower priority
than the current render, we may have bailed out on the boundary fiber
without ever entering `processUpdateQueue`. So we need to lazily clone
the queue.
  • Loading branch information
acdlite committed Mar 11, 2020
1 parent 287932c commit bbb7134
Show file tree
Hide file tree
Showing 2 changed files with 112 additions and 10 deletions.
77 changes: 67 additions & 10 deletions packages/react-reconciler/src/ReactUpdateQueue.js
Expand Up @@ -240,24 +240,81 @@ export function enqueueUpdate<State>(fiber: Fiber, update: Update<State>) {

export function enqueueCapturedUpdate<State>(
workInProgress: Fiber,
update: Update<State>,
capturedUpdate: Update<State>,
) {
// 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<State> = (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<State> = (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<State> = {
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<State> = (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<State>(
Expand Down
Expand Up @@ -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(<Oops />);
});

await ReactNoop.act(async () => {
// Schedule a high pri and a low pri update on the root.
ReactNoop.discreteUpdates(() => {
root.render(<Oops />);
});
root.render(<AllGood />);
// 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();
Expand Down

0 comments on commit bbb7134

Please sign in to comment.