Remove dependency on Offscreen Fiber updateQueue for React Cache (#23229

) We need to use the Offscreen Fiber's update queue for interaction tracing. This PR removes the optimization that React Cache uses to not need to push and pop the cache in special circumstances and defaults to always pushing and popping the cache as long as there was a previous cache.
facebook · Feb 19, 2022 · 40eaa22 · 40eaa22
1 parent caf6d47
commit 40eaa22
Show file tree

Hide file tree

Showing 10 changed files with 150 additions and 146 deletions.
diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js
@@ -234,7 +234,7 @@ import {
   pushRootCachePool,
   CacheContext,
   getSuspendedCachePool,
-  restoreSpawnedCachePool,
+  pushSpawnedCachePool,
   getOffscreenDeferredCachePool,
 } from './ReactFiberCacheComponent.new';
 import {createCapturedValue} from './ReactCapturedValue';
@@ -635,11 +635,6 @@ function updateOffscreenComponent(
   const prevState: OffscreenState | null =
     current !== null ? current.memoizedState : null;
 
-  // If this is not null, this is a cache pool that was carried over from the
-  // previous render. We will push this to the cache pool context so that we can
-  // resume in-flight requests.
-  let spawnedCachePool: SpawnedCachePool | null = null;
-
   if (
     nextProps.mode === 'hidden' ||
     nextProps.mode === 'unstable-defer-without-hiding'
@@ -652,8 +647,16 @@ function updateOffscreenComponent(
         cachePool: null,
       };
       workInProgress.memoizedState = nextState;
+      if (enableCache) {
+        // push the cache pool even though we're going to bail out
+        // because otherwise there'd be a context mismatch
+        if (current !== null) {
+          pushSpawnedCachePool(workInProgress, null);
+        }
+      }
       pushRenderLanes(workInProgress, renderLanes);
     } else if (!includesSomeLane(renderLanes, (OffscreenLane: Lane))) {
+      let spawnedCachePool: SpawnedCachePool | null = null;
       // We're hidden, and we're not rendering at Offscreen. We will bail out
       // and resume this tree later.
       let nextBaseLanes;
@@ -663,9 +666,6 @@ function updateOffscreenComponent(
         if (enableCache) {
           // Save the cache pool so we can resume later.
           spawnedCachePool = getOffscreenDeferredCachePool();
-          // We don't need to push to the cache pool because we're about to
-          // bail out. There won't be a context mismatch because we only pop
-          // the cache pool if `updateQueue` is non-null.
         }
       } else {
         nextBaseLanes = renderLanes;
@@ -681,6 +681,14 @@ function updateOffscreenComponent(
       };
       workInProgress.memoizedState = nextState;
       workInProgress.updateQueue = null;
+      if (enableCache) {
+        // push the cache pool even though we're going to bail out
+        // because otherwise there'd be a context mismatch
+        if (current !== null) {
+          pushSpawnedCachePool(workInProgress, null);
+        }
+      }
+
       // We're about to bail out, but we need to push this to the stack anyway
       // to avoid a push/pop misalignment.
       pushRenderLanes(workInProgress, nextBaseLanes);
@@ -701,19 +709,6 @@ function updateOffscreenComponent(
       // This is the second render. The surrounding visible content has already
       // committed. Now we resume rendering the hidden tree.
 
-      if (enableCache && prevState !== null) {
-        // If the render that spawned this one accessed the cache pool, resume
-        // using the same cache. Unless the parent changed, since that means
-        // there was a refresh.
-        const prevCachePool = prevState.cachePool;
-        if (prevCachePool !== null) {
-          spawnedCachePool = restoreSpawnedCachePool(
-            workInProgress,
-            prevCachePool,
-          );
-        }
-      }
-
       // Rendering at offscreen, so we can clear the base lanes.
       const nextState: OffscreenState = {
         baseLanes: NoLanes,
@@ -723,6 +718,14 @@ function updateOffscreenComponent(
       // Push the lanes that were skipped when we bailed out.
       const subtreeRenderLanes =
         prevState !== null ? prevState.baseLanes : renderLanes;
+      if (enableCache && current !== null) {
+        // If the render that spawned this one accessed the cache pool, resume
+        // using the same cache. Unless the parent changed, since that means
+        // there was a refresh.
+        const prevCachePool = prevState !== null ? prevState.cachePool : null;
+        pushSpawnedCachePool(workInProgress, prevCachePool);
+      }
+
       pushRenderLanes(workInProgress, subtreeRenderLanes);
     }
   } else {
@@ -738,12 +741,7 @@ function updateOffscreenComponent(
         // using the same cache. Unless the parent changed, since that means
         // there was a refresh.
         const prevCachePool = prevState.cachePool;
-        if (prevCachePool !== null) {
-          spawnedCachePool = restoreSpawnedCachePool(
-            workInProgress,
-            prevCachePool,
-          );
-        }
+        pushSpawnedCachePool(workInProgress, prevCachePool);
       }
 
       // Since we're not hidden anymore, reset the state
@@ -753,16 +751,19 @@ function updateOffscreenComponent(
       // special to do. Need to push to the stack regardless, though, to avoid
       // a push/pop misalignment.
       subtreeRenderLanes = renderLanes;
+
+      if (enableCache) {
+        // If the render that spawned this one accessed the cache pool, resume
+        // using the same cache. Unless the parent changed, since that means
+        // there was a refresh.
+        if (current !== null) {
+          pushSpawnedCachePool(workInProgress, null);
+        }
+      }
     }
     pushRenderLanes(workInProgress, subtreeRenderLanes);
   }
 
-  if (enableCache) {
-    // If we have a cache pool from a previous render attempt, then this will be
-    // non-null. We use this to infer whether to push/pop the cache context.
-    workInProgress.updateQueue = spawnedCachePool;
-  }
-
   if (enablePersistentOffscreenHostContainer && supportsPersistence) {
     // In persistent mode, the offscreen children are wrapped in a host node.
     // TODO: Optimize this to use the OffscreenComponent fiber instead of
@@ -2072,6 +2073,7 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
 
     const nextPrimaryChildren = nextProps.children;
     const nextFallbackChildren = nextProps.fallback;
+
     if (showFallback) {
       const fallbackFragment = mountSuspenseFallbackChildren(
         workInProgress,

diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js
@@ -234,7 +234,7 @@ import {
   pushRootCachePool,
   CacheContext,
   getSuspendedCachePool,
-  restoreSpawnedCachePool,
+  pushSpawnedCachePool,
   getOffscreenDeferredCachePool,
 } from './ReactFiberCacheComponent.old';
 import {createCapturedValue} from './ReactCapturedValue';
@@ -635,11 +635,6 @@ function updateOffscreenComponent(
   const prevState: OffscreenState | null =
     current !== null ? current.memoizedState : null;
 
-  // If this is not null, this is a cache pool that was carried over from the
-  // previous render. We will push this to the cache pool context so that we can
-  // resume in-flight requests.
-  let spawnedCachePool: SpawnedCachePool | null = null;
-
   if (
     nextProps.mode === 'hidden' ||
     nextProps.mode === 'unstable-defer-without-hiding'
@@ -652,8 +647,16 @@ function updateOffscreenComponent(
         cachePool: null,
       };
       workInProgress.memoizedState = nextState;
+      if (enableCache) {
+        // push the cache pool even though we're going to bail out
+        // because otherwise there'd be a context mismatch
+        if (current !== null) {
+          pushSpawnedCachePool(workInProgress, null);
+        }
+      }
       pushRenderLanes(workInProgress, renderLanes);
     } else if (!includesSomeLane(renderLanes, (OffscreenLane: Lane))) {
+      let spawnedCachePool: SpawnedCachePool | null = null;
       // We're hidden, and we're not rendering at Offscreen. We will bail out
       // and resume this tree later.
       let nextBaseLanes;
@@ -663,9 +666,6 @@ function updateOffscreenComponent(
         if (enableCache) {
           // Save the cache pool so we can resume later.
           spawnedCachePool = getOffscreenDeferredCachePool();
-          // We don't need to push to the cache pool because we're about to
-          // bail out. There won't be a context mismatch because we only pop
-          // the cache pool if `updateQueue` is non-null.
         }
       } else {
         nextBaseLanes = renderLanes;
@@ -681,6 +681,14 @@ function updateOffscreenComponent(
       };
       workInProgress.memoizedState = nextState;
       workInProgress.updateQueue = null;
+      if (enableCache) {
+        // push the cache pool even though we're going to bail out
+        // because otherwise there'd be a context mismatch
+        if (current !== null) {
+          pushSpawnedCachePool(workInProgress, null);
+        }
+      }
+
       // We're about to bail out, but we need to push this to the stack anyway
       // to avoid a push/pop misalignment.
       pushRenderLanes(workInProgress, nextBaseLanes);
@@ -701,19 +709,6 @@ function updateOffscreenComponent(
       // This is the second render. The surrounding visible content has already
       // committed. Now we resume rendering the hidden tree.
 
-      if (enableCache && prevState !== null) {
-        // If the render that spawned this one accessed the cache pool, resume
-        // using the same cache. Unless the parent changed, since that means
-        // there was a refresh.
-        const prevCachePool = prevState.cachePool;
-        if (prevCachePool !== null) {
-          spawnedCachePool = restoreSpawnedCachePool(
-            workInProgress,
-            prevCachePool,
-          );
-        }
-      }
-
       // Rendering at offscreen, so we can clear the base lanes.
       const nextState: OffscreenState = {
         baseLanes: NoLanes,
@@ -723,6 +718,14 @@ function updateOffscreenComponent(
       // Push the lanes that were skipped when we bailed out.
       const subtreeRenderLanes =
         prevState !== null ? prevState.baseLanes : renderLanes;
+      if (enableCache && current !== null) {
+        // If the render that spawned this one accessed the cache pool, resume
+        // using the same cache. Unless the parent changed, since that means
+        // there was a refresh.
+        const prevCachePool = prevState !== null ? prevState.cachePool : null;
+        pushSpawnedCachePool(workInProgress, prevCachePool);
+      }
+
       pushRenderLanes(workInProgress, subtreeRenderLanes);
     }
   } else {
@@ -738,12 +741,7 @@ function updateOffscreenComponent(
         // using the same cache. Unless the parent changed, since that means
         // there was a refresh.
         const prevCachePool = prevState.cachePool;
-        if (prevCachePool !== null) {
-          spawnedCachePool = restoreSpawnedCachePool(
-            workInProgress,
-            prevCachePool,
-          );
-        }
+        pushSpawnedCachePool(workInProgress, prevCachePool);
       }
 
       // Since we're not hidden anymore, reset the state
@@ -753,16 +751,19 @@ function updateOffscreenComponent(
       // special to do. Need to push to the stack regardless, though, to avoid
       // a push/pop misalignment.
       subtreeRenderLanes = renderLanes;
+
+      if (enableCache) {
+        // If the render that spawned this one accessed the cache pool, resume
+        // using the same cache. Unless the parent changed, since that means
+        // there was a refresh.
+        if (current !== null) {
+          pushSpawnedCachePool(workInProgress, null);
+        }
+      }
     }
     pushRenderLanes(workInProgress, subtreeRenderLanes);
   }
 
-  if (enableCache) {
-    // If we have a cache pool from a previous render attempt, then this will be
-    // non-null. We use this to infer whether to push/pop the cache context.
-    workInProgress.updateQueue = spawnedCachePool;
-  }
-
   if (enablePersistentOffscreenHostContainer && supportsPersistence) {
     // In persistent mode, the offscreen children are wrapped in a host node.
     // TODO: Optimize this to use the OffscreenComponent fiber instead of
@@ -2072,6 +2073,7 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) {
 
     const nextPrimaryChildren = nextProps.children;
     const nextFallbackChildren = nextProps.fallback;
+
     if (showFallback) {
       const fallbackFragment = mountSuspenseFallbackChildren(
         workInProgress,

diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
@@ -198,36 +198,26 @@ export function popRootCachePool(root: FiberRoot, renderLanes: Lanes) {
   // code organization purposes in case that changes.
 }
 
-export function restoreSpawnedCachePool(
+export function pushSpawnedCachePool(
   offscreenWorkInProgress: Fiber,
-  prevCachePool: SpawnedCachePool,
-): SpawnedCachePool | null {
+  prevCachePool: SpawnedCachePool | null,
+): void {
   if (!enableCache) {
-    return (null: any);
+    return;
   }
-  const nextParentCache = isPrimaryRenderer
-    ? CacheContext._currentValue
-    : CacheContext._currentValue2;
-  if (nextParentCache !== prevCachePool.parent) {
-    // There was a refresh. Don't bother restoring anything since the refresh
-    // will override it.
-    return null;
+
+  if (prevCachePool === null) {
+    push(resumedCache, resumedCache.current, offscreenWorkInProgress);
   } else {
-    // No refresh. Resume with the previous cache. New Cache boundaries in the
-    // subtree use this one instead of requesting a fresh one (see
-    // peekCacheFromPool).
     push(resumedCache, prevCachePool.pool, offscreenWorkInProgress);
-
-    // Return the cache pool to signal that we did in fact push it. We will
-    // assign this to the field on the fiber so we know to pop the context.
-    return prevCachePool;
   }
 }
 
 export function popCachePool(workInProgress: Fiber) {
   if (!enableCache) {
     return;
   }
+
   pop(resumedCache, workInProgress);
 }
 

diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.old.js b/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
@@ -198,36 +198,26 @@ export function popRootCachePool(root: FiberRoot, renderLanes: Lanes) {
   // code organization purposes in case that changes.
 }
 
-export function restoreSpawnedCachePool(
+export function pushSpawnedCachePool(
   offscreenWorkInProgress: Fiber,
-  prevCachePool: SpawnedCachePool,
-): SpawnedCachePool | null {
+  prevCachePool: SpawnedCachePool | null,
+): void {
   if (!enableCache) {
-    return (null: any);
+    return;
   }
-  const nextParentCache = isPrimaryRenderer
-    ? CacheContext._currentValue
-    : CacheContext._currentValue2;
-  if (nextParentCache !== prevCachePool.parent) {
-    // There was a refresh. Don't bother restoring anything since the refresh
-    // will override it.
-    return null;
+
+  if (prevCachePool === null) {
+    push(resumedCache, resumedCache.current, offscreenWorkInProgress);
   } else {
-    // No refresh. Resume with the previous cache. New Cache boundaries in the
-    // subtree use this one instead of requesting a fresh one (see
-    // peekCacheFromPool).
     push(resumedCache, prevCachePool.pool, offscreenWorkInProgress);
-
-    // Return the cache pool to signal that we did in fact push it. We will
-    // assign this to the field on the fiber so we know to pop the context.
-    return prevCachePool;
   }
 }
 
 export function popCachePool(workInProgress: Fiber) {
   if (!enableCache) {
     return;
   }
+
   pop(resumedCache, workInProgress);
 }