diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index b23f45fb3174e..cbefbc7f26d30 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -3385,7 +3385,6 @@ body { ); }); - // @gate FIXME it('loading a stylesheet as part of an error boundary UI, during initial render', async () => { class ErrorBoundary extends React.Component { state = {error: null}; diff --git a/packages/react-reconciler/src/ReactFiberThenable.js b/packages/react-reconciler/src/ReactFiberThenable.js index 6d99a07132218..5da965e41c4fb 100644 --- a/packages/react-reconciler/src/ReactFiberThenable.js +++ b/packages/react-reconciler/src/ReactFiberThenable.js @@ -42,7 +42,16 @@ export const SuspenseyCommitException: mixed = new Error( // TODO: It would be better to refactor throwException into multiple functions // so we can trigger a fallback directly without having to check the type. But // for now this will do. -export const noopSuspenseyCommitThenable = {then() {}}; +export const noopSuspenseyCommitThenable = { + then() { + if (__DEV__) { + console.error( + 'Internal React error: A listener was unexpectedly attached to a ' + + '"noop" thenable. This is a bug in React. Please file an issue.', + ); + } + }, +}; export function createThenableState(): ThenableState { // The ThenableState is created the first time a component suspends. If it diff --git a/packages/react-reconciler/src/ReactFiberThrow.js b/packages/react-reconciler/src/ReactFiberThrow.js index a632104268458..b979f194652d1 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.js +++ b/packages/react-reconciler/src/ReactFiberThrow.js @@ -438,8 +438,15 @@ function throwException( } else { retryQueue.add(wakeable); } + + // 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); + } } - break; + return; } case OffscreenComponent: { if (suspenseBoundary.mode & ConcurrentMode) { @@ -466,24 +473,17 @@ function throwException( retryQueue.add(wakeable); } } + + attachPingListener(root, wakeable, rootRenderLanes); } - break; + return; } - // Fall through - } - default: { - throw new Error( - `Unexpected Suspense handler tag (${suspenseBoundary.tag}). This ` + - 'is a bug in React.', - ); } } - // 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); - } - return; + throw new Error( + `Unexpected Suspense handler tag (${suspenseBoundary.tag}). This ` + + 'is a bug in React.', + ); } else { // No boundary was found. Unless this is a sync update, this is OK. // We can suspend and wait for more data to arrive. diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 0f39ff56a24ce..10c87305ad472 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -1687,6 +1687,16 @@ export function shouldRemainOnPreviousScreen(): boolean { // takes into account both the priority of render and also whether showing a // fallback would produce a desirable user experience. + const handler = getSuspenseHandler(); + if (handler === null) { + // There's no Suspense boundary that can provide a fallback. We have no + // choice but to remain on the previous screen. + // NOTE: We do this even for sync updates, for lack of any better option. In + // the future, we may change how we handle this, like by putting the whole + // root into a "detached" mode. + return true; + } + // TODO: Once `use` has fully replaced the `throw promise` pattern, we should // be able to remove the equivalent check in finishConcurrentRender, and rely // just on this one. @@ -1705,29 +1715,22 @@ export function shouldRemainOnPreviousScreen(): boolean { } } - const handler = getSuspenseHandler(); - if (handler === null) { - // TODO: We should support suspending in the case where there's no - // parent Suspense boundary, even outside a transition. Somehow. Otherwise, - // an uncached promise can fall into an infinite loop. - } else { - if ( - includesOnlyRetries(workInProgressRootRenderLanes) || - // In this context, an OffscreenLane counts as a Retry - // TODO: It's become increasingly clear that Retries and Offscreen are - // deeply connected. They probably can be unified further. - includesSomeLane(workInProgressRootRenderLanes, OffscreenLane) - ) { - // During a retry, we can suspend rendering if the nearest Suspense boundary - // is the boundary of the "shell", because we're guaranteed not to block - // any new content from appearing. - // - // The reason we must check if this is a retry is because it guarantees - // that suspending the work loop won't block an actual update, because - // retries don't "update" anything; they fill in fallbacks that were left - // behind by a previous transition. - return handler === getShellBoundary(); - } + if ( + includesOnlyRetries(workInProgressRootRenderLanes) || + // In this context, an OffscreenLane counts as a Retry + // TODO: It's become increasingly clear that Retries and Offscreen are + // deeply connected. They probably can be unified further. + includesSomeLane(workInProgressRootRenderLanes, OffscreenLane) + ) { + // During a retry, we can suspend rendering if the nearest Suspense boundary + // is the boundary of the "shell", because we're guaranteed not to block + // any new content from appearing. + // + // The reason we must check if this is a retry is because it guarantees + // that suspending the work loop won't block an actual update, because + // retries don't "update" anything; they fill in fallbacks that were left + // behind by a previous transition. + return handler === getShellBoundary(); } // For all other Lanes besides Transitions and Retries, we should not wait