Change retry priority to "Never" for dehydrated boundaries

packages/react-dom/src/__tests__/ReactDOMServerPartialHydration-test.internal.js

@@ -78,6 +78,7 @@ describe('ReactDOMServerPartialHydration', () => {
     ReactFeatureFlags.enableSuspenseServerRenderer = true;
     ReactFeatureFlags.enableSuspenseCallback = true;
     ReactFeatureFlags.enableFlareAPI = true;
+    ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
 
     React = require('react');
     ReactDOM = require('react-dom');
@@ -2471,4 +2472,85 @@ describe('ReactDOMServerPartialHydration', () => {
 
     document.body.removeChild(container);
   });
+
+  it('finishes normal pri work before continuing to hydrate a retry', async () => {
+    let suspend = false;
+    let resolve;
+    let promise = new Promise(resolvePromise => (resolve = resolvePromise));
+    let ref = React.createRef();
+
+    function Child() {
+      if (suspend) {
+        throw promise;
+      } else {
+        Scheduler.unstable_yieldValue('Child');
+        return 'Hello';
+      }
+    }
+
+    function Sibling() {
+      Scheduler.unstable_yieldValue('Sibling');
+      React.useLayoutEffect(() => {
+        Scheduler.unstable_yieldValue('Commit Sibling');
+      });
+      return 'World';
+    }
+
+    // Avoid rerendering the tree by hoisting it.
+    const tree = (
+      <Suspense fallback="Loading...">
+        <span ref={ref}>
+          <Child />
+        </span>
+      </Suspense>
+    );
+
+    function App({showSibling}) {
+      return (
+        <div>
+          {tree}
+          {showSibling ? <Sibling /> : null}
+        </div>
+      );
+    }
+
+    suspend = false;
+    let finalHTML = ReactDOMServer.renderToString(<App />);
+    expect(Scheduler).toHaveYielded(['Child']);
+
+    let container = document.createElement('div');
+    container.innerHTML = finalHTML;
+
+    suspend = true;
+    let root = ReactDOM.unstable_createRoot(container, {hydrate: true});
+    root.render(<App showSibling={false} />);
+    expect(Scheduler).toFlushAndYield([]);
+
+    expect(ref.current).toBe(null);
+    expect(container.textContent).toBe('Hello');
+
+    // Resolving the promise should continue hydration
+    suspend = false;
+    resolve();
+    await promise;
+
+    Scheduler.unstable_advanceTime(100);
+
+    // Before we have a chance to flush it, we'll also render an update.
+    root.render(<App showSibling={true} />);
+
+    // When we flush we expect the Normal pri render to take priority
+    // over hydration.
+    expect(Scheduler).toFlushAndYieldThrough(['Sibling', 'Commit Sibling']);
+
+    // We shouldn't have hydrated the child yet.
+    expect(ref.current).toBe(null);
+    // But we did have a chance to update the content.
+    expect(container.textContent).toBe('HelloWorld');
+
+    expect(Scheduler).toFlushAndYield(['Child']);
+
+    // Now we're hydrated.
+    expect(ref.current).not.toBe(null);
+  });
 });

packages/react-reconciler/src/ReactFiberBeginWork.js

@@ -1482,7 +1482,7 @@ function validateFunctionComponentInDev(workInProgress: Fiber, Component: any) {
 
 const SUSPENDED_MARKER: SuspenseState = {
   dehydrated: null,
-  retryTime: Never,
+  retryTime: NoWork,
 };
 
 function shouldRemainOnFallback(

packages/react-reconciler/src/ReactFiberSuspenseComponent.js

@@ -35,7 +35,9 @@ export type SuspenseState = {|
   // to check things like isSuspenseInstancePending.
   dehydrated: null | SuspenseInstance,
   // Represents the earliest expiration time we should attempt to hydrate
-  // a dehydrated boundary at. Never is the default.
+  // a dehydrated boundary at.
+  // Never is the default for dehydrated boundaries.
+  // NoWork is the default for normal boundaries, which turns into "normal" pri.
   retryTime: ExpirationTime,
 |};
 

packages/react-reconciler/src/ReactFiberWorkLoop.js

@@ -741,17 +741,19 @@ function finishConcurrentRender(
     // statement, but eslint doesn't know about invariant, so it complains
     // if I do. eslint-disable-next-line no-fallthrough
     case RootErrored: {
-      if (expirationTime !== Idle) {
-        // If this was an async render, the error may have happened due to
-        // a mutation in a concurrent event. Try rendering one more time,
-        // synchronously, to see if the error goes away. If there are
-        // lower priority updates, let's include those, too, in case they
-        // fix the inconsistency. Render at Idle to include all updates.
-        markRootExpiredAtTime(root, Idle);
-        break;
-      }
-      // Commit the root in its errored state.
-      commitRoot(root);
+      // If this was an async render, the error may have happened due to
+      // a mutation in a concurrent event. Try rendering one more time,
+      // synchronously, to see if the error goes away. If there are
+      // lower priority updates, let's include those, too, in case they
+      // fix the inconsistency. Render at Idle to include all updates.
+      // If it was Idle or Never or some not-yet-invented time, render
+      // at that time.
+      markRootExpiredAtTime(
+        root,
+        expirationTime > Idle ? Idle : expirationTime,
+      );
+      // We assume that this second render pass will be synchronous
+      // and therefore not hit this path again.
       break;
     }
     case RootSuspended: {
@@ -2376,7 +2378,7 @@ function retryTimedOutBoundary(
   // previously was rendered in its fallback state. One of the promises that
   // suspended it has resolved, which means at least part of the tree was
   // likely unblocked. Try rendering again, at a new expiration time.
-  if (retryTime === Never) {
+  if (retryTime === NoWork) {
     const suspenseConfig = null; // Retries don't carry over the already committed update.
     const currentTime = requestCurrentTime();
     retryTime = computeExpirationForFiber(
@@ -2395,15 +2397,15 @@ function retryTimedOutBoundary(
 
 export function retryDehydratedSuspenseBoundary(boundaryFiber: Fiber) {
   const suspenseState: null | SuspenseState = boundaryFiber.memoizedState;
-  let retryTime = Never;
+  let retryTime = NoWork;
   if (suspenseState !== null) {
     retryTime = suspenseState.retryTime;
   }
   retryTimedOutBoundary(boundaryFiber, retryTime);
 }
 
 export function resolveRetryThenable(boundaryFiber: Fiber, thenable: Thenable) {
-  let retryTime = Never; // Default
+  let retryTime = NoWork; // Default
   let retryCache: WeakSet<Thenable> | Set<Thenable> | null;
   if (enableSuspenseServerRenderer) {
     switch (boundaryFiber.tag) {