diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index 6a0c70f8e6d00..824f89e967707 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -62,6 +62,7 @@ import { } from './ReactFiberSuspenseContext.new'; import { renderDidError, + renderDidSuspendDelayIfPossible, onUncaughtError, markLegacyErrorBoundaryAsFailed, isAlreadyFailedLegacyErrorBoundary, @@ -78,6 +79,7 @@ import { includesSomeLane, mergeLanes, pickArbitraryLane, + includesOnlyTransitions, } from './ReactFiberLane.new'; import { getIsHydrating, @@ -165,12 +167,7 @@ function createClassErrorUpdate( return update; } -function attachWakeableListeners( - suspenseBoundary: Fiber, - root: FiberRoot, - wakeable: Wakeable, - lanes: Lanes, -) { +function attachPingListener(root: FiberRoot, wakeable: Wakeable, lanes: Lanes) { // Attach a ping listener // // The data might resolve before we have a chance to commit the fallback. Or, @@ -183,34 +180,39 @@ function attachWakeableListeners( // // We only need to do this in concurrent mode. Legacy Suspense always // commits fallbacks synchronously, so there are no pings. - if (suspenseBoundary.mode & ConcurrentMode) { - let pingCache = root.pingCache; - let threadIDs; - if (pingCache === null) { - pingCache = root.pingCache = new PossiblyWeakMap(); + let pingCache = root.pingCache; + let threadIDs; + if (pingCache === null) { + pingCache = root.pingCache = new PossiblyWeakMap(); + threadIDs = new Set(); + pingCache.set(wakeable, threadIDs); + } else { + threadIDs = pingCache.get(wakeable); + if (threadIDs === undefined) { threadIDs = new Set(); pingCache.set(wakeable, threadIDs); - } else { - threadIDs = pingCache.get(wakeable); - if (threadIDs === undefined) { - threadIDs = new Set(); - pingCache.set(wakeable, threadIDs); - } } - if (!threadIDs.has(lanes)) { - // Memoize using the thread ID to prevent redundant listeners. - threadIDs.add(lanes); - const ping = pingSuspendedRoot.bind(null, root, wakeable, lanes); - if (enableUpdaterTracking) { - if (isDevToolsPresent) { - // If we have pending work still, restore the original updaters - restorePendingUpdaters(root, lanes); - } + } + if (!threadIDs.has(lanes)) { + // Memoize using the thread ID to prevent redundant listeners. + threadIDs.add(lanes); + const ping = pingSuspendedRoot.bind(null, root, wakeable, lanes); + if (enableUpdaterTracking) { + if (isDevToolsPresent) { + // If we have pending work still, restore the original updaters + restorePendingUpdaters(root, lanes); } - wakeable.then(ping, ping); } + wakeable.then(ping, ping); } +} +function attachRetryListener( + suspenseBoundary: Fiber, + root: FiberRoot, + wakeable: Wakeable, + lanes: Lanes, +) { // Retry listener // // If the fallback does commit, we need to attach a different type of @@ -470,24 +472,47 @@ function throwException( root, rootRenderLanes, ); - attachWakeableListeners( - suspenseBoundary, - root, - wakeable, - rootRenderLanes, - ); + // We only attach ping listeners in concurrent mode. Legacy Suspense always + // commits fallbacks synchronously, so there are no pings. + if (suspenseBoundary.mode & ConcurrentMode) { + attachPingListener(root, wakeable, rootRenderLanes); + } + attachRetryListener(suspenseBoundary, root, wakeable, rootRenderLanes); return; } else { - // No boundary was found. Fallthrough to error mode. + // No boundary was found. If we're inside startTransition, this is OK. + // We can suspend and wait for more data to arrive. + + if (includesOnlyTransitions(rootRenderLanes)) { + // This is a transition. Suspend. Since we're not activating a Suspense + // boundary, this will unwind all the way to the root without performing + // a second pass to render a fallback. (This is arguably how refresh + // transitions should work, too, since we're not going to commit the + // fallbacks anyway.) + attachPingListener(root, wakeable, rootRenderLanes); + renderDidSuspendDelayIfPossible(); + return; + } + + // We're not in a transition. We treat this case like an error because + // discrete renders are expected to finish synchronously to maintain + // consistency with external state. + // TODO: This will error during non-transition concurrent renders, too. + // But maybe it shouldn't? + // TODO: We should never call getComponentNameFromFiber in production. // Log a warning or something to prevent us from accidentally bundling it. - value = new Error( + const uncaughtSuspenseError = new Error( (getComponentNameFromFiber(sourceFiber) || 'A React component') + ' suspended while rendering, but no fallback UI was specified.\n' + '\n' + 'Add a component higher in the tree to ' + 'provide a loading indicator or placeholder to display.', ); + + // If we're outside a transition, fall through to the regular error path. + // The error will be caught by the nearest suspense boundary. + value = uncaughtSuspenseError; } } else { // This is a regular error, not a Suspense wakeable. diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js index 21ab03f4ac925..86a43a4dcaf5a 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -62,6 +62,7 @@ import { } from './ReactFiberSuspenseContext.old'; import { renderDidError, + renderDidSuspendDelayIfPossible, onUncaughtError, markLegacyErrorBoundaryAsFailed, isAlreadyFailedLegacyErrorBoundary, @@ -78,6 +79,7 @@ import { includesSomeLane, mergeLanes, pickArbitraryLane, + includesOnlyTransitions, } from './ReactFiberLane.old'; import { getIsHydrating, @@ -165,12 +167,7 @@ function createClassErrorUpdate( return update; } -function attachWakeableListeners( - suspenseBoundary: Fiber, - root: FiberRoot, - wakeable: Wakeable, - lanes: Lanes, -) { +function attachPingListener(root: FiberRoot, wakeable: Wakeable, lanes: Lanes) { // Attach a ping listener // // The data might resolve before we have a chance to commit the fallback. Or, @@ -183,34 +180,39 @@ function attachWakeableListeners( // // We only need to do this in concurrent mode. Legacy Suspense always // commits fallbacks synchronously, so there are no pings. - if (suspenseBoundary.mode & ConcurrentMode) { - let pingCache = root.pingCache; - let threadIDs; - if (pingCache === null) { - pingCache = root.pingCache = new PossiblyWeakMap(); + let pingCache = root.pingCache; + let threadIDs; + if (pingCache === null) { + pingCache = root.pingCache = new PossiblyWeakMap(); + threadIDs = new Set(); + pingCache.set(wakeable, threadIDs); + } else { + threadIDs = pingCache.get(wakeable); + if (threadIDs === undefined) { threadIDs = new Set(); pingCache.set(wakeable, threadIDs); - } else { - threadIDs = pingCache.get(wakeable); - if (threadIDs === undefined) { - threadIDs = new Set(); - pingCache.set(wakeable, threadIDs); - } } - if (!threadIDs.has(lanes)) { - // Memoize using the thread ID to prevent redundant listeners. - threadIDs.add(lanes); - const ping = pingSuspendedRoot.bind(null, root, wakeable, lanes); - if (enableUpdaterTracking) { - if (isDevToolsPresent) { - // If we have pending work still, restore the original updaters - restorePendingUpdaters(root, lanes); - } + } + if (!threadIDs.has(lanes)) { + // Memoize using the thread ID to prevent redundant listeners. + threadIDs.add(lanes); + const ping = pingSuspendedRoot.bind(null, root, wakeable, lanes); + if (enableUpdaterTracking) { + if (isDevToolsPresent) { + // If we have pending work still, restore the original updaters + restorePendingUpdaters(root, lanes); } - wakeable.then(ping, ping); } + wakeable.then(ping, ping); } +} +function attachRetryListener( + suspenseBoundary: Fiber, + root: FiberRoot, + wakeable: Wakeable, + lanes: Lanes, +) { // Retry listener // // If the fallback does commit, we need to attach a different type of @@ -470,24 +472,47 @@ function throwException( root, rootRenderLanes, ); - attachWakeableListeners( - suspenseBoundary, - root, - wakeable, - rootRenderLanes, - ); + // We only attach ping listeners in concurrent mode. Legacy Suspense always + // commits fallbacks synchronously, so there are no pings. + if (suspenseBoundary.mode & ConcurrentMode) { + attachPingListener(root, wakeable, rootRenderLanes); + } + attachRetryListener(suspenseBoundary, root, wakeable, rootRenderLanes); return; } else { - // No boundary was found. Fallthrough to error mode. + // No boundary was found. If we're inside startTransition, this is OK. + // We can suspend and wait for more data to arrive. + + if (includesOnlyTransitions(rootRenderLanes)) { + // This is a transition. Suspend. Since we're not activating a Suspense + // boundary, this will unwind all the way to the root without performing + // a second pass to render a fallback. (This is arguably how refresh + // transitions should work, too, since we're not going to commit the + // fallbacks anyway.) + attachPingListener(root, wakeable, rootRenderLanes); + renderDidSuspendDelayIfPossible(); + return; + } + + // We're not in a transition. We treat this case like an error because + // discrete renders are expected to finish synchronously to maintain + // consistency with external state. + // TODO: This will error during non-transition concurrent renders, too. + // But maybe it shouldn't? + // TODO: We should never call getComponentNameFromFiber in production. // Log a warning or something to prevent us from accidentally bundling it. - value = new Error( + const uncaughtSuspenseError = new Error( (getComponentNameFromFiber(sourceFiber) || 'A React component') + ' suspended while rendering, but no fallback UI was specified.\n' + '\n' + 'Add a component higher in the tree to ' + 'provide a loading indicator or placeholder to display.', ); + + // If we're outside a transition, fall through to the regular error path. + // The error will be caught by the nearest suspense boundary. + value = uncaughtSuspenseError; } } else { // This is a regular error, not a Suspense wakeable. diff --git a/packages/react-reconciler/src/ReactFiberUnwindWork.new.js b/packages/react-reconciler/src/ReactFiberUnwindWork.new.js index bb002b9e71b3a..a43c021d987e5 100644 --- a/packages/react-reconciler/src/ReactFiberUnwindWork.new.js +++ b/packages/react-reconciler/src/ReactFiberUnwindWork.new.js @@ -89,16 +89,17 @@ function unwindWork(workInProgress: Fiber, renderLanes: Lanes) { popTopLevelLegacyContextObject(workInProgress); resetMutableSourceWorkInProgressVersions(); const flags = workInProgress.flags; - - if ((flags & DidCapture) !== NoFlags) { - throw new Error( - 'The root failed to unmount after an error. This is likely a bug in ' + - 'React. Please file an issue.', - ); + if ( + (flags & ShouldCapture) !== NoFlags && + (flags & DidCapture) === NoFlags + ) { + // There was an error during render that wasn't captured by a suspense + // boundary. Do a second pass on the root to unmount the children. + workInProgress.flags = (flags & ~ShouldCapture) | DidCapture; + return workInProgress; } - - workInProgress.flags = (flags & ~ShouldCapture) | DidCapture; - return workInProgress; + // We unwound to the root without completing it. Exit. + return null; } case HostComponent: { // TODO: popHydrationState diff --git a/packages/react-reconciler/src/ReactFiberUnwindWork.old.js b/packages/react-reconciler/src/ReactFiberUnwindWork.old.js index 7f161513a4afa..e0cf7cc2f0fcc 100644 --- a/packages/react-reconciler/src/ReactFiberUnwindWork.old.js +++ b/packages/react-reconciler/src/ReactFiberUnwindWork.old.js @@ -89,16 +89,17 @@ function unwindWork(workInProgress: Fiber, renderLanes: Lanes) { popTopLevelLegacyContextObject(workInProgress); resetMutableSourceWorkInProgressVersions(); const flags = workInProgress.flags; - - if ((flags & DidCapture) !== NoFlags) { - throw new Error( - 'The root failed to unmount after an error. This is likely a bug in ' + - 'React. Please file an issue.', - ); + if ( + (flags & ShouldCapture) !== NoFlags && + (flags & DidCapture) === NoFlags + ) { + // There was an error during render that wasn't captured by a suspense + // boundary. Do a second pass on the root to unmount the children. + workInProgress.flags = (flags & ~ShouldCapture) | DidCapture; + return workInProgress; } - - workInProgress.flags = (flags & ~ShouldCapture) | DidCapture; - return workInProgress; + // We unwound to the root without completing it. Exit. + return null; } case HostComponent: { // TODO: popHydrationState diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index d9a11759a6bf8..42d230a4a655f 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -251,13 +251,14 @@ const RenderContext = /* */ 0b0010; const CommitContext = /* */ 0b0100; export const RetryAfterError = /* */ 0b1000; -type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5; -const RootIncomplete = 0; +type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5 | 6; +const RootInProgress = 0; const RootFatalErrored = 1; const RootErrored = 2; const RootSuspended = 3; const RootSuspendedWithDelay = 4; const RootCompleted = 5; +const RootDidNotComplete = 6; // Describes where we are in the React execution stack let executionContext: ExecutionContext = NoContext; @@ -280,7 +281,7 @@ export let subtreeRenderLanes: Lanes = NoLanes; const subtreeRenderLanesCursor: StackCursor = createCursor(NoLanes); // Whether to root completed, errored, suspended, etc. -let workInProgressRootExitStatus: RootExitStatus = RootIncomplete; +let workInProgressRootExitStatus: RootExitStatus = RootInProgress; // A fatal error, if one is thrown let workInProgressRootFatalError: mixed = null; // "Included" lanes refer to lanes that were worked on during this render. It's @@ -817,7 +818,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) { let exitStatus = shouldTimeSlice ? renderRootConcurrent(root, lanes) : renderRootSync(root, lanes); - if (exitStatus !== RootIncomplete) { + if (exitStatus !== RootInProgress) { if (exitStatus === RootErrored) { // If something threw an error, try rendering one more time. We'll // render synchronously to block concurrent data mutations, and we'll @@ -837,45 +838,58 @@ function performConcurrentWorkOnRoot(root, didTimeout) { throw fatalError; } - // Check if this render may have yielded to a concurrent event, and if so, - // confirm that any newly rendered stores are consistent. - // TODO: It's possible that even a concurrent render may never have yielded - // to the main thread, if it was fast enough, or if it expired. We could - // skip the consistency check in that case, too. - const renderWasConcurrent = !includesBlockingLane(root, lanes); - const finishedWork: Fiber = (root.current.alternate: any); - if ( - renderWasConcurrent && - !isRenderConsistentWithExternalStores(finishedWork) - ) { - // A store was mutated in an interleaved event. Render again, - // synchronously, to block further mutations. - exitStatus = renderRootSync(root, lanes); - - // We need to check again if something threw - if (exitStatus === RootErrored) { - const errorRetryLanes = getLanesToRetrySynchronouslyOnError(root); - if (errorRetryLanes !== NoLanes) { - lanes = errorRetryLanes; - exitStatus = recoverFromConcurrentError(root, errorRetryLanes); - // We assume the tree is now consistent because we didn't yield to any - // concurrent events. + if (exitStatus === RootDidNotComplete) { + // The render unwound without completing the tree. This happens in special + // cases where need to exit the current render without producing a + // consistent tree or committing. + // + // This should only happen during a concurrent render, not a discrete or + // synchronous update. We should have already checked for this when we + // unwound the stack. + markRootSuspended(root, lanes); + } else { + // The render completed. + + // Check if this render may have yielded to a concurrent event, and if so, + // confirm that any newly rendered stores are consistent. + // TODO: It's possible that even a concurrent render may never have yielded + // to the main thread, if it was fast enough, or if it expired. We could + // skip the consistency check in that case, too. + const renderWasConcurrent = !includesBlockingLane(root, lanes); + const finishedWork: Fiber = (root.current.alternate: any); + if ( + renderWasConcurrent && + !isRenderConsistentWithExternalStores(finishedWork) + ) { + // A store was mutated in an interleaved event. Render again, + // synchronously, to block further mutations. + exitStatus = renderRootSync(root, lanes); + + // We need to check again if something threw + if (exitStatus === RootErrored) { + const errorRetryLanes = getLanesToRetrySynchronouslyOnError(root); + if (errorRetryLanes !== NoLanes) { + lanes = errorRetryLanes; + exitStatus = recoverFromConcurrentError(root, errorRetryLanes); + // We assume the tree is now consistent because we didn't yield to any + // concurrent events. + } + } + if (exitStatus === RootFatalErrored) { + const fatalError = workInProgressRootFatalError; + prepareFreshStack(root, NoLanes); + markRootSuspended(root, lanes); + ensureRootIsScheduled(root, now()); + throw fatalError; } } - if (exitStatus === RootFatalErrored) { - const fatalError = workInProgressRootFatalError; - prepareFreshStack(root, NoLanes); - markRootSuspended(root, lanes); - ensureRootIsScheduled(root, now()); - throw fatalError; - } - } - // We now have a consistent tree. The next step is either to commit it, - // or, if something suspended, wait to commit it after a timeout. - root.finishedWork = finishedWork; - root.finishedLanes = lanes; - finishConcurrentRender(root, exitStatus, lanes); + // We now have a consistent tree. The next step is either to commit it, + // or, if something suspended, wait to commit it after a timeout. + root.finishedWork = finishedWork; + root.finishedLanes = lanes; + finishConcurrentRender(root, exitStatus, lanes); + } } ensureRootIsScheduled(root, now()); @@ -932,7 +946,7 @@ export function queueRecoverableErrors(errors: Array) { function finishConcurrentRender(root, exitStatus, lanes) { switch (exitStatus) { - case RootIncomplete: + case RootInProgress: case RootFatalErrored: { throw new Error('Root did not complete. This is a bug in React.'); } @@ -1148,6 +1162,10 @@ function performSyncWorkOnRoot(root) { throw fatalError; } + if (exitStatus === RootDidNotComplete) { + throw new Error('Root did not complete. This is a bug in React.'); + } + // We now have a consistent tree. Because this is a sync render, we // will commit it even if something suspended. const finishedWork: Fiber = (root.current.alternate: any); @@ -1343,7 +1361,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes) { workInProgressRoot = root; workInProgress = createWorkInProgress(root.current, null); workInProgressRootRenderLanes = subtreeRenderLanes = workInProgressRootIncludedLanes = lanes; - workInProgressRootExitStatus = RootIncomplete; + workInProgressRootExitStatus = RootInProgress; workInProgressRootFatalError = null; workInProgressRootSkippedLanes = NoLanes; workInProgressRootInterleavedUpdatedLanes = NoLanes; @@ -1473,14 +1491,14 @@ export function markSkippedUpdateLanes(lane: Lane | Lanes): void { } export function renderDidSuspend(): void { - if (workInProgressRootExitStatus === RootIncomplete) { + if (workInProgressRootExitStatus === RootInProgress) { workInProgressRootExitStatus = RootSuspended; } } export function renderDidSuspendDelayIfPossible(): void { if ( - workInProgressRootExitStatus === RootIncomplete || + workInProgressRootExitStatus === RootInProgress || workInProgressRootExitStatus === RootSuspended || workInProgressRootExitStatus === RootErrored ) { @@ -1521,7 +1539,7 @@ export function renderDidError(error: mixed) { export function renderHasNotSuspendedYet(): boolean { // If something errored or completed, we can't really be sure, // so those are false. - return workInProgressRootExitStatus === RootIncomplete; + return workInProgressRootExitStatus === RootInProgress; } function renderRootSync(root: FiberRoot, lanes: Lanes) { @@ -1671,7 +1689,7 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { if (enableSchedulingProfiler) { markRenderYielded(); } - return RootIncomplete; + return RootInProgress; } else { // Completed the tree. if (enableSchedulingProfiler) { @@ -1796,6 +1814,11 @@ function completeUnitOfWork(unitOfWork: Fiber): void { returnFiber.flags |= Incomplete; returnFiber.subtreeFlags = NoFlags; returnFiber.deletions = null; + } else { + // We've unwound all the way to the root. + workInProgressRootExitStatus = RootDidNotComplete; + workInProgress = null; + return; } } @@ -1812,7 +1835,7 @@ function completeUnitOfWork(unitOfWork: Fiber): void { } while (completedWork !== null); // We've reached the root. - if (workInProgressRootExitStatus === RootIncomplete) { + if (workInProgressRootExitStatus === RootInProgress) { workInProgressRootExitStatus = RootCompleted; } } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 8cb20434be660..4098e196488b4 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -251,13 +251,14 @@ const RenderContext = /* */ 0b0010; const CommitContext = /* */ 0b0100; export const RetryAfterError = /* */ 0b1000; -type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5; -const RootIncomplete = 0; +type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5 | 6; +const RootInProgress = 0; const RootFatalErrored = 1; const RootErrored = 2; const RootSuspended = 3; const RootSuspendedWithDelay = 4; const RootCompleted = 5; +const RootDidNotComplete = 6; // Describes where we are in the React execution stack let executionContext: ExecutionContext = NoContext; @@ -280,7 +281,7 @@ export let subtreeRenderLanes: Lanes = NoLanes; const subtreeRenderLanesCursor: StackCursor = createCursor(NoLanes); // Whether to root completed, errored, suspended, etc. -let workInProgressRootExitStatus: RootExitStatus = RootIncomplete; +let workInProgressRootExitStatus: RootExitStatus = RootInProgress; // A fatal error, if one is thrown let workInProgressRootFatalError: mixed = null; // "Included" lanes refer to lanes that were worked on during this render. It's @@ -817,7 +818,7 @@ function performConcurrentWorkOnRoot(root, didTimeout) { let exitStatus = shouldTimeSlice ? renderRootConcurrent(root, lanes) : renderRootSync(root, lanes); - if (exitStatus !== RootIncomplete) { + if (exitStatus !== RootInProgress) { if (exitStatus === RootErrored) { // If something threw an error, try rendering one more time. We'll // render synchronously to block concurrent data mutations, and we'll @@ -837,45 +838,58 @@ function performConcurrentWorkOnRoot(root, didTimeout) { throw fatalError; } - // Check if this render may have yielded to a concurrent event, and if so, - // confirm that any newly rendered stores are consistent. - // TODO: It's possible that even a concurrent render may never have yielded - // to the main thread, if it was fast enough, or if it expired. We could - // skip the consistency check in that case, too. - const renderWasConcurrent = !includesBlockingLane(root, lanes); - const finishedWork: Fiber = (root.current.alternate: any); - if ( - renderWasConcurrent && - !isRenderConsistentWithExternalStores(finishedWork) - ) { - // A store was mutated in an interleaved event. Render again, - // synchronously, to block further mutations. - exitStatus = renderRootSync(root, lanes); - - // We need to check again if something threw - if (exitStatus === RootErrored) { - const errorRetryLanes = getLanesToRetrySynchronouslyOnError(root); - if (errorRetryLanes !== NoLanes) { - lanes = errorRetryLanes; - exitStatus = recoverFromConcurrentError(root, errorRetryLanes); - // We assume the tree is now consistent because we didn't yield to any - // concurrent events. + if (exitStatus === RootDidNotComplete) { + // The render unwound without completing the tree. This happens in special + // cases where need to exit the current render without producing a + // consistent tree or committing. + // + // This should only happen during a concurrent render, not a discrete or + // synchronous update. We should have already checked for this when we + // unwound the stack. + markRootSuspended(root, lanes); + } else { + // The render completed. + + // Check if this render may have yielded to a concurrent event, and if so, + // confirm that any newly rendered stores are consistent. + // TODO: It's possible that even a concurrent render may never have yielded + // to the main thread, if it was fast enough, or if it expired. We could + // skip the consistency check in that case, too. + const renderWasConcurrent = !includesBlockingLane(root, lanes); + const finishedWork: Fiber = (root.current.alternate: any); + if ( + renderWasConcurrent && + !isRenderConsistentWithExternalStores(finishedWork) + ) { + // A store was mutated in an interleaved event. Render again, + // synchronously, to block further mutations. + exitStatus = renderRootSync(root, lanes); + + // We need to check again if something threw + if (exitStatus === RootErrored) { + const errorRetryLanes = getLanesToRetrySynchronouslyOnError(root); + if (errorRetryLanes !== NoLanes) { + lanes = errorRetryLanes; + exitStatus = recoverFromConcurrentError(root, errorRetryLanes); + // We assume the tree is now consistent because we didn't yield to any + // concurrent events. + } + } + if (exitStatus === RootFatalErrored) { + const fatalError = workInProgressRootFatalError; + prepareFreshStack(root, NoLanes); + markRootSuspended(root, lanes); + ensureRootIsScheduled(root, now()); + throw fatalError; } } - if (exitStatus === RootFatalErrored) { - const fatalError = workInProgressRootFatalError; - prepareFreshStack(root, NoLanes); - markRootSuspended(root, lanes); - ensureRootIsScheduled(root, now()); - throw fatalError; - } - } - // We now have a consistent tree. The next step is either to commit it, - // or, if something suspended, wait to commit it after a timeout. - root.finishedWork = finishedWork; - root.finishedLanes = lanes; - finishConcurrentRender(root, exitStatus, lanes); + // We now have a consistent tree. The next step is either to commit it, + // or, if something suspended, wait to commit it after a timeout. + root.finishedWork = finishedWork; + root.finishedLanes = lanes; + finishConcurrentRender(root, exitStatus, lanes); + } } ensureRootIsScheduled(root, now()); @@ -932,7 +946,7 @@ export function queueRecoverableErrors(errors: Array) { function finishConcurrentRender(root, exitStatus, lanes) { switch (exitStatus) { - case RootIncomplete: + case RootInProgress: case RootFatalErrored: { throw new Error('Root did not complete. This is a bug in React.'); } @@ -1148,6 +1162,10 @@ function performSyncWorkOnRoot(root) { throw fatalError; } + if (exitStatus === RootDidNotComplete) { + throw new Error('Root did not complete. This is a bug in React.'); + } + // We now have a consistent tree. Because this is a sync render, we // will commit it even if something suspended. const finishedWork: Fiber = (root.current.alternate: any); @@ -1343,7 +1361,7 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes) { workInProgressRoot = root; workInProgress = createWorkInProgress(root.current, null); workInProgressRootRenderLanes = subtreeRenderLanes = workInProgressRootIncludedLanes = lanes; - workInProgressRootExitStatus = RootIncomplete; + workInProgressRootExitStatus = RootInProgress; workInProgressRootFatalError = null; workInProgressRootSkippedLanes = NoLanes; workInProgressRootInterleavedUpdatedLanes = NoLanes; @@ -1473,14 +1491,14 @@ export function markSkippedUpdateLanes(lane: Lane | Lanes): void { } export function renderDidSuspend(): void { - if (workInProgressRootExitStatus === RootIncomplete) { + if (workInProgressRootExitStatus === RootInProgress) { workInProgressRootExitStatus = RootSuspended; } } export function renderDidSuspendDelayIfPossible(): void { if ( - workInProgressRootExitStatus === RootIncomplete || + workInProgressRootExitStatus === RootInProgress || workInProgressRootExitStatus === RootSuspended || workInProgressRootExitStatus === RootErrored ) { @@ -1521,7 +1539,7 @@ export function renderDidError(error: mixed) { export function renderHasNotSuspendedYet(): boolean { // If something errored or completed, we can't really be sure, // so those are false. - return workInProgressRootExitStatus === RootIncomplete; + return workInProgressRootExitStatus === RootInProgress; } function renderRootSync(root: FiberRoot, lanes: Lanes) { @@ -1671,7 +1689,7 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { if (enableSchedulingProfiler) { markRenderYielded(); } - return RootIncomplete; + return RootInProgress; } else { // Completed the tree. if (enableSchedulingProfiler) { @@ -1796,6 +1814,11 @@ function completeUnitOfWork(unitOfWork: Fiber): void { returnFiber.flags |= Incomplete; returnFiber.subtreeFlags = NoFlags; returnFiber.deletions = null; + } else { + // We've unwound all the way to the root. + workInProgressRootExitStatus = RootDidNotComplete; + workInProgress = null; + return; } } @@ -1812,7 +1835,7 @@ function completeUnitOfWork(unitOfWork: Fiber): void { } while (completedWork !== null); // We've reached the root. - if (workInProgressRootExitStatus === RootIncomplete) { + if (workInProgressRootExitStatus === RootInProgress) { workInProgressRootExitStatus = RootCompleted; } } diff --git a/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js b/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js index b968826845923..4c44a31b2b120 100644 --- a/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js +++ b/packages/react-reconciler/src/__tests__/ReactConcurrentErrorRecovery-test.js @@ -398,4 +398,138 @@ describe('ReactConcurrentErrorRecovery', () => { // Now we can show the error boundary that's wrapped around B. expect(root).toMatchRenderedOutput('Oops!B2'); }); + + // @gate enableCache + test('suspending in the shell (outside a Suspense boundary) should not throw, warn, or log during a transition', async () => { + class ErrorBoundary extends React.Component { + state = {error: null}; + static getDerivedStateFromError(error) { + return {error}; + } + render() { + if (this.state.error !== null) { + return ; + } + return this.props.children; + } + } + + // The initial render suspends without a Suspense boundary. Since it's + // wrapped in startTransition, it suspends instead of erroring. + const root = ReactNoop.createRoot(); + await act(async () => { + startTransition(() => { + root.render(); + }); + }); + expect(Scheduler).toHaveYielded(['Suspend! [Async]']); + expect(root).toMatchRenderedOutput(null); + + // This also works if the suspended component is wrapped with an error + // boundary. (This is only interesting because when a component suspends + // outside of a transition, we throw an error, which can be captured by + // an error boundary. + await act(async () => { + startTransition(() => { + root.render( + + + , + ); + }); + }); + expect(Scheduler).toHaveYielded(['Suspend! [Async]']); + expect(root).toMatchRenderedOutput(null); + + // Continues rendering once data resolves + await act(async () => { + resolveText('Async'); + }); + expect(Scheduler).toHaveYielded(['Async']); + expect(root).toMatchRenderedOutput('Async'); + }); + + // @gate enableCache + test( + 'errors during a suspended transition at the shell should not force ' + + 'fallbacks to display (error then suspend)', + async () => { + // This is similar to the earlier test for errors that occur during + // a refresh transition. Suspending in the shell is conceptually the same + // as a refresh, but they have slightly different implementation paths. + + class ErrorBoundary extends React.Component { + state = {error: null}; + static getDerivedStateFromError(error) { + return {error}; + } + render() { + if (this.state.error !== null) { + return ( + + ); + } + return this.props.children; + } + } + + function Throws() { + throw new Error('Oops!'); + } + + // Suspend and throw in the same transition + const root = ReactNoop.createRoot(); + await act(async () => { + startTransition(() => { + root.render( + + + + , + ); + }); + }); + expect(Scheduler).toHaveYielded([ + 'Suspend! [Async]', + // TODO: Ideally we would skip this second render pass to render the + // error UI, since it's not going to commit anyway. The same goes for + // Suspense fallbacks during a refresh transition. + 'Caught an error: Oops!', + ]); + // The render suspended without committing or surfacing the error. + expect(root).toMatchRenderedOutput(null); + + // Try the reverse order, too: throw then suspend + await act(async () => { + startTransition(() => { + root.render( + + + + , + ); + }); + }); + expect(Scheduler).toHaveYielded([ + 'Suspend! [Async]', + 'Caught an error: Oops!', + ]); + expect(root).toMatchRenderedOutput(null); + + await act(async () => { + await resolveText('Async'); + }); + + expect(Scheduler).toHaveYielded([ + 'Async', + 'Caught an error: Oops!', + + // Try recovering from the error by rendering again synchronously + 'Async', + 'Caught an error: Oops!', + ]); + + expect(root).toMatchRenderedOutput('Caught an error: Oops!'); + }, + ); });