diff --git a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js index 91321ea6dd5bf..048d6c9db2972 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerSelectiveHydration-test.internal.js @@ -1695,4 +1695,81 @@ describe('ReactDOMServerSelectiveHydration', () => { expect(triggeredParent).toBe(false); expect(triggeredChild).toBe(false); }); + + it('can attempt sync hydration if suspended root is still concurrently rendering', async () => { + let suspend = false; + let resolve; + const promise = new Promise(resolvePromise => (resolve = resolvePromise)); + function Child({text}) { + if (suspend) { + throw promise; + } + Scheduler.unstable_yieldValue(text); + return ( + { + e.preventDefault(); + Scheduler.unstable_yieldValue('Clicked ' + text); + }}> + {text} + + ); + } + + function App() { + Scheduler.unstable_yieldValue('App'); + return ( +
+ +
+ ); + } + + const finalHTML = ReactDOMServer.renderToString(); + + expect(Scheduler).toHaveYielded(['App', 'A']); + + const container = document.createElement('div'); + // We need this to be in the document since we'll dispatch events on it. + document.body.appendChild(container); + + container.innerHTML = finalHTML; + + const span = container.getElementsByTagName('span')[0]; + + // We suspend on the client. + suspend = true; + + React.startTransition(() => { + ReactDOMClient.hydrateRoot(container, ); + }); + expect(Scheduler).toFlushAndYieldThrough(['App']); + + // This should attempt to synchronously hydrate the root, then pause + // because it still suspended + const result = dispatchClickEvent(span); + expect(Scheduler).toHaveYielded(['App']); + // The event should not have been cancelled because we didn't hydrate. + expect(result).toBe(true); + + // Finish loading the data + await act(async () => { + suspend = false; + await resolve(); + }); + + // The app should have successfully hydrated and rendered + if ( + gate( + flags => + flags.enableCapturePhaseSelectiveHydrationWithoutDiscreteEventReplay, + ) + ) { + expect(Scheduler).toHaveYielded(['App', 'A']); + } else { + expect(Scheduler).toHaveYielded(['App', 'A', 'Clicked A']); + } + + document.body.removeChild(container); + }); }); diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index 97e02cd8dec6b..e05b037d9f2c8 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -69,13 +69,13 @@ import { includesSomeLane, mergeLanes, pickArbitraryLane, - includesSyncLane, } from './ReactFiberLane.new'; import { getIsHydrating, markDidThrowWhileHydratingDEV, queueHydrationError, } from './ReactFiberHydrationContext.new'; +import {ConcurrentRoot} from './ReactRootTags'; function createRootErrorUpdate( fiber: Fiber, @@ -421,32 +421,26 @@ function throwException( // No boundary was found. Unless this is a sync update, this is OK. // We can suspend and wait for more data to arrive. - if (!includesSyncLane(rootRenderLanes)) { - // This is not a sync update. 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.) + if (root.tag === ConcurrentRoot) { + // In a concurrent root, suspending without a Suspense boundary is + // allowed. It will suspend indefinitely without committing. // - // This case also applies to initial hydration. + // TODO: Should we have different behavior for discrete updates? What + // about flushSync? Maybe it should put the tree into an inert state, + // and potentially log a warning. Revisit this for a future release. attachPingListener(root, wakeable, rootRenderLanes); renderDidSuspendDelayIfPossible(); return; + } else { + // In a legacy root, suspending without a boundary is always an error. + const uncaughtSuspenseError = new Error( + 'A component suspended while responding to synchronous input. This ' + + 'will cause the UI to be replaced with a loading indicator. To ' + + 'fix, updates that suspend should be wrapped ' + + 'with startTransition.', + ); + value = uncaughtSuspenseError; } - - // This is a sync/discrete update. We treat this case like an error - // because discrete renders are expected to produce a complete tree - // synchronously to maintain consistency with external state. - const uncaughtSuspenseError = new Error( - 'A component suspended while responding to synchronous input. This ' + - 'will cause the UI to be replaced with a loading indicator. To ' + - 'fix, updates that suspend should be wrapped ' + - 'with startTransition.', - ); - - // 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 f20047c8f05d1..fcd35f8b32b3d 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -69,13 +69,13 @@ import { includesSomeLane, mergeLanes, pickArbitraryLane, - includesSyncLane, } from './ReactFiberLane.old'; import { getIsHydrating, markDidThrowWhileHydratingDEV, queueHydrationError, } from './ReactFiberHydrationContext.old'; +import {ConcurrentRoot} from './ReactRootTags'; function createRootErrorUpdate( fiber: Fiber, @@ -421,32 +421,26 @@ function throwException( // No boundary was found. Unless this is a sync update, this is OK. // We can suspend and wait for more data to arrive. - if (!includesSyncLane(rootRenderLanes)) { - // This is not a sync update. 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.) + if (root.tag === ConcurrentRoot) { + // In a concurrent root, suspending without a Suspense boundary is + // allowed. It will suspend indefinitely without committing. // - // This case also applies to initial hydration. + // TODO: Should we have different behavior for discrete updates? What + // about flushSync? Maybe it should put the tree into an inert state, + // and potentially log a warning. Revisit this for a future release. attachPingListener(root, wakeable, rootRenderLanes); renderDidSuspendDelayIfPossible(); return; + } else { + // In a legacy root, suspending without a boundary is always an error. + const uncaughtSuspenseError = new Error( + 'A component suspended while responding to synchronous input. This ' + + 'will cause the UI to be replaced with a loading indicator. To ' + + 'fix, updates that suspend should be wrapped ' + + 'with startTransition.', + ); + value = uncaughtSuspenseError; } - - // This is a sync/discrete update. We treat this case like an error - // because discrete renders are expected to produce a complete tree - // synchronously to maintain consistency with external state. - const uncaughtSuspenseError = new Error( - 'A component suspended while responding to synchronous input. This ' + - 'will cause the UI to be replaced with a loading indicator. To ' + - 'fix, updates that suspend should be wrapped ' + - 'with startTransition.', - ); - - // 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/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 38994cdef5291..172b956d8fe7f 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -1063,10 +1063,6 @@ function performConcurrentWorkOnRoot(root, didTimeout) { // 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. @@ -1111,6 +1107,9 @@ function performConcurrentWorkOnRoot(root, didTimeout) { ensureRootIsScheduled(root, now()); throw fatalError; } + + // FIXME: Need to check for RootDidNotComplete again. The factoring here + // isn't ideal. } // We now have a consistent tree. The next step is either to commit it, @@ -1473,7 +1472,12 @@ function performSyncWorkOnRoot(root) { } if (exitStatus === RootDidNotComplete) { - throw new Error('Root did not complete. This is a bug in React.'); + // 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. + markRootSuspended(root, lanes); + ensureRootIsScheduled(root, now()); + return null; } // We now have a consistent tree. Because this is a sync render, we diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index 1dacb1fd5685e..a439b90e40843 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -1063,10 +1063,6 @@ function performConcurrentWorkOnRoot(root, didTimeout) { // 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. @@ -1111,6 +1107,9 @@ function performConcurrentWorkOnRoot(root, didTimeout) { ensureRootIsScheduled(root, now()); throw fatalError; } + + // FIXME: Need to check for RootDidNotComplete again. The factoring here + // isn't ideal. } // We now have a consistent tree. The next step is either to commit it, @@ -1473,7 +1472,12 @@ function performSyncWorkOnRoot(root) { } if (exitStatus === RootDidNotComplete) { - throw new Error('Root did not complete. This is a bug in React.'); + // 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. + markRootSuspended(root, lanes); + ensureRootIsScheduled(root, now()); + return null; } // We now have a consistent tree. Because this is a sync render, we diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js index 07838b223e687..a0a7089157440 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.js @@ -1012,15 +1012,21 @@ describe('ReactSuspenseWithNoopRenderer', () => { }); // @gate enableCache - it('errors when an update suspends without a placeholder during a sync update', () => { - // This is an error because sync/discrete updates are expected to produce - // a complete tree immediately to maintain consistency with external state - // — we can't delay the commit. + it('in concurrent mode, does not error when an update suspends without a Suspense boundary during a sync update', () => { + // NOTE: We may change this to be a warning in the future. expect(() => { ReactNoop.flushSync(() => { ReactNoop.render(); }); - }).toThrow('A component suspended while responding to synchronous input.'); + }).not.toThrow(); + }); + + // @gate enableCache + it('in legacy mode, errors when an update suspends without a Suspense boundary during a sync update', () => { + const root = ReactNoop.createLegacyRoot(); + expect(() => root.render()).toThrow( + 'A component suspended while responding to synchronous input.', + ); }); // @gate enableCache