From c72b1da62aa69edb4dd7cafab65c404bd96e24c8 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Wed, 29 Sep 2021 14:46:17 -0700
Subject: [PATCH 01/25] Release pooled cache reference in complete/unwind

---
 .../src/ReactFiberCacheComponent.new.js                | 10 ++++++----
 .../src/ReactFiberCacheComponent.old.js                | 10 ++++++----
 2 files changed, 12 insertions(+), 8 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
index 042a8b2efc0..0d284c683ed 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
@@ -99,14 +99,16 @@ export function popRootCachePool(root: FiberRoot, renderLanes: Lanes) {
     return;
   }
   // The `pooledCache` variable points to the cache that was used for new
-  // cache boundaries during this render, if any. Stash it on the root so that
-  // parallel transitions may share the same cache. We will clear this field
-  // once all the transitions that depend on it (which we track with
-  // `pooledCacheLanes`) have committed.
+  // cache boundaries during this render, if any. Move ownership of the
+  // cache to the root so that parallel transitions may share the same
+  // cache. We will clear this field once all the transitions that depend
+  // on it (which we track with `pooledCacheLanes`) have committed.
   root.pooledCache = pooledCache;
   if (pooledCache !== null) {
     root.pooledCacheLanes |= renderLanes;
   }
+  // set to null, conceptually we are moving ownership to the root
+  pooledCache = null;
 }
 
 export function restoreSpawnedCachePool(
diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.old.js b/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
index 2bc64254d30..dd450fff76b 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
@@ -99,14 +99,16 @@ export function popRootCachePool(root: FiberRoot, renderLanes: Lanes) {
     return;
   }
   // The `pooledCache` variable points to the cache that was used for new
-  // cache boundaries during this render, if any. Stash it on the root so that
-  // parallel transitions may share the same cache. We will clear this field
-  // once all the transitions that depend on it (which we track with
-  // `pooledCacheLanes`) have committed.
+  // cache boundaries during this render, if any. Move ownership of the
+  // cache to the root so that parallel transitions may share the same
+  // cache. We will clear this field once all the transitions that depend
+  // on it (which we track with `pooledCacheLanes`) have committed.
   root.pooledCache = pooledCache;
   if (pooledCache !== null) {
     root.pooledCacheLanes |= renderLanes;
   }
+  // set to null, conceptually we are moving ownership to the root
+  pooledCache = null;
 }
 
 export function restoreSpawnedCachePool(

From 8d5b37bdf6f1b116e4a2f054e000571fab35d40a Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Wed, 29 Sep 2021 15:29:32 -0700
Subject: [PATCH 02/25] Expand Cache type w ref count

---
 .../src/ReactFiberCacheComponent.new.js       | 41 ++++++++++++++++++-
 .../src/ReactFiberCacheComponent.old.js       | 41 ++++++++++++++++++-
 .../src/ReactFiberHooks.new.js                | 10 ++---
 .../src/ReactFiberHooks.old.js                | 10 ++---
 .../src/ReactFiberRoot.new.js                 |  3 +-
 .../src/ReactFiberRoot.old.js                 |  3 +-
 6 files changed, 92 insertions(+), 16 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
index 0d284c683ed..61a8bea0f81 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
@@ -19,7 +19,10 @@ import {isPrimaryRenderer} from './ReactFiberHostConfig';
 import {createCursor, push, pop} from './ReactFiberStack.new';
 import {pushProvider, popProvider} from './ReactFiberNewContext.new';
 
-export type Cache = Map<() => mixed, mixed>;
+export type Cache = {|
+  data: Map<() => mixed, mixed>,
+  refCount: number,
+|};
 
 export type CacheComponentState = {|
   +parent: Cache,
@@ -57,6 +60,40 @@ let pooledCache: Cache | null = null;
 // cache from the render that suspended.
 const prevFreshCacheOnStack: StackCursor<Cache | null> = createCursor(null);
 
+// Creates a new empty Cache instance with a ref-count of 1: the caller is
+// the implicit "owner". Note that retain/release must be called to ensure
+// that the ref count is accurate:
+// * Call retainCache() when a new reference to the cache instance is created.
+//   This *excludes* the original reference created w createCache().
+// * Call releaseCache() when any reference to the cache is "released" (ie
+//   when the reference is no longer reachable). This *includes* the original
+//   reference created w createCache().
+export function createCache(): Cache {
+  return {
+    data: new Map(),
+    refCount: 1,
+  };
+}
+
+export function retainCache(cache: Cache) {
+  cache.refCount++;
+}
+
+export function releaseCache(cache: Cache) {
+  cache.refCount--;
+  if (__DEV__) {
+    if (cache.refCount < 0) {
+      throw new Error(
+        'Error in React: cache reference count should not be negative',
+      );
+    }
+  }
+  if (cache.refCount === 0) {
+    // TODO: cleanup the cache, considering scheduling and error handling for any
+    // event listeners that get triggered.
+  }
+}
+
 export function pushCacheProvider(workInProgress: Fiber, cache: Cache) {
   if (!enableCache) {
     return;
@@ -79,7 +116,7 @@ export function requestCacheFromPool(renderLanes: Lanes): Cache {
     return pooledCache;
   }
   // Create a fresh cache.
-  pooledCache = new Map();
+  pooledCache = createCache();
   return pooledCache;
 }
 
diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.old.js b/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
index dd450fff76b..3b0e873a794 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
@@ -19,7 +19,10 @@ import {isPrimaryRenderer} from './ReactFiberHostConfig';
 import {createCursor, push, pop} from './ReactFiberStack.old';
 import {pushProvider, popProvider} from './ReactFiberNewContext.old';
 
-export type Cache = Map<() => mixed, mixed>;
+export type Cache = {|
+  data: Map<() => mixed, mixed>,
+  refCount: number,
+|};
 
 export type CacheComponentState = {|
   +parent: Cache,
@@ -57,6 +60,40 @@ let pooledCache: Cache | null = null;
 // cache from the render that suspended.
 const prevFreshCacheOnStack: StackCursor<Cache | null> = createCursor(null);
 
+// Creates a new empty Cache instance with a ref-count of 1: the caller is
+// the implicit "owner". Note that retain/release must be called to ensure
+// that the ref count is accurate:
+// * Call retainCache() when a new reference to the cache instance is created.
+//   This *excludes* the original reference created w createCache().
+// * Call releaseCache() when any reference to the cache is "released" (ie
+//   when the reference is no longer reachable). This *includes* the original
+//   reference created w createCache().
+export function createCache(): Cache {
+  return {
+    data: new Map(),
+    refCount: 1,
+  };
+}
+
+export function retainCache(cache: Cache) {
+  cache.refCount++;
+}
+
+export function releaseCache(cache: Cache) {
+  cache.refCount--;
+  if (__DEV__) {
+    if (cache.refCount < 0) {
+      throw new Error(
+        'Error in React: cache reference count should not be negative',
+      );
+    }
+  }
+  if (cache.refCount === 0) {
+    // TODO: cleanup the cache, considering scheduling and error handling for any
+    // event listeners that get triggered.
+  }
+}
+
 export function pushCacheProvider(workInProgress: Fiber, cache: Cache) {
   if (!enableCache) {
     return;
@@ -79,7 +116,7 @@ export function requestCacheFromPool(renderLanes: Lanes): Cache {
     return pooledCache;
   }
   // Create a fresh cache.
-  pooledCache = new Map();
+  pooledCache = createCache();
   return pooledCache;
 }
 
diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js
index d6123454972..213b11b6047 100644
--- a/packages/react-reconciler/src/ReactFiberHooks.new.js
+++ b/packages/react-reconciler/src/ReactFiberHooks.new.js
@@ -111,7 +111,7 @@ import {
 import {getIsRendering} from './ReactCurrentFiber';
 import {logStateUpdateScheduled} from './DebugTracing';
 import {markStateUpdateScheduled} from './SchedulingProfiler';
-import {CacheContext} from './ReactFiberCacheComponent.new';
+import {createCache, CacheContext} from './ReactFiberCacheComponent.new';
 import {
   createUpdate as createLegacyQueueUpdate,
   enqueueUpdate as enqueueLegacyQueueUpdate,
@@ -2128,11 +2128,11 @@ function refreshCache<T>(fiber: Fiber, seedKey: ?() => T, seedValue: T) {
           entangleLegacyQueueTransitions(root, provider, lane);
         }
 
-        const seededCache = new Map();
+        const seededCache = createCache();
         if (seedKey !== null && seedKey !== undefined && root !== null) {
           // Seed the cache with the value passed by the caller. This could be
           // from a server mutation, or it could be a streaming response.
-          seededCache.set(seedKey, seedValue);
+          seededCache.data.set(seedKey, seedValue);
         }
 
         // Schedule an update on the cache boundary to trigger a refresh.
@@ -2386,10 +2386,10 @@ function getCacheForType<T>(resourceType: () => T): T {
     invariant(false, 'Not implemented.');
   }
   const cache: Cache = readContext(CacheContext);
-  let cacheForType: T | void = (cache.get(resourceType): any);
+  let cacheForType: T | void = (cache.data.get(resourceType): any);
   if (cacheForType === undefined) {
     cacheForType = resourceType();
-    cache.set(resourceType, cacheForType);
+    cache.data.set(resourceType, cacheForType);
   }
   return cacheForType;
 }
diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js
index d4dacbc8833..5c28c5ff857 100644
--- a/packages/react-reconciler/src/ReactFiberHooks.old.js
+++ b/packages/react-reconciler/src/ReactFiberHooks.old.js
@@ -111,7 +111,7 @@ import {
 import {getIsRendering} from './ReactCurrentFiber';
 import {logStateUpdateScheduled} from './DebugTracing';
 import {markStateUpdateScheduled} from './SchedulingProfiler';
-import {CacheContext} from './ReactFiberCacheComponent.old';
+import {createCache, CacheContext} from './ReactFiberCacheComponent.old';
 import {
   createUpdate as createLegacyQueueUpdate,
   enqueueUpdate as enqueueLegacyQueueUpdate,
@@ -2128,11 +2128,11 @@ function refreshCache<T>(fiber: Fiber, seedKey: ?() => T, seedValue: T) {
           entangleLegacyQueueTransitions(root, provider, lane);
         }
 
-        const seededCache = new Map();
+        const seededCache = createCache();
         if (seedKey !== null && seedKey !== undefined && root !== null) {
           // Seed the cache with the value passed by the caller. This could be
           // from a server mutation, or it could be a streaming response.
-          seededCache.set(seedKey, seedValue);
+          seededCache.data.set(seedKey, seedValue);
         }
 
         // Schedule an update on the cache boundary to trigger a refresh.
@@ -2386,10 +2386,10 @@ function getCacheForType<T>(resourceType: () => T): T {
     invariant(false, 'Not implemented.');
   }
   const cache: Cache = readContext(CacheContext);
-  let cacheForType: T | void = (cache.get(resourceType): any);
+  let cacheForType: T | void = (cache.data.get(resourceType): any);
   if (cacheForType === undefined) {
     cacheForType = resourceType();
-    cache.set(resourceType, cacheForType);
+    cache.data.set(resourceType, cacheForType);
   }
   return cacheForType;
 }
diff --git a/packages/react-reconciler/src/ReactFiberRoot.new.js b/packages/react-reconciler/src/ReactFiberRoot.new.js
index db012ad1db1..75a621b610f 100644
--- a/packages/react-reconciler/src/ReactFiberRoot.new.js
+++ b/packages/react-reconciler/src/ReactFiberRoot.new.js
@@ -28,6 +28,7 @@ import {
 } from 'shared/ReactFeatureFlags';
 import {initializeUpdateQueue} from './ReactUpdateQueue.new';
 import {LegacyRoot, ConcurrentRoot} from './ReactRootTags';
+import {createCache} from './ReactFiberCacheComponent.new';
 
 function FiberRootNode(containerInfo, tag, hydrate) {
   this.tag = tag;
@@ -117,7 +118,7 @@ export function createFiberRoot(
   uninitializedFiber.stateNode = root;
 
   if (enableCache) {
-    const initialCache = new Map();
+    const initialCache = createCache();
     root.pooledCache = initialCache;
     const initialState = {
       element: null,
diff --git a/packages/react-reconciler/src/ReactFiberRoot.old.js b/packages/react-reconciler/src/ReactFiberRoot.old.js
index 03332fd5456..982e0f82119 100644
--- a/packages/react-reconciler/src/ReactFiberRoot.old.js
+++ b/packages/react-reconciler/src/ReactFiberRoot.old.js
@@ -28,6 +28,7 @@ import {
 } from 'shared/ReactFeatureFlags';
 import {initializeUpdateQueue} from './ReactUpdateQueue.old';
 import {LegacyRoot, ConcurrentRoot} from './ReactRootTags';
+import {createCache} from './ReactFiberCacheComponent.old';
 
 function FiberRootNode(containerInfo, tag, hydrate) {
   this.tag = tag;
@@ -117,7 +118,7 @@ export function createFiberRoot(
   uninitializedFiber.stateNode = root;
 
   if (enableCache) {
-    const initialCache = new Map();
+    const initialCache = createCache();
     root.pooledCache = initialCache;
     const initialState = {
       element: null,

From 78c4bdc23503c54efde2771042509d42a1485bc5 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Wed, 29 Sep 2021 16:30:38 -0700
Subject: [PATCH 03/25] Add AbortController to cache object and abort when
 refcount=0

---
 .../react-reconciler/src/ReactFiberCacheComponent.new.js     | 5 ++++-
 .../react-reconciler/src/ReactFiberCacheComponent.old.js     | 5 ++++-
 scripts/jest/setupEnvironment.js                             | 4 ++++
 3 files changed, 12 insertions(+), 2 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
index 61a8bea0f81..e3738730321 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
@@ -20,6 +20,7 @@ import {createCursor, push, pop} from './ReactFiberStack.new';
 import {pushProvider, popProvider} from './ReactFiberNewContext.new';
 
 export type Cache = {|
+  controller: AbortController,
   data: Map<() => mixed, mixed>,
   refCount: number,
 |};
@@ -70,6 +71,7 @@ const prevFreshCacheOnStack: StackCursor<Cache | null> = createCursor(null);
 //   reference created w createCache().
 export function createCache(): Cache {
   return {
+    controller: new AbortController(),
     data: new Map(),
     refCount: 1,
   };
@@ -89,8 +91,9 @@ export function releaseCache(cache: Cache) {
     }
   }
   if (cache.refCount === 0) {
-    // TODO: cleanup the cache, considering scheduling and error handling for any
+    // TODO: considering scheduling and error handling for any
     // event listeners that get triggered.
+    cache.controller.abort();
   }
 }
 
diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.old.js b/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
index 3b0e873a794..df5a49e4aae 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
@@ -20,6 +20,7 @@ import {createCursor, push, pop} from './ReactFiberStack.old';
 import {pushProvider, popProvider} from './ReactFiberNewContext.old';
 
 export type Cache = {|
+  controller: AbortController,
   data: Map<() => mixed, mixed>,
   refCount: number,
 |};
@@ -70,6 +71,7 @@ const prevFreshCacheOnStack: StackCursor<Cache | null> = createCursor(null);
 //   reference created w createCache().
 export function createCache(): Cache {
   return {
+    controller: new AbortController(),
     data: new Map(),
     refCount: 1,
   };
@@ -89,8 +91,9 @@ export function releaseCache(cache: Cache) {
     }
   }
   if (cache.refCount === 0) {
-    // TODO: cleanup the cache, considering scheduling and error handling for any
+    // TODO: considering scheduling and error handling for any
     // event listeners that get triggered.
+    cache.controller.abort();
   }
 }
 
diff --git a/scripts/jest/setupEnvironment.js b/scripts/jest/setupEnvironment.js
index 2ba88b15616..d2d510088c4 100644
--- a/scripts/jest/setupEnvironment.js
+++ b/scripts/jest/setupEnvironment.js
@@ -1,5 +1,7 @@
 /* eslint-disable */
 
+const AbortController = require('abort-controller');
+
 const NODE_ENV = process.env.NODE_ENV;
 if (NODE_ENV !== 'development' && NODE_ENV !== 'production') {
   throw new Error('NODE_ENV must either be set to development or production.');
@@ -21,6 +23,8 @@ global.__EXPERIMENTAL__ =
 
 global.__VARIANT__ = !!process.env.VARIANT;
 
+global.AbortController = AbortController;
+
 if (typeof window !== 'undefined') {
   global.requestIdleCallback = function(callback) {
     return setTimeout(() => {

From 93020d4c17febda862b5a75f34c2ac120b4b3d7e Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Thu, 30 Sep 2021 13:19:10 -0700
Subject: [PATCH 04/25] Add getCacheSignal() and release pooled cache in
 commitRoot()

---
 .../src/server/ReactPartialRendererHooks.js   |  5 +++++
 .../src/ReactFiberHooks.new.js                | 19 +++++++++++++++++++
 .../src/ReactFiberHooks.old.js                | 19 +++++++++++++++++++
 .../src/ReactFiberLane.new.js                 | 10 ----------
 .../src/ReactFiberLane.old.js                 | 10 ----------
 .../src/ReactFiberWorkLoop.new.js             | 13 +++++++++++++
 .../src/ReactFiberWorkLoop.old.js             | 13 +++++++++++++
 .../src/ReactInternalTypes.js                 |  1 +
 .../src/__tests__/ReactCache-test.js          |  7 +++++++
 packages/react/index.js                       |  1 +
 packages/react/src/React.js                   |  2 ++
 packages/react/src/ReactHooks.js              |  6 ++++++
 12 files changed, 86 insertions(+), 20 deletions(-)

diff --git a/packages/react-dom/src/server/ReactPartialRendererHooks.js b/packages/react-dom/src/server/ReactPartialRendererHooks.js
index edc5df3dce4..2edc53c6e53 100644
--- a/packages/react-dom/src/server/ReactPartialRendererHooks.js
+++ b/packages/react-dom/src/server/ReactPartialRendererHooks.js
@@ -215,6 +215,10 @@ export function resetHooksState(): void {
   workInProgressHook = null;
 }
 
+function getCacheSignal() {
+  invariant(false, 'Not implemented.');
+}
+
 function getCacheForType<T>(resourceType: () => T): T {
   invariant(false, 'Not implemented.');
 }
@@ -550,6 +554,7 @@ export const Dispatcher: DispatcherType = {
 };
 
 if (enableCache) {
+  Dispatcher.getCacheSignal = getCacheSignal;
   Dispatcher.getCacheForType = getCacheForType;
   Dispatcher.useCacheRefresh = useCacheRefresh;
 }
diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js
index 213b11b6047..09cbc4d24fd 100644
--- a/packages/react-reconciler/src/ReactFiberHooks.new.js
+++ b/packages/react-reconciler/src/ReactFiberHooks.new.js
@@ -2381,6 +2381,14 @@ function markUpdateInDevTools(fiber, lane, action) {
   }
 }
 
+function getCacheSignal(): AbortSignal {
+  if (!enableCache) {
+    invariant(false, 'Not implemented.');
+  }
+  const cache: Cache = readContext(CacheContext);
+  return cache.controller.signal;
+}
+
 function getCacheForType<T>(resourceType: () => T): T {
   if (!enableCache) {
     invariant(false, 'Not implemented.');
@@ -2417,6 +2425,7 @@ export const ContextOnlyDispatcher: Dispatcher = {
   unstable_isNewReconciler: enableNewReconciler,
 };
 if (enableCache) {
+  (ContextOnlyDispatcher: Dispatcher).getCacheSignal = getCacheSignal;
   (ContextOnlyDispatcher: Dispatcher).getCacheForType = getCacheForType;
   (ContextOnlyDispatcher: Dispatcher).useCacheRefresh = throwInvalidHookError;
 }
@@ -2444,6 +2453,7 @@ const HooksDispatcherOnMount: Dispatcher = {
   unstable_isNewReconciler: enableNewReconciler,
 };
 if (enableCache) {
+  (HooksDispatcherOnMount: Dispatcher).getCacheSignal = getCacheSignal;
   (HooksDispatcherOnMount: Dispatcher).getCacheForType = getCacheForType;
   (HooksDispatcherOnMount: Dispatcher).useCacheRefresh = mountRefresh;
 }
@@ -2471,6 +2481,7 @@ const HooksDispatcherOnUpdate: Dispatcher = {
   unstable_isNewReconciler: enableNewReconciler,
 };
 if (enableCache) {
+  (HooksDispatcherOnUpdate: Dispatcher).getCacheSignal = getCacheSignal;
   (HooksDispatcherOnUpdate: Dispatcher).getCacheForType = getCacheForType;
   (HooksDispatcherOnUpdate: Dispatcher).useCacheRefresh = updateRefresh;
 }
@@ -2498,6 +2509,7 @@ const HooksDispatcherOnRerender: Dispatcher = {
   unstable_isNewReconciler: enableNewReconciler,
 };
 if (enableCache) {
+  (HooksDispatcherOnRerender: Dispatcher).getCacheSignal = getCacheSignal;
   (HooksDispatcherOnRerender: Dispatcher).getCacheForType = getCacheForType;
   (HooksDispatcherOnRerender: Dispatcher).useCacheRefresh = updateRefresh;
 }
@@ -2668,6 +2680,7 @@ if (__DEV__) {
     unstable_isNewReconciler: enableNewReconciler,
   };
   if (enableCache) {
+    (HooksDispatcherOnMountInDEV: Dispatcher).getCacheSignal = getCacheSignal;
     (HooksDispatcherOnMountInDEV: Dispatcher).getCacheForType = getCacheForType;
     (HooksDispatcherOnMountInDEV: Dispatcher).useCacheRefresh = function useCacheRefresh() {
       currentHookNameInDev = 'useCacheRefresh';
@@ -2809,6 +2822,7 @@ if (__DEV__) {
     unstable_isNewReconciler: enableNewReconciler,
   };
   if (enableCache) {
+    (HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher).getCacheSignal = getCacheSignal;
     (HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher).getCacheForType = getCacheForType;
     (HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher).useCacheRefresh = function useCacheRefresh() {
       currentHookNameInDev = 'useCacheRefresh';
@@ -2950,6 +2964,7 @@ if (__DEV__) {
     unstable_isNewReconciler: enableNewReconciler,
   };
   if (enableCache) {
+    (HooksDispatcherOnUpdateInDEV: Dispatcher).getCacheSignal = getCacheSignal;
     (HooksDispatcherOnUpdateInDEV: Dispatcher).getCacheForType = getCacheForType;
     (HooksDispatcherOnUpdateInDEV: Dispatcher).useCacheRefresh = function useCacheRefresh() {
       currentHookNameInDev = 'useCacheRefresh';
@@ -3092,6 +3107,7 @@ if (__DEV__) {
     unstable_isNewReconciler: enableNewReconciler,
   };
   if (enableCache) {
+    (HooksDispatcherOnRerenderInDEV: Dispatcher).getCacheSignal = getCacheSignal;
     (HooksDispatcherOnRerenderInDEV: Dispatcher).getCacheForType = getCacheForType;
     (HooksDispatcherOnRerenderInDEV: Dispatcher).useCacheRefresh = function useCacheRefresh() {
       currentHookNameInDev = 'useCacheRefresh';
@@ -3250,6 +3266,7 @@ if (__DEV__) {
     unstable_isNewReconciler: enableNewReconciler,
   };
   if (enableCache) {
+    (InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher).getCacheSignal = getCacheSignal;
     (InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher).getCacheForType = getCacheForType;
     (InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher).useCacheRefresh = function useCacheRefresh() {
       currentHookNameInDev = 'useCacheRefresh';
@@ -3408,6 +3425,7 @@ if (__DEV__) {
     unstable_isNewReconciler: enableNewReconciler,
   };
   if (enableCache) {
+    (InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher).getCacheSignal = getCacheSignal;
     (InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher).getCacheForType = getCacheForType;
     (InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher).useCacheRefresh = function useCacheRefresh() {
       currentHookNameInDev = 'useCacheRefresh';
@@ -3567,6 +3585,7 @@ if (__DEV__) {
     unstable_isNewReconciler: enableNewReconciler,
   };
   if (enableCache) {
+    (InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher).getCacheSignal = getCacheSignal;
     (InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher).getCacheForType = getCacheForType;
     (InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher).useCacheRefresh = function useCacheRefresh() {
       currentHookNameInDev = 'useCacheRefresh';
diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js
index 5c28c5ff857..cbc861a2921 100644
--- a/packages/react-reconciler/src/ReactFiberHooks.old.js
+++ b/packages/react-reconciler/src/ReactFiberHooks.old.js
@@ -2381,6 +2381,14 @@ function markUpdateInDevTools(fiber, lane, action) {
   }
 }
 
+function getCacheSignal(): AbortSignal {
+  if (!enableCache) {
+    invariant(false, 'Not implemented.');
+  }
+  const cache: Cache = readContext(CacheContext);
+  return cache.controller.signal;
+}
+
 function getCacheForType<T>(resourceType: () => T): T {
   if (!enableCache) {
     invariant(false, 'Not implemented.');
@@ -2417,6 +2425,7 @@ export const ContextOnlyDispatcher: Dispatcher = {
   unstable_isNewReconciler: enableNewReconciler,
 };
 if (enableCache) {
+  (ContextOnlyDispatcher: Dispatcher).getCacheSignal = getCacheSignal;
   (ContextOnlyDispatcher: Dispatcher).getCacheForType = getCacheForType;
   (ContextOnlyDispatcher: Dispatcher).useCacheRefresh = throwInvalidHookError;
 }
@@ -2444,6 +2453,7 @@ const HooksDispatcherOnMount: Dispatcher = {
   unstable_isNewReconciler: enableNewReconciler,
 };
 if (enableCache) {
+  (HooksDispatcherOnMount: Dispatcher).getCacheSignal = getCacheSignal;
   (HooksDispatcherOnMount: Dispatcher).getCacheForType = getCacheForType;
   (HooksDispatcherOnMount: Dispatcher).useCacheRefresh = mountRefresh;
 }
@@ -2471,6 +2481,7 @@ const HooksDispatcherOnUpdate: Dispatcher = {
   unstable_isNewReconciler: enableNewReconciler,
 };
 if (enableCache) {
+  (HooksDispatcherOnUpdate: Dispatcher).getCacheSignal = getCacheSignal;
   (HooksDispatcherOnUpdate: Dispatcher).getCacheForType = getCacheForType;
   (HooksDispatcherOnUpdate: Dispatcher).useCacheRefresh = updateRefresh;
 }
@@ -2498,6 +2509,7 @@ const HooksDispatcherOnRerender: Dispatcher = {
   unstable_isNewReconciler: enableNewReconciler,
 };
 if (enableCache) {
+  (HooksDispatcherOnRerender: Dispatcher).getCacheSignal = getCacheSignal;
   (HooksDispatcherOnRerender: Dispatcher).getCacheForType = getCacheForType;
   (HooksDispatcherOnRerender: Dispatcher).useCacheRefresh = updateRefresh;
 }
@@ -2668,6 +2680,7 @@ if (__DEV__) {
     unstable_isNewReconciler: enableNewReconciler,
   };
   if (enableCache) {
+    (HooksDispatcherOnMountInDEV: Dispatcher).getCacheSignal = getCacheSignal;
     (HooksDispatcherOnMountInDEV: Dispatcher).getCacheForType = getCacheForType;
     (HooksDispatcherOnMountInDEV: Dispatcher).useCacheRefresh = function useCacheRefresh() {
       currentHookNameInDev = 'useCacheRefresh';
@@ -2809,6 +2822,7 @@ if (__DEV__) {
     unstable_isNewReconciler: enableNewReconciler,
   };
   if (enableCache) {
+    (HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher).getCacheSignal = getCacheSignal;
     (HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher).getCacheForType = getCacheForType;
     (HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher).useCacheRefresh = function useCacheRefresh() {
       currentHookNameInDev = 'useCacheRefresh';
@@ -2950,6 +2964,7 @@ if (__DEV__) {
     unstable_isNewReconciler: enableNewReconciler,
   };
   if (enableCache) {
+    (HooksDispatcherOnUpdateInDEV: Dispatcher).getCacheSignal = getCacheSignal;
     (HooksDispatcherOnUpdateInDEV: Dispatcher).getCacheForType = getCacheForType;
     (HooksDispatcherOnUpdateInDEV: Dispatcher).useCacheRefresh = function useCacheRefresh() {
       currentHookNameInDev = 'useCacheRefresh';
@@ -3092,6 +3107,7 @@ if (__DEV__) {
     unstable_isNewReconciler: enableNewReconciler,
   };
   if (enableCache) {
+    (HooksDispatcherOnRerenderInDEV: Dispatcher).getCacheSignal = getCacheSignal;
     (HooksDispatcherOnRerenderInDEV: Dispatcher).getCacheForType = getCacheForType;
     (HooksDispatcherOnRerenderInDEV: Dispatcher).useCacheRefresh = function useCacheRefresh() {
       currentHookNameInDev = 'useCacheRefresh';
@@ -3250,6 +3266,7 @@ if (__DEV__) {
     unstable_isNewReconciler: enableNewReconciler,
   };
   if (enableCache) {
+    (InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher).getCacheSignal = getCacheSignal;
     (InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher).getCacheForType = getCacheForType;
     (InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher).useCacheRefresh = function useCacheRefresh() {
       currentHookNameInDev = 'useCacheRefresh';
@@ -3408,6 +3425,7 @@ if (__DEV__) {
     unstable_isNewReconciler: enableNewReconciler,
   };
   if (enableCache) {
+    (InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher).getCacheSignal = getCacheSignal;
     (InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher).getCacheForType = getCacheForType;
     (InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher).useCacheRefresh = function useCacheRefresh() {
       currentHookNameInDev = 'useCacheRefresh';
@@ -3567,6 +3585,7 @@ if (__DEV__) {
     unstable_isNewReconciler: enableNewReconciler,
   };
   if (enableCache) {
+    (InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher).getCacheSignal = getCacheSignal;
     (InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher).getCacheForType = getCacheForType;
     (InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher).useCacheRefresh = function useCacheRefresh() {
       currentHookNameInDev = 'useCacheRefresh';
diff --git a/packages/react-reconciler/src/ReactFiberLane.new.js b/packages/react-reconciler/src/ReactFiberLane.new.js
index ad124a432a4..c1f34e1052f 100644
--- a/packages/react-reconciler/src/ReactFiberLane.new.js
+++ b/packages/react-reconciler/src/ReactFiberLane.new.js
@@ -17,7 +17,6 @@ export type Lane = number;
 export type LaneMap<T> = Array<T>;
 
 import {
-  enableCache,
   enableSchedulingProfiler,
   enableUpdaterTracking,
   allowConcurrentByDefault,
@@ -635,15 +634,6 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) {
 
   root.entangledLanes &= remainingLanes;
 
-  if (enableCache) {
-    const pooledCacheLanes = (root.pooledCacheLanes &= remainingLanes);
-    if (pooledCacheLanes === NoLanes) {
-      // None of the remaining work relies on the cache pool. Clear it so
-      // subsequent requests get a new cache.
-      root.pooledCache = null;
-    }
-  }
-
   const entanglements = root.entanglements;
   const eventTimes = root.eventTimes;
   const expirationTimes = root.expirationTimes;
diff --git a/packages/react-reconciler/src/ReactFiberLane.old.js b/packages/react-reconciler/src/ReactFiberLane.old.js
index 4a064a38465..c81191f6a07 100644
--- a/packages/react-reconciler/src/ReactFiberLane.old.js
+++ b/packages/react-reconciler/src/ReactFiberLane.old.js
@@ -17,7 +17,6 @@ export type Lane = number;
 export type LaneMap<T> = Array<T>;
 
 import {
-  enableCache,
   enableSchedulingProfiler,
   enableUpdaterTracking,
   allowConcurrentByDefault,
@@ -635,15 +634,6 @@ export function markRootFinished(root: FiberRoot, remainingLanes: Lanes) {
 
   root.entangledLanes &= remainingLanes;
 
-  if (enableCache) {
-    const pooledCacheLanes = (root.pooledCacheLanes &= remainingLanes);
-    if (pooledCacheLanes === NoLanes) {
-      // None of the remaining work relies on the cache pool. Clear it so
-      // subsequent requests get a new cache.
-      root.pooledCache = null;
-    }
-  }
-
   const entanglements = root.entanglements;
   const eventTimes = root.eventTimes;
   const expirationTimes = root.expirationTimes;
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
index 830ae650066..0858fa1cc27 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -32,6 +32,7 @@ import {
   skipUnmountedBoundaries,
   enableUpdaterTracking,
   warnOnSubscriptionInsideStartTransition,
+  enableCache,
 } from 'shared/ReactFeatureFlags';
 import ReactSharedInternals from 'shared/ReactSharedInternals';
 import invariant from 'shared/invariant';
@@ -236,6 +237,7 @@ import {
   isDevToolsPresent,
 } from './ReactFiberDevToolsHook.new';
 import {onCommitRoot as onCommitRootTestSelector} from './ReactTestSelectors';
+import {releaseCache} from './ReactFiberCacheComponent.new';
 
 const ceil = Math.ceil;
 
@@ -2051,6 +2053,7 @@ function commitRootImpl(root, renderPriorityLevel) {
     hasUncaughtError = false;
     const error = firstUncaughtError;
     firstUncaughtError = null;
+    // TODO: how to handle cache cleanup here?
     throw error;
   }
 
@@ -2091,6 +2094,16 @@ function commitRootImpl(root, renderPriorityLevel) {
   // If layout work was scheduled, flush it now.
   flushSyncCallbacks();
 
+  // Now that effects have run - giving Cache boundaries a chance to retain the
+  // cache instance - release the root's cache in case since the render is complete
+  if (enableCache) {
+    const pooledCache = root.pooledCache;
+    if (pooledCache != null) {
+      releaseCache(pooledCache);
+      root.pooledCache = null;
+    }
+  }
+
   if (__DEV__) {
     if (enableDebugTracing) {
       logCommitStopped();
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
index 9cfea1563a1..9b1ccdd3dcb 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
@@ -32,6 +32,7 @@ import {
   skipUnmountedBoundaries,
   enableUpdaterTracking,
   warnOnSubscriptionInsideStartTransition,
+  enableCache,
 } from 'shared/ReactFeatureFlags';
 import ReactSharedInternals from 'shared/ReactSharedInternals';
 import invariant from 'shared/invariant';
@@ -236,6 +237,7 @@ import {
   isDevToolsPresent,
 } from './ReactFiberDevToolsHook.old';
 import {onCommitRoot as onCommitRootTestSelector} from './ReactTestSelectors';
+import {releaseCache} from './ReactFiberCacheComponent.old';
 
 const ceil = Math.ceil;
 
@@ -2051,6 +2053,7 @@ function commitRootImpl(root, renderPriorityLevel) {
     hasUncaughtError = false;
     const error = firstUncaughtError;
     firstUncaughtError = null;
+    // TODO: how to handle cache cleanup here?
     throw error;
   }
 
@@ -2091,6 +2094,16 @@ function commitRootImpl(root, renderPriorityLevel) {
   // If layout work was scheduled, flush it now.
   flushSyncCallbacks();
 
+  // Now that effects have run - giving Cache boundaries a chance to retain the
+  // cache instance - release the root's cache in case since the render is complete
+  if (enableCache) {
+    const pooledCache = root.pooledCache;
+    if (pooledCache != null) {
+      releaseCache(pooledCache);
+      root.pooledCache = null;
+    }
+  }
+
   if (__DEV__) {
     if (enableDebugTracing) {
       logCommitStopped();
diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js
index f079b3e8f2b..e971828233c 100644
--- a/packages/react-reconciler/src/ReactInternalTypes.js
+++ b/packages/react-reconciler/src/ReactInternalTypes.js
@@ -273,6 +273,7 @@ type BasicStateAction<S> = (S => S) | S;
 type Dispatch<A> = A => void;
 
 export type Dispatcher = {|
+  getCacheSignal?: () => AbortSignal,
   getCacheForType?: <T>(resourceType: () => T) => T,
   readContext<T>(context: ReactContext<T>): T,
   useState<S>(initialState: (() => S) | S): [S, Dispatch<BasicStateAction<S>>],
diff --git a/packages/react-reconciler/src/__tests__/ReactCache-test.js b/packages/react-reconciler/src/__tests__/ReactCache-test.js
index 31321eb07a5..76c9212d387 100644
--- a/packages/react-reconciler/src/__tests__/ReactCache-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactCache-test.js
@@ -1,6 +1,7 @@
 let React;
 let ReactNoop;
 let Cache;
+let getCacheSignal;
 let getCacheForType;
 let Scheduler;
 let act;
@@ -22,6 +23,7 @@ describe('ReactCache', () => {
     Scheduler = require('scheduler');
     act = require('jest-react').act;
     Suspense = React.Suspense;
+    getCacheSignal = React.unstable_getCacheSignal;
     getCacheForType = React.unstable_getCacheForType;
     useCacheRefresh = React.unstable_useCacheRefresh;
     startTransition = React.startTransition;
@@ -118,6 +120,11 @@ describe('ReactCache', () => {
       };
       textCache.data.set(text, newRecord);
 
+      const signal = getCacheSignal();
+      signal.addEventListener('abort', () => {
+        // console.log(`Cache cleanup: [${text}]`);
+      });
+
       throw thenable;
     }
   }
diff --git a/packages/react/index.js b/packages/react/index.js
index 1b4552a90f5..d7cab92fe7e 100644
--- a/packages/react/index.js
+++ b/packages/react/index.js
@@ -58,6 +58,7 @@ export {
   unstable_LegacyHidden,
   unstable_Offscreen,
   unstable_Scope,
+  unstable_getCacheSignal,
   unstable_getCacheForType,
   unstable_useCacheRefresh,
   unstable_useOpaqueIdentifier,
diff --git a/packages/react/src/React.js b/packages/react/src/React.js
index 541e7a35d3d..5bf9b4daadd 100644
--- a/packages/react/src/React.js
+++ b/packages/react/src/React.js
@@ -35,6 +35,7 @@ import {lazy} from './ReactLazy';
 import {forwardRef} from './ReactForwardRef';
 import {memo} from './ReactMemo';
 import {
+  getCacheSignal,
   getCacheForType,
   useCallback,
   useContext,
@@ -119,6 +120,7 @@ export {
   REACT_SUSPENSE_LIST_TYPE as SuspenseList,
   REACT_LEGACY_HIDDEN_TYPE as unstable_LegacyHidden,
   REACT_OFFSCREEN_TYPE as unstable_Offscreen,
+  getCacheSignal as unstable_getCacheSignal,
   getCacheForType as unstable_getCacheForType,
   useCacheRefresh as unstable_useCacheRefresh,
   REACT_CACHE_TYPE as unstable_Cache,
diff --git a/packages/react/src/ReactHooks.js b/packages/react/src/ReactHooks.js
index 0108c545fae..1892f926c59 100644
--- a/packages/react/src/ReactHooks.js
+++ b/packages/react/src/ReactHooks.js
@@ -41,6 +41,12 @@ function resolveDispatcher() {
   return ((dispatcher: any): Dispatcher);
 }
 
+export function getCacheSignal(): AbortSignal {
+  const dispatcher = resolveDispatcher();
+  // $FlowFixMe This is unstable, thus optional
+  return dispatcher.getCacheSignal();
+}
+
 export function getCacheForType<T>(resourceType: () => T): T {
   const dispatcher = resolveDispatcher();
   // $FlowFixMe This is unstable, thus optional

From 903145735ce62d36e121ac71dec115cd15694111 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Thu, 30 Sep 2021 14:07:02 -0700
Subject: [PATCH 05/25] Extract commit root cache cleanup into a function,
 handle error case

---
 .../src/ReactFiberWorkLoop.new.js             | 20 +++++++++++--------
 1 file changed, 12 insertions(+), 8 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
index 0858fa1cc27..b21481485d9 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -2053,7 +2053,7 @@ function commitRootImpl(root, renderPriorityLevel) {
     hasUncaughtError = false;
     const error = firstUncaughtError;
     firstUncaughtError = null;
-    // TODO: how to handle cache cleanup here?
+    releaseRootPooledCache(root);
     throw error;
   }
 
@@ -2096,13 +2096,7 @@ function commitRootImpl(root, renderPriorityLevel) {
 
   // Now that effects have run - giving Cache boundaries a chance to retain the
   // cache instance - release the root's cache in case since the render is complete
-  if (enableCache) {
-    const pooledCache = root.pooledCache;
-    if (pooledCache != null) {
-      releaseCache(pooledCache);
-      root.pooledCache = null;
-    }
-  }
+  releaseRootPooledCache(root);
 
   if (__DEV__) {
     if (enableDebugTracing) {
@@ -2117,6 +2111,16 @@ function commitRootImpl(root, renderPriorityLevel) {
   return null;
 }
 
+function releaseRootPooledCache(root) {
+  if (enableCache) {
+    const pooledCache = root.pooledCache;
+    if (pooledCache != null) {
+      releaseCache(pooledCache);
+      root.pooledCache = null;
+    }
+  }
+}
+
 export function flushPassiveEffects(): boolean {
   // Returns whether passive effects were flushed.
   // TODO: Combine this check with the one in flushPassiveEFfectsImpl. We should

From b8e67c3d6aa1d6250fe62fe5006b914d1e5b6826 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Fri, 1 Oct 2021 16:04:38 -0700
Subject: [PATCH 06/25] [wip] partial passive effects stage cleanup of cache
 instances

---
 .../src/ReactFiberBeginWork.new.js            |  5 +-
 .../src/ReactFiberCacheComponent.new.js       | 38 ++++++++++--
 .../src/ReactFiberCommitWork.new.js           | 62 +++++++++++++++++++
 .../src/ReactFiberCompleteWork.new.js         |  3 +
 .../src/ReactFiberWorkLoop.new.js             |  5 +-
 .../src/__tests__/ReactCache-test.js          | 35 ++++++++++-
 6 files changed, 140 insertions(+), 8 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js
index 6ad63ae763c..a02ec8ff383 100644
--- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js
@@ -23,9 +23,11 @@ import type {
   OffscreenProps,
   OffscreenState,
 } from './ReactFiberOffscreenComponent';
-import type {
+import {
   Cache,
   CacheComponentState,
+  releaseCache,
+  retainCache,
   SpawnedCachePool,
 } from './ReactFiberCacheComponent.new';
 import type {UpdateQueue} from './ReactUpdateQueue.new';
@@ -75,6 +77,7 @@ import {
   StaticMask,
   ShouldCapture,
   ForceClientRender,
+  Passive,
 } from './ReactFiberFlags';
 import ReactSharedInternals from 'shared/ReactSharedInternals';
 import {
diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
index e3738730321..10f2ab489d5 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
@@ -69,25 +69,55 @@ const prevFreshCacheOnStack: StackCursor<Cache | null> = createCursor(null);
 // * Call releaseCache() when any reference to the cache is "released" (ie
 //   when the reference is no longer reachable). This *includes* the original
 //   reference created w createCache().
+let _cacheIndex = 0;
 export function createCache(): Cache {
-  return {
+  const index = _cacheIndex++;
+  const stack = new Error().stack
+    .split('\n')
+    .slice(1)
+    .join('\n');
+  const cache: Cache = {
     controller: new AbortController(),
     data: new Map(),
     refCount: 1,
   };
+  (cache: any).stack = stack;
+  (cache: any).key = String(index);
+  // console.log(`createCache #${cache.key}:\n` + stack);
+  return cache;
 }
 
 export function retainCache(cache: Cache) {
+  console.log(
+    `retainCache #${cache.key} ${cache.refCount} -> ${cache.refCount + 1}:\n`,
+  );
+  // console.log(
+  //   `retainCache ${cache.refCount} -> ${cache.refCount + 1}:\n` +
+  //     new Error().stack
+  //       .split('\n')
+  //       .slice(1)
+  //       .join('\n'),
+  // );
   cache.refCount++;
 }
 
 export function releaseCache(cache: Cache) {
+  console.log(
+    `releaseCache #${cache.key} ${cache.refCount} -> ${cache.refCount - 1}:\n`,
+  );
+  // console.log(
+  //   `releaseCache ${cache.refCount} -> ${cache.refCount - 1}:\n` +
+  //     new Error().stack
+  //       .split('\n')
+  //       .slice(1)
+  //       .join('\n'),
+  // );
   cache.refCount--;
   if (__DEV__) {
     if (cache.refCount < 0) {
-      throw new Error(
-        'Error in React: cache reference count should not be negative',
-      );
+      // throw new Error(
+      //   'Error in React: cache reference count should not be negative',
+      // );
     }
   }
   if (cache.refCount === 0) {
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
index 699c6867fa1..603fa439ba3 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
@@ -24,6 +24,7 @@ import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.new';
 import type {Wakeable} from 'shared/ReactTypes';
 import type {OffscreenState} from './ReactFiberOffscreenComponent';
 import type {HookFlags} from './ReactHookEffectTags';
+import type {Cache} from './ReactFiberCacheComponent.new';
 
 import {
   enableCreateEventHandleAPI,
@@ -38,6 +39,7 @@ import {
   enableSuspenseLayoutEffectSemantics,
   enableUpdaterTracking,
   warnAboutCallbackRefReturningFunction,
+  enableCache,
 } from 'shared/ReactFeatureFlags';
 import {
   FunctionComponent,
@@ -57,6 +59,7 @@ import {
   ScopeComponent,
   OffscreenComponent,
   LegacyHiddenComponent,
+  CacheComponent,
 } from './ReactWorkTags';
 import {detachDeletedInstance} from './ReactFiberHostConfig';
 import {
@@ -143,6 +146,7 @@ import {
 import {didWarnAboutReassigningProps} from './ReactFiberBeginWork.new';
 import {doesFiberContain} from './ReactFiberTreeReflection';
 import {invokeGuardedCallback, clearCaughtError} from 'shared/ReactErrorUtils';
+import {releaseCache, retainCache} from './ReactFiberCacheComponent.new';
 
 let didWarnAboutUndefinedSnapshotBeforeUpdate: Set<mixed> | null = null;
 if (__DEV__) {
@@ -2594,6 +2598,8 @@ function commitPassiveMountEffects_complete(
 function commitPassiveMountOnFiber(
   finishedRoot: FiberRoot,
   finishedWork: Fiber,
+  // maybe thread through previous fiber or cache from previous fiber
+  //
 ): void {
   switch (finishedWork.tag) {
     case FunctionComponent:
@@ -2615,6 +2621,46 @@ function commitPassiveMountOnFiber(
       }
       break;
     }
+    case HostRoot: {
+      if (enableCache) {
+        // todo compare caches here
+        // if caches not same
+        // - retain next cache
+        // - if prev cache non-null: release (or move to unmount phase?)
+        // add comment that retain/release on child is technically not required
+        const previousCache: Cache =
+          finishedWork.alternate?.memoizedState.cache;
+        const nextCache: Cache = finishedWork.memoizedState.cache;
+        if (nextCache !== previousCache) {
+          console.log('retain/release HostRoot cache');
+          // retainCache(nextCache); root is already the owner, no need to retain
+          if (previousCache != null) {
+            releaseCache(previousCache);
+          }
+        }
+      }
+      break;
+    }
+    case CacheComponent: {
+      if (enableCache) {
+        // todo compare caches here
+        // if caches not same
+        // - retain next cache
+        // - if prev cache non-null: release (or move to unmount phase?)
+        // add comment that retain/release on child is technically not required
+        const previousCache: Cache =
+          finishedWork.alternate?.memoizedState.cache;
+        const nextCache: Cache = finishedWork.memoizedState.cache;
+        if (nextCache !== previousCache) {
+          console.log('retain/release CacheComponent cache');
+          retainCache(nextCache);
+          if (previousCache != null) {
+            releaseCache(previousCache);
+          }
+        }
+      }
+      break;
+    }
   }
 }
 
@@ -2821,6 +2867,22 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber(
       }
       break;
     }
+    case HostRoot: {
+      if (enableCache) {
+        const cache = current.memoizedState.cache;
+        console.log('release HostRoot cache');
+        releaseCache(cache);
+      }
+      break;
+    }
+    case CacheComponent: {
+      if (enableCache) {
+        const cache = current.memoizedState.cache;
+        console.log('release CacheComponent cache');
+        releaseCache(cache);
+      }
+      break;
+    }
   }
 }
 
diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js
index 03136ce30ba..99af01ff38e 100644
--- a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js
@@ -72,6 +72,7 @@ import {
   ChildDeletion,
   StaticMask,
   MutationMask,
+  Passive,
 } from './ReactFiberFlags';
 import invariant from 'shared/invariant';
 
@@ -847,6 +848,7 @@ function completeWork(
     case HostRoot: {
       const fiberRoot = (workInProgress.stateNode: FiberRoot);
       if (enableCache) {
+        workInProgress.flags |= Passive;
         popRootCachePool(fiberRoot, renderLanes);
 
         const cache: Cache = workInProgress.memoizedState.cache;
@@ -1472,6 +1474,7 @@ function completeWork(
     case CacheComponent: {
       if (enableCache) {
         const cache: Cache = workInProgress.memoizedState.cache;
+        workInProgress.flags |= Passive;
         popCacheProvider(workInProgress, cache);
         bubbleProperties(workInProgress);
         return null;
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
index b21481485d9..59411711a09 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -237,7 +237,7 @@ import {
   isDevToolsPresent,
 } from './ReactFiberDevToolsHook.new';
 import {onCommitRoot as onCommitRootTestSelector} from './ReactTestSelectors';
-import {releaseCache} from './ReactFiberCacheComponent.new';
+import {releaseCache, retainCache} from './ReactFiberCacheComponent.new';
 
 const ceil = Math.ceil;
 
@@ -2115,7 +2115,7 @@ function releaseRootPooledCache(root) {
   if (enableCache) {
     const pooledCache = root.pooledCache;
     if (pooledCache != null) {
-      releaseCache(pooledCache);
+      // releaseCache(pooledCache);
       root.pooledCache = null;
     }
   }
@@ -2160,6 +2160,7 @@ export function enqueuePendingPassiveProfilerEffect(fiber: Fiber): void {
 
 function flushPassiveEffectsImpl() {
   if (rootWithPendingPassiveEffects === null) {
+    console.log('flushPassiveEffectsImpl bailout');
     return false;
   }
 
diff --git a/packages/react-reconciler/src/__tests__/ReactCache-test.js b/packages/react-reconciler/src/__tests__/ReactCache-test.js
index 76c9212d387..02fb8e382c4 100644
--- a/packages/react-reconciler/src/__tests__/ReactCache-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactCache-test.js
@@ -161,10 +161,43 @@ describe('ReactCache', () => {
   // @gate experimental || www
   test('render Cache component', async () => {
     const root = ReactNoop.createRoot();
+    function Example(props) {
+      // React.useEffect(() => {
+      //   console.log(
+      //     'effect:\n' +
+      //       new Error().stack
+      //         .split('\n')
+      //         .slice(1)
+      //         .join('\n'),
+      //   );
+      //   return () => {
+      //     console.log(
+      //       'cleanup:\n' +
+      //         new Error().stack
+      //           .split('\n')
+      //           .slice(1)
+      //           .join('\n'),
+      //     );
+      //   };
+      // }, [props.text]);
+      return <Cache>{props.text}</Cache>;
+    }
+    console.log('render: hi');
     await act(async () => {
-      root.render(<Cache>Hi</Cache>);
+      // root.render(<Cache>Hi</Cache>);
+      root.render(<Example text="Hi" />);
     });
     expect(root).toMatchRenderedOutput('Hi');
+    console.log('render: ...');
+    await act(async () => {
+      root.render(<Example key={2} text="..." />);
+    });
+    expect(root).toMatchRenderedOutput('...');
+    console.log('render: bye');
+    await act(async () => {
+      root.render('Bye');
+    });
+    expect(root).toMatchRenderedOutput('Bye');
   });
 
   // @gate experimental || www

From bf8591323c056c9763d0d2393c954aa75b8e56e0 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Fri, 1 Oct 2021 16:13:30 -0700
Subject: [PATCH 07/25] small cleanup

---
 .../src/ReactFiberBeginWork.new.js            |  5 +--
 .../src/ReactFiberCacheComponent.new.js       | 37 +++++++------------
 2 files changed, 14 insertions(+), 28 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js
index a02ec8ff383..6ad63ae763c 100644
--- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js
@@ -23,11 +23,9 @@ import type {
   OffscreenProps,
   OffscreenState,
 } from './ReactFiberOffscreenComponent';
-import {
+import type {
   Cache,
   CacheComponentState,
-  releaseCache,
-  retainCache,
   SpawnedCachePool,
 } from './ReactFiberCacheComponent.new';
 import type {UpdateQueue} from './ReactUpdateQueue.new';
@@ -77,7 +75,6 @@ import {
   StaticMask,
   ShouldCapture,
   ForceClientRender,
-  Passive,
 } from './ReactFiberFlags';
 import ReactSharedInternals from 'shared/ReactSharedInternals';
 import {
diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
index 10f2ab489d5..908363afbbe 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
@@ -71,19 +71,22 @@ const prevFreshCacheOnStack: StackCursor<Cache | null> = createCursor(null);
 //   reference created w createCache().
 let _cacheIndex = 0;
 export function createCache(): Cache {
-  const index = _cacheIndex++;
-  const stack = new Error().stack
-    .split('\n')
-    .slice(1)
-    .join('\n');
   const cache: Cache = {
     controller: new AbortController(),
     data: new Map(),
     refCount: 1,
   };
+
+  // TODO: remove debugging code
+  const index = _cacheIndex++;
+  const stack = new Error().stack
+    .split('\n')
+    .slice(1)
+    .join('\n');
   (cache: any).stack = stack;
   (cache: any).key = String(index);
   // console.log(`createCache #${cache.key}:\n` + stack);
+
   return cache;
 }
 
@@ -91,13 +94,6 @@ export function retainCache(cache: Cache) {
   console.log(
     `retainCache #${cache.key} ${cache.refCount} -> ${cache.refCount + 1}:\n`,
   );
-  // console.log(
-  //   `retainCache ${cache.refCount} -> ${cache.refCount + 1}:\n` +
-  //     new Error().stack
-  //       .split('\n')
-  //       .slice(1)
-  //       .join('\n'),
-  // );
   cache.refCount++;
 }
 
@@ -105,24 +101,17 @@ export function releaseCache(cache: Cache) {
   console.log(
     `releaseCache #${cache.key} ${cache.refCount} -> ${cache.refCount - 1}:\n`,
   );
-  // console.log(
-  //   `releaseCache ${cache.refCount} -> ${cache.refCount - 1}:\n` +
-  //     new Error().stack
-  //       .split('\n')
-  //       .slice(1)
-  //       .join('\n'),
-  // );
   cache.refCount--;
   if (__DEV__) {
     if (cache.refCount < 0) {
-      // throw new Error(
-      //   'Error in React: cache reference count should not be negative',
-      // );
+      throw new Error(
+        'Error in React: cache reference count should not be negative',
+      );
     }
   }
   if (cache.refCount === 0) {
-    // TODO: considering scheduling and error handling for any
-    // event listeners that get triggered.
+    // TODO: considering scheduling this call, and adding error handling for
+    // any event listeners that get triggered.
     cache.controller.abort();
   }
 }

From 5521d7d64d243c0518e04c9170c80575c5d9254d Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Fri, 1 Oct 2021 16:40:21 -0700
Subject: [PATCH 08/25] new caches can be created from various sources (newly
 mouned cache component, refresh) so we do not know which referent (HostRoot,
 CacheComponent) created and "owns" the cache. Since any one of these could
 "own" the cache, retaining in passive effects could double-increment the ref
 count (double on top of the initial refCount=1). So instead of retaining in
 passive effects, we use a new cloneCache(cache) operation each time we create
 a new reference to the same cache. Then, passive effects change to only
 *releasing* cache references that are no longer reachable (due to an updated
 cache value at some fiber or deletinga fiber).

---
 .../src/ReactFiberBeginWork.new.js            |  3 +-
 .../src/ReactFiberCacheComponent.new.js       | 31 ++++++++++++++++---
 .../src/ReactFiberCommitWork.new.js           | 18 ++++-------
 .../src/ReactFiberWorkLoop.new.js             |  3 +-
 .../src/__tests__/ReactCache-test.js          |  4 ++-
 5 files changed, 40 insertions(+), 19 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js
index 6ad63ae763c..d70bddf476d 100644
--- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js
@@ -223,6 +223,7 @@ import {
 } from './ReactFiberWorkLoop.new';
 import {setWorkInProgressVersion} from './ReactMutableSource.new';
 import {
+  cloneCache,
   requestCacheFromPool,
   pushCacheProvider,
   pushRootCachePool,
@@ -849,7 +850,7 @@ function updateCacheComponent(
       // Refresh in parent. Update the parent.
       const derivedState: CacheComponentState = {
         parent: parentCache,
-        cache: parentCache,
+        cache: cloneCache(parentCache),
       };
 
       // Copied from getDerivedStateFromProps implementation. Once the update
diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
index 908363afbbe..cb0eb0c2566 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
@@ -85,8 +85,26 @@ export function createCache(): Cache {
     .join('\n');
   (cache: any).stack = stack;
   (cache: any).key = String(index);
-  // console.log(`createCache #${cache.key}:\n` + stack);
+  console.log(
+    `createCache #${cache.key}:\n` +
+      stack
+        .split('\n')
+        .slice(2, 4)
+        .join('\n'),
+  );
+
+  return cache;
+}
 
+export function cloneCache(cache: Cache): Cache {
+  console.log(
+    `cloneCache #${cache.key} ${cache.refCount} -> ${cache.refCount + 1}:\n` +
+      new Error().stack
+        .split('\n')
+        .slice(2, 4)
+        .join('\n'),
+  );
+  cache.refCount++;
   return cache;
 }
 
@@ -135,11 +153,11 @@ export function requestCacheFromPool(renderLanes: Lanes): Cache {
     return (null: any);
   }
   if (pooledCache !== null) {
-    return pooledCache;
+    return cloneCache(pooledCache);
   }
   // Create a fresh cache.
   pooledCache = createCache();
-  return pooledCache;
+  return cloneCache(pooledCache);
 }
 
 export function pushRootCachePool(root: FiberRoot) {
@@ -150,7 +168,12 @@ export function pushRootCachePool(root: FiberRoot) {
   // from `root.pooledCache`. If it's currently `null`, we will lazily
   // initialize it the first type it's requested. However, we only mutate
   // the root itself during the complete/unwind phase of the HostRoot.
-  pooledCache = root.pooledCache;
+  const rootCache = root.pooledCache;
+  if (rootCache != null) {
+    pooledCache = cloneCache(rootCache);
+  } else {
+    pooledCache = null;
+  }
 }
 
 export function popRootCachePool(root: FiberRoot, renderLanes: Lanes) {
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
index 603fa439ba3..af6bca8ac10 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
@@ -2631,12 +2631,9 @@ function commitPassiveMountOnFiber(
         const previousCache: Cache =
           finishedWork.alternate?.memoizedState.cache;
         const nextCache: Cache = finishedWork.memoizedState.cache;
-        if (nextCache !== previousCache) {
-          console.log('retain/release HostRoot cache');
-          // retainCache(nextCache); root is already the owner, no need to retain
-          if (previousCache != null) {
-            releaseCache(previousCache);
-          }
+        if (nextCache !== previousCache && previousCache != null) {
+          console.log('update HostRoot cache');
+          releaseCache(previousCache);
         }
       }
       break;
@@ -2651,12 +2648,9 @@ function commitPassiveMountOnFiber(
         const previousCache: Cache =
           finishedWork.alternate?.memoizedState.cache;
         const nextCache: Cache = finishedWork.memoizedState.cache;
-        if (nextCache !== previousCache) {
-          console.log('retain/release CacheComponent cache');
-          retainCache(nextCache);
-          if (previousCache != null) {
-            releaseCache(previousCache);
-          }
+        if (nextCache !== previousCache && previousCache != null) {
+          console.log('update CacheComponent cache');
+          releaseCache(previousCache);
         }
       }
       break;
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
index 59411711a09..024be251663 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -2115,7 +2115,8 @@ function releaseRootPooledCache(root) {
   if (enableCache) {
     const pooledCache = root.pooledCache;
     if (pooledCache != null) {
-      // releaseCache(pooledCache);
+      console.log('releaseRootPooledCache');
+      releaseCache(pooledCache);
       root.pooledCache = null;
     }
   }
diff --git a/packages/react-reconciler/src/__tests__/ReactCache-test.js b/packages/react-reconciler/src/__tests__/ReactCache-test.js
index 02fb8e382c4..7106c6600ad 100644
--- a/packages/react-reconciler/src/__tests__/ReactCache-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactCache-test.js
@@ -1,3 +1,5 @@
+const {test} = require('jest-snapshot-serializer-raw');
+
 let React;
 let ReactNoop;
 let Cache;
@@ -159,7 +161,7 @@ describe('ReactCache', () => {
   }
 
   // @gate experimental || www
-  test('render Cache component', async () => {
+  test.only('render Cache component', async () => {
     const root = ReactNoop.createRoot();
     function Example(props) {
       // React.useEffect(() => {

From f506e7796bb601ba6d6519596531cb99e072efdc Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Fri, 1 Oct 2021 17:20:00 -0700
Subject: [PATCH 09/25] code comments; first test of cleanup

---
 .../src/ReactFiberCacheComponent.new.js       | 66 ++++++++-----------
 .../src/ReactFiberCommitWork.new.js           |  2 +-
 .../src/__tests__/ReactCache-test.js          | 19 +++++-
 3 files changed, 46 insertions(+), 41 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
index cb0eb0c2566..53fd329a630 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
@@ -62,13 +62,15 @@ let pooledCache: Cache | null = null;
 const prevFreshCacheOnStack: StackCursor<Cache | null> = createCursor(null);
 
 // Creates a new empty Cache instance with a ref-count of 1: the caller is
-// the implicit "owner". Note that retain/release must be called to ensure
-// that the ref count is accurate:
-// * Call retainCache() when a new reference to the cache instance is created.
-//   This *excludes* the original reference created w createCache().
-// * Call releaseCache() when any reference to the cache is "released" (ie
-//   when the reference is no longer reachable). This *includes* the original
-//   reference created w createCache().
+// the implicit "owner" of the cache instance. The developer is responsible
+// for calling `releaseCache(cache)` when the reference to this cache is no
+// longer reachable.
+//
+// To create additional references to a Cache (ie to share it across the root,
+// multiple <Cache> components, etc), create a new "clone" with
+// eg `clone = cloneCache(cache)`. The developer is then calling
+// `releaseCache(clone)` for each clone created w cloneCache, when that clone
+// is no longer reachable.
 let _cacheIndex = 0;
 export function createCache(): Cache {
   const cache: Cache = {
@@ -78,46 +80,26 @@ export function createCache(): Cache {
   };
 
   // TODO: remove debugging code
-  const index = _cacheIndex++;
-  const stack = new Error().stack
-    .split('\n')
-    .slice(1)
-    .join('\n');
-  (cache: any).stack = stack;
-  (cache: any).key = String(index);
-  console.log(
-    `createCache #${cache.key}:\n` +
-      stack
-        .split('\n')
-        .slice(2, 4)
-        .join('\n'),
-  );
+  const key = _cacheIndex++;
+  (cache: any).key = String(key);
+  trace(`createCache #${cache.key}`);
 
   return cache;
 }
 
+// Creates a lightweight clone of the given Cache instance that *shares the same data*.
+// When the returned cache instance is no longer reference it must be cleaned up by
+// calling `releaseCache(clone)`
 export function cloneCache(cache: Cache): Cache {
-  console.log(
-    `cloneCache #${cache.key} ${cache.refCount} -> ${cache.refCount + 1}:\n` +
-      new Error().stack
-        .split('\n')
-        .slice(2, 4)
-        .join('\n'),
-  );
+  trace(`cloneCache #${cache.key} ${cache.refCount} -> ${cache.refCount + 1}`);
   cache.refCount++;
   return cache;
 }
 
-export function retainCache(cache: Cache) {
-  console.log(
-    `retainCache #${cache.key} ${cache.refCount} -> ${cache.refCount + 1}:\n`,
-  );
-  cache.refCount++;
-}
-
+// Cleanup a cache instance, potentially freeing it if there are no more references
 export function releaseCache(cache: Cache) {
-  console.log(
-    `releaseCache #${cache.key} ${cache.refCount} -> ${cache.refCount - 1}:\n`,
+  trace(
+    `releaseCache #${cache.key} ${cache.refCount} -> ${cache.refCount - 1}`,
   );
   cache.refCount--;
   if (__DEV__) {
@@ -134,6 +116,16 @@ export function releaseCache(cache: Cache) {
   }
 }
 
+function trace(msg, levels = 2) {
+  console.log(
+    `${msg}:\n` +
+      new Error().stack
+        .split('\n')
+        .slice(3, 3 + Math.min(levels, 1))
+        .join('\n'),
+  );
+}
+
 export function pushCacheProvider(workInProgress: Fiber, cache: Cache) {
   if (!enableCache) {
     return;
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
index af6bca8ac10..b3b39acddbf 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
@@ -146,7 +146,7 @@ import {
 import {didWarnAboutReassigningProps} from './ReactFiberBeginWork.new';
 import {doesFiberContain} from './ReactFiberTreeReflection';
 import {invokeGuardedCallback, clearCaughtError} from 'shared/ReactErrorUtils';
-import {releaseCache, retainCache} from './ReactFiberCacheComponent.new';
+import {releaseCache} from './ReactFiberCacheComponent.new';
 
 let didWarnAboutUndefinedSnapshotBeforeUpdate: Set<mixed> | null = null;
 if (__DEV__) {
diff --git a/packages/react-reconciler/src/__tests__/ReactCache-test.js b/packages/react-reconciler/src/__tests__/ReactCache-test.js
index 7106c6600ad..58565239bee 100644
--- a/packages/react-reconciler/src/__tests__/ReactCache-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactCache-test.js
@@ -124,7 +124,9 @@ describe('ReactCache', () => {
 
       const signal = getCacheSignal();
       signal.addEventListener('abort', () => {
-        // console.log(`Cache cleanup: [${text}]`);
+        Scheduler.unstable_yieldValue(
+          `Cache cleanup: ${text} [v${textCache.version}]`,
+        );
       });
 
       throw thenable;
@@ -161,7 +163,7 @@ describe('ReactCache', () => {
   }
 
   // @gate experimental || www
-  test.only('render Cache component', async () => {
+  test('render Cache component', async () => {
     const root = ReactNoop.createRoot();
     function Example(props) {
       // React.useEffect(() => {
@@ -222,6 +224,11 @@ describe('ReactCache', () => {
     });
     expect(Scheduler).toHaveYielded(['A']);
     expect(root).toMatchRenderedOutput('A');
+
+    await act(async () => {
+      root.render('Bye');
+    });
+    expect(root).toMatchRenderedOutput('Bye');
   });
 
   // @gate experimental || www
@@ -354,7 +361,7 @@ describe('ReactCache', () => {
   });
 
   // @gate experimental || www
-  test('a new Cache boundary uses fresh cache', async () => {
+  test.only('a new Cache boundary uses fresh cache', async () => {
     // The only difference from the previous test is that the "Show More"
     // content is wrapped in a nested <Cache /> boundary
     function App({showMore}) {
@@ -398,6 +405,12 @@ describe('ReactCache', () => {
     });
     expect(Scheduler).toHaveYielded(['A [v2]']);
     expect(root).toMatchRenderedOutput('A [v1]A [v2]');
+
+    await act(async => {
+      root.render('Bye!');
+    });
+    expect(Scheduler).toHaveYielded(['Cache cleanup: A [v2]']);
+    expect(root).toMatchRenderedOutput('Bye!');
   });
 
   // @gate experimental || www

From 2af984a9c2abaad4f7a6e42b48c1abacbc34244a Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Mon, 4 Oct 2021 15:44:42 -0700
Subject: [PATCH 10/25] wip many tests passing

---
 .../src/ReactFiberCacheComponent.new.js       |   7 +-
 .../src/ReactFiberRoot.new.js                 |   4 +-
 .../src/ReactFiberUnwindWork.new.js           |   7 +-
 .../src/ReactFiberWorkLoop.new.js             |   3 +-
 .../src/__tests__/ReactCache-test.js          | 413 ++++++++++++++++--
 5 files changed, 384 insertions(+), 50 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
index 53fd329a630..0813b779696 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
@@ -121,7 +121,7 @@ function trace(msg, levels = 2) {
     `${msg}:\n` +
       new Error().stack
         .split('\n')
-        .slice(3, 3 + Math.min(levels, 1))
+        .slice(3, 3 + Math.max(levels, 1))
         .join('\n'),
   );
 }
@@ -162,7 +162,9 @@ export function pushRootCachePool(root: FiberRoot) {
   // the root itself during the complete/unwind phase of the HostRoot.
   const rootCache = root.pooledCache;
   if (rootCache != null) {
-    pooledCache = cloneCache(rootCache);
+    trace('pushRootCachePool');
+    pooledCache = rootCache;
+    root.pooledCache = null;
   } else {
     pooledCache = null;
   }
@@ -179,6 +181,7 @@ export function popRootCachePool(root: FiberRoot, renderLanes: Lanes) {
   // on it (which we track with `pooledCacheLanes`) have committed.
   root.pooledCache = pooledCache;
   if (pooledCache !== null) {
+    trace('popRootCachePool');
     root.pooledCacheLanes |= renderLanes;
   }
   // set to null, conceptually we are moving ownership to the root
diff --git a/packages/react-reconciler/src/ReactFiberRoot.new.js b/packages/react-reconciler/src/ReactFiberRoot.new.js
index 75a621b610f..1b49e45289c 100644
--- a/packages/react-reconciler/src/ReactFiberRoot.new.js
+++ b/packages/react-reconciler/src/ReactFiberRoot.new.js
@@ -28,7 +28,7 @@ import {
 } from 'shared/ReactFeatureFlags';
 import {initializeUpdateQueue} from './ReactUpdateQueue.new';
 import {LegacyRoot, ConcurrentRoot} from './ReactRootTags';
-import {createCache} from './ReactFiberCacheComponent.new';
+import {cloneCache, createCache} from './ReactFiberCacheComponent.new';
 
 function FiberRootNode(containerInfo, tag, hydrate) {
   this.tag = tag;
@@ -119,7 +119,7 @@ export function createFiberRoot(
 
   if (enableCache) {
     const initialCache = createCache();
-    root.pooledCache = initialCache;
+    root.pooledCache = cloneCache(initialCache);
     const initialState = {
       element: null,
       cache: initialCache,
diff --git a/packages/react-reconciler/src/ReactFiberUnwindWork.new.js b/packages/react-reconciler/src/ReactFiberUnwindWork.new.js
index 152837286f5..1c096f3e207 100644
--- a/packages/react-reconciler/src/ReactFiberUnwindWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberUnwindWork.new.js
@@ -11,7 +11,11 @@ import type {ReactContext} from 'shared/ReactTypes';
 import type {Fiber, FiberRoot} from './ReactInternalTypes';
 import type {Lanes} from './ReactFiberLane.new';
 import type {SuspenseState} from './ReactFiberSuspenseComponent.new';
-import type {Cache, SpawnedCachePool} from './ReactFiberCacheComponent.new';
+import {
+  Cache,
+  releaseCache,
+  SpawnedCachePool,
+} from './ReactFiberCacheComponent.new';
 
 import {resetWorkInProgressVersions as resetMutableSourceWorkInProgressVersions} from './ReactMutableSource.new';
 import {
@@ -153,6 +157,7 @@ function unwindWork(workInProgress: Fiber, renderLanes: Lanes) {
       if (enableCache) {
         const cache: Cache = workInProgress.memoizedState.cache;
         popCacheProvider(workInProgress, cache);
+        releaseCache(cache);
       }
       return null;
     default:
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
index 024be251663..5960a68b03b 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -237,7 +237,7 @@ import {
   isDevToolsPresent,
 } from './ReactFiberDevToolsHook.new';
 import {onCommitRoot as onCommitRootTestSelector} from './ReactTestSelectors';
-import {releaseCache, retainCache} from './ReactFiberCacheComponent.new';
+import {releaseCache} from './ReactFiberCacheComponent.new';
 
 const ceil = Math.ceil;
 
@@ -2115,7 +2115,6 @@ function releaseRootPooledCache(root) {
   if (enableCache) {
     const pooledCache = root.pooledCache;
     if (pooledCache != null) {
-      console.log('releaseRootPooledCache');
       releaseCache(pooledCache);
       root.pooledCache = null;
     }
diff --git a/packages/react-reconciler/src/__tests__/ReactCache-test.js b/packages/react-reconciler/src/__tests__/ReactCache-test.js
index 58565239bee..66c57e55853 100644
--- a/packages/react-reconciler/src/__tests__/ReactCache-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactCache-test.js
@@ -80,9 +80,15 @@ describe('ReactCache', () => {
   }
 
   function readText(text) {
+    const signal = getCacheSignal();
     const textCache = getCacheForType(createTextCache);
     const record = textCache.data.get(text);
     if (record !== undefined) {
+      signal.addEventListener('abort', () => {
+        Scheduler.unstable_yieldValue(
+          `Cache cleanup: ${text} [v${textCache.version}] (cached)`,
+        );
+      });
       switch (record.status) {
         case 'pending':
           throw record.value;
@@ -122,13 +128,11 @@ describe('ReactCache', () => {
       };
       textCache.data.set(text, newRecord);
 
-      const signal = getCacheSignal();
       signal.addEventListener('abort', () => {
         Scheduler.unstable_yieldValue(
           `Cache cleanup: ${text} [v${textCache.version}]`,
         );
       });
-
       throw thenable;
     }
   }
@@ -162,48 +166,21 @@ describe('ReactCache', () => {
     }
   }
 
+  // TODO OK
   // @gate experimental || www
   test('render Cache component', async () => {
     const root = ReactNoop.createRoot();
     function Example(props) {
-      // React.useEffect(() => {
-      //   console.log(
-      //     'effect:\n' +
-      //       new Error().stack
-      //         .split('\n')
-      //         .slice(1)
-      //         .join('\n'),
-      //   );
-      //   return () => {
-      //     console.log(
-      //       'cleanup:\n' +
-      //         new Error().stack
-      //           .split('\n')
-      //           .slice(1)
-      //           .join('\n'),
-      //     );
-      //   };
-      // }, [props.text]);
       return <Cache>{props.text}</Cache>;
     }
     console.log('render: hi');
     await act(async () => {
-      // root.render(<Cache>Hi</Cache>);
       root.render(<Example text="Hi" />);
     });
     expect(root).toMatchRenderedOutput('Hi');
-    console.log('render: ...');
-    await act(async () => {
-      root.render(<Example key={2} text="..." />);
-    });
-    expect(root).toMatchRenderedOutput('...');
-    console.log('render: bye');
-    await act(async () => {
-      root.render('Bye');
-    });
-    expect(root).toMatchRenderedOutput('Bye');
   });
 
+  // TODO OK
   // @gate experimental || www
   test('mount new data', async () => {
     const root = ReactNoop.createRoot();
@@ -228,9 +205,12 @@ describe('ReactCache', () => {
     await act(async () => {
       root.render('Bye');
     });
+    // no cleanup: cache is still retained at the root
+    expect(Scheduler).toHaveYielded([]);
     expect(root).toMatchRenderedOutput('Bye');
   });
 
+  // TODO OK
   // @gate experimental || www
   test('root acts as implicit cache boundary', async () => {
     const root = ReactNoop.createRoot();
@@ -249,12 +229,20 @@ describe('ReactCache', () => {
     });
     expect(Scheduler).toHaveYielded(['A']);
     expect(root).toMatchRenderedOutput('A');
+
+    await act(async () => {
+      root.render('Bye');
+    });
+    // no cleanup: cache is still retained at the root
+    expect(Scheduler).toHaveYielded([]);
+    expect(root).toMatchRenderedOutput('Bye');
   });
 
+  // TODO OK
   // @gate experimental || www
   test('multiple new Cache boundaries in the same update share the same, fresh cache', async () => {
-    function App({text}) {
-      return (
+    function App({showMore}) {
+      return showMore ? (
         <>
           <Cache>
             <Suspense fallback={<Text text="Loading..." />}>
@@ -267,6 +255,8 @@ describe('ReactCache', () => {
             </Suspense>
           </Cache>
         </>
+      ) : (
+        '(empty)'
       );
     }
 
@@ -274,6 +264,12 @@ describe('ReactCache', () => {
     await act(async () => {
       root.render(<App showMore={false} />);
     });
+    expect(Scheduler).toHaveYielded([]);
+    expect(root).toMatchRenderedOutput('(empty)');
+
+    await act(async => {
+      root.render(<App showMore={true} />);
+    });
     // Even though there are two new <Cache /> trees, they should share the same
     // data cache. So there should be only a single cache miss for A.
     expect(Scheduler).toHaveYielded([
@@ -288,8 +284,23 @@ describe('ReactCache', () => {
     });
     expect(Scheduler).toHaveYielded(['A', 'A']);
     expect(root).toMatchRenderedOutput('AA');
+
+    await act(async () => {
+      root.render('Bye');
+    });
+    // cleanup occurs for the cache shared by the inner cache boundaries (which
+    // are not shared w the root because they were added in an update)
+    // note that no cache is created for the root since the cache is never accessed
+    expect(Scheduler).toHaveYielded([
+      'Cache cleanup: A [v1]', // first AsyncText gets a cache miss
+      'Cache cleanup: A [v1] (cached)', // second AsyncText shares the same entry
+      'Cache cleanup: A [v1] (cached)', // 2nd render of first AsyncText
+      'Cache cleanup: A [v1] (cached)', // 2nd render of second AsyncText
+    ]);
+    expect(root).toMatchRenderedOutput('Bye');
   });
 
+  // TODO OK
   // @gate experimental || www
   test(
     'nested cache boundaries share the same cache as the root during ' +
@@ -310,8 +321,8 @@ describe('ReactCache', () => {
       await act(async () => {
         root.render(<App />);
       });
-      // Even though there are two new <Cache /> trees, they should share the same
-      // data cache. So there should be only a single cache miss for A.
+      // Even though there is a nested <Cache /> boundary, it should share the same
+      // data cache as the root. So there should be only a single cache miss for A.
       expect(Scheduler).toHaveYielded(['Cache miss! [A]', 'Loading...']);
       expect(root).toMatchRenderedOutput('Loading...');
 
@@ -320,9 +331,17 @@ describe('ReactCache', () => {
       });
       expect(Scheduler).toHaveYielded(['A', 'A']);
       expect(root).toMatchRenderedOutput('AA');
+
+      await act(async () => {
+        root.render('Bye');
+      });
+      // no cleanup: cache is still retained at the root
+      expect(Scheduler).toHaveYielded([]);
+      expect(root).toMatchRenderedOutput('Bye');
     },
   );
 
+  // TODO OK
   // @gate experimental || www
   test('new content inside an existing Cache boundary should re-use already cached data', async () => {
     function App({showMore}) {
@@ -358,10 +377,18 @@ describe('ReactCache', () => {
       'A [v1]',
     ]);
     expect(root).toMatchRenderedOutput('A [v1]A [v1]');
+
+    await act(async () => {
+      root.render('Bye');
+    });
+    // no cleanup: cache is still retained at the root
+    expect(Scheduler).toHaveYielded([]);
+    expect(root).toMatchRenderedOutput('Bye');
   });
 
+  // TODO OK
   // @gate experimental || www
-  test.only('a new Cache boundary uses fresh cache', async () => {
+  test('a new Cache boundary uses fresh cache', async () => {
     // The only difference from the previous test is that the "Show More"
     // content is wrapped in a nested <Cache /> boundary
     function App({showMore}) {
@@ -406,15 +433,24 @@ describe('ReactCache', () => {
     expect(Scheduler).toHaveYielded(['A [v2]']);
     expect(root).toMatchRenderedOutput('A [v1]A [v2]');
 
+    // Replace all the children: this should retain the root Cache instance,
+    // but cleanup the separate cache instance created for the fresh cache
+    // boundary
     await act(async => {
       root.render('Bye!');
     });
-    expect(Scheduler).toHaveYielded(['Cache cleanup: A [v2]']);
+    // Cleanup occurs for the *second* cache instance: the first is still
+    // referenced by the root
+    expect(Scheduler).toHaveYielded([
+      'Cache cleanup: A [v2]', // initial render (suspends)
+      'Cache cleanup: A [v2] (cached)', // second render (after data resolves)
+    ]);
     expect(root).toMatchRenderedOutput('Bye!');
   });
 
+  // TODO OK
   // @gate experimental || www
-  test('inner content uses same cache as shell if spawned by the same transition', async () => {
+  test('inner/outer cache boundaries uses the same cache instance on initial render', async () => {
     const root = ReactNoop.createRoot();
 
     function App() {
@@ -486,10 +522,122 @@ describe('ReactCache', () => {
         <div>Content</div>
       </>,
     );
+
+    await act(async () => {
+      root.render('Bye');
+    });
+    // no cleanup: cache is still retained at the root
+    expect(Scheduler).toHaveYielded([]);
+    expect(root).toMatchRenderedOutput('Bye');
   });
 
+  // TODO OK
   // @gate experimental || www
-  test('refresh a cache', async () => {
+  test('inner/ outer cache boundaries added in the same update use the same cache instance', async () => {
+    const root = ReactNoop.createRoot();
+
+    function App({showMore}) {
+      return showMore ? (
+        <Cache>
+          <Suspense fallback={<Text text="Loading shell..." />}>
+            {/* The shell reads A */}
+            <Shell>
+              {/* The inner content reads both A and B */}
+              <Suspense fallback={<Text text="Loading content..." />}>
+                <Cache>
+                  <Content />
+                </Cache>
+              </Suspense>
+            </Shell>
+          </Suspense>
+        </Cache>
+      ) : (
+        '(empty)'
+      );
+    }
+
+    function Shell({children}) {
+      readText('A');
+      return (
+        <>
+          <div>
+            <Text text="Shell" />
+          </div>
+          <div>{children}</div>
+        </>
+      );
+    }
+
+    function Content() {
+      readText('A');
+      readText('B');
+      return <Text text="Content" />;
+    }
+
+    await act(async () => {
+      root.render(<App showMore={false} />);
+    });
+    expect(Scheduler).toHaveYielded([]);
+    expect(root).toMatchRenderedOutput('(empty)');
+
+    console.log('*** show cache boundaries ***');
+    await act(async () => {
+      root.render(<App showMore={true} />);
+    });
+    expect(Scheduler).toHaveYielded(['Cache miss! [A]', 'Loading shell...']);
+    expect(root).toMatchRenderedOutput('Loading shell...');
+
+    console.log('*** resolve A ***');
+    await act(async () => {
+      resolveMostRecentTextCache('A');
+    });
+    expect(Scheduler).toHaveYielded([
+      'Shell',
+      // There's a cache miss for B, because it hasn't been read yet. But not
+      // A, because it was cached when we rendered the shell.
+      'Cache miss! [B]',
+      'Loading content...',
+    ]);
+    expect(root).toMatchRenderedOutput(
+      <>
+        <div>Shell</div>
+        <div>Loading content...</div>
+      </>,
+    );
+
+    console.log('*** resolve B ***');
+    await act(async () => {
+      resolveMostRecentTextCache('B');
+    });
+    expect(Scheduler).toHaveYielded(['Content']);
+    expect(root).toMatchRenderedOutput(
+      <>
+        <div>Shell</div>
+        <div>Content</div>
+      </>,
+    );
+
+    console.log('*** unmount children ***');
+    await act(async () => {
+      root.render('Bye');
+    });
+    expect(Scheduler).toHaveYielded([
+      // first render stops on cache miss of A
+      'Cache cleanup: A [v1]',
+      // resume: shell reads A, Content reads A and gets a cache miss on B
+      'Cache cleanup: A [v1] (cached)',
+      'Cache cleanup: A [v1] (cached)',
+      'Cache cleanup: B [v1]',
+      // resume again: cache hits for A and B
+      'Cache cleanup: A [v1] (cached)',
+      'Cache cleanup: B [v1] (cached)',
+    ]);
+    expect(root).toMatchRenderedOutput('Bye');
+  });
+
+  // TODO OK
+  // @gate experimental || www
+  test('refresh a cache boundary', async () => {
     let refresh;
     function App() {
       refresh = useCacheRefresh();
@@ -529,8 +677,20 @@ describe('ReactCache', () => {
     // Note that the version has updated
     expect(Scheduler).toHaveYielded(['A [v2]']);
     expect(root).toMatchRenderedOutput('A [v2]');
+
+    await act(async () => {
+      root.render('Bye');
+    });
+    // the original cache instance does not cleanup since it is still referenced
+    // by the root, but the refreshed inner cache does cleanup
+    expect(Scheduler).toHaveYielded([
+      'Cache cleanup: A [v2]',
+      'Cache cleanup: A [v2] (cached)',
+    ]);
+    expect(root).toMatchRenderedOutput('Bye');
   });
 
+  // TODO OK
   // @gate experimental || www
   test('refresh the root cache', async () => {
     let refresh;
@@ -567,11 +727,79 @@ describe('ReactCache', () => {
     await act(async () => {
       resolveMostRecentTextCache('A');
     });
-    // Note that the version has updated
+    // Note that the version has updated, and the previous cache is cleared
+    expect(Scheduler).toHaveYielded([
+      'A [v2]',
+      'Cache cleanup: A [v1]', // initial suspended render
+      'Cache cleanup: A [v1] (cached)', // resumption
+    ]);
+    expect(root).toMatchRenderedOutput('A [v2]');
+
+    await act(async () => {
+      root.render('Bye');
+    });
+    // the original root cache already cleaned up when the refresh completed
+    expect(Scheduler).toHaveYielded([]);
+    expect(root).toMatchRenderedOutput('Bye');
+  });
+
+  // TODO OK
+  // @gate experimental || www
+  test('refresh the root cache without a transition', async () => {
+    let refresh;
+    function App() {
+      refresh = useCacheRefresh();
+      return <AsyncText showVersion={true} text="A" />;
+    }
+
+    // Mount initial data
+    const root = ReactNoop.createRoot();
+    await act(async () => {
+      root.render(
+        <Suspense fallback={<Text text="Loading..." />}>
+          <App />
+        </Suspense>,
+      );
+    });
+    expect(Scheduler).toHaveYielded(['Cache miss! [A]', 'Loading...']);
+    expect(root).toMatchRenderedOutput('Loading...');
+
+    await act(async () => {
+      resolveMostRecentTextCache('A');
+    });
+    expect(Scheduler).toHaveYielded(['A [v1]']);
+    expect(root).toMatchRenderedOutput('A [v1]');
+
+    // Refresh for new data.
+    await act(async () => {
+      refresh();
+    });
+    expect(Scheduler).toHaveYielded([
+      'Cache miss! [A]',
+      'Loading...',
+
+      // cleanup occurs immediately bc the root commits w a new cache
+      'Cache cleanup: A [v1]',
+      'Cache cleanup: A [v1] (cached)',
+    ]);
+    expect(root).toMatchRenderedOutput('Loading...');
+
+    await act(async () => {
+      resolveMostRecentTextCache('A');
+    });
+    // Note that the version has updated, and the previous cache is cleared
     expect(Scheduler).toHaveYielded(['A [v2]']);
     expect(root).toMatchRenderedOutput('A [v2]');
+
+    await act(async () => {
+      root.render('Bye');
+    });
+    // the original root cache already cleaned up when the refresh completed
+    expect(Scheduler).toHaveYielded([]);
+    expect(root).toMatchRenderedOutput('Bye');
   });
 
+  // TODO OK
   // @gate experimental || www
   test('refresh a cache with seed data', async () => {
     let refresh;
@@ -611,12 +839,24 @@ describe('ReactCache', () => {
       startTransition(() => refresh(createTextCache, cache));
     });
     // The root should re-render without a cache miss.
+    // The cache is not cleared up yet, since it's still reference by the root
     expect(Scheduler).toHaveYielded(['A [v2]']);
     expect(root).toMatchRenderedOutput('A [v2]');
+
+    await act(async () => {
+      root.render('Bye');
+    });
+    // the refreshed cache boundary is unmounted and cleans up
+    expect(Scheduler).toHaveYielded([
+      // note there is no non '(cached)' version, since the refresh cache was seeded
+      'Cache cleanup: A [v2] (cached)',
+    ]);
+    expect(root).toMatchRenderedOutput('Bye');
   });
 
+  // TODO FIX!!!
   // @gate experimental || www
-  test('refreshing a parent cache also refreshes its children', async () => {
+  test.only('refreshing a parent cache also refreshes its children', async () => {
     let refreshShell;
     function RefreshShell() {
       refreshShell = useCacheRefresh();
@@ -642,6 +882,7 @@ describe('ReactCache', () => {
     }
 
     const root = ReactNoop.createRoot();
+    console.log('*** initial render ***');
     await act(async () => {
       seedNextTextCache('A');
       root.render(<App showMore={false} />);
@@ -650,6 +891,7 @@ describe('ReactCache', () => {
     expect(root).toMatchRenderedOutput('A [v1]');
 
     // Add a new cache boundary
+    console.log('*** new cache boundary ***');
     await act(async () => {
       seedNextTextCache('A');
       root.render(<App showMore={true} />);
@@ -663,6 +905,7 @@ describe('ReactCache', () => {
 
     // Now refresh the shell. This should also cause the "Show More" contents to
     // refresh, since its cache is nested inside the outer one.
+    console.log('*** refresh shell ***');
     await act(async () => {
       startTransition(() => refreshShell());
     });
@@ -673,13 +916,32 @@ describe('ReactCache', () => {
     ]);
     expect(root).toMatchRenderedOutput('A [v1]A [v2]');
 
+    console.log('*** resolve text ***');
     await act(async () => {
       resolveMostRecentTextCache('A');
     });
-    expect(Scheduler).toHaveYielded(['A [v3]', 'A [v3]']);
+    expect(Scheduler).toHaveYielded([
+      'A [v3]',
+      'A [v3]',
+      'Cache cleanup: A [v2] (cached)',
+    ]);
     expect(root).toMatchRenderedOutput('A [v3]A [v3]');
+
+    // Replace all the children: this should clear *both* cache instances:
+    // the root doesn't have a cache instance (since it wasn't accessed
+    // during the initial render, and all subsequent cache accesses were within
+    // a fresh boundary). Therefore this causes cleanup for both the fresh cache
+    // instance in the refreshed first boundary and cleanup for the non-refreshed
+    // sibling boundary.
+    console.log('*** remove children ***');
+    await act(async => {
+      root.render('Bye!');
+    });
+    expect(Scheduler).toHaveYielded([]);
+    expect(root).toMatchRenderedOutput('Bye!');
   });
 
+  // TODO OK
   // @gate experimental || www
   test(
     'refreshing a cache boundary does not refresh the other boundaries ' +
@@ -750,9 +1012,29 @@ describe('ReactCache', () => {
       });
       expect(Scheduler).toHaveYielded(['A [v2]']);
       expect(root).toMatchRenderedOutput('A [v2]A [v1]');
+
+      // Replace all the children: this should clear *both* cache instances:
+      // the root doesn't have a cache instance (since it wasn't accessed
+      // during the initial render, and all subsequent cache accesses were within
+      // a fresh boundary). Therefore this causes cleanup for both the fresh cache
+      // instance in the refreshed first boundary and cleanup for the non-refreshed
+      // sibling boundary.
+      await act(async => {
+        root.render('Bye!');
+      });
+      expect(Scheduler).toHaveYielded([
+        'Cache cleanup: A [v2]',
+        'Cache cleanup: A [v2] (cached)',
+        'Cache cleanup: A [v1]',
+        'Cache cleanup: A [v1] (cached)',
+        'Cache cleanup: A [v1] (cached)',
+        'Cache cleanup: A [v1] (cached)',
+      ]);
+      expect(root).toMatchRenderedOutput('Bye!');
     },
   );
 
+  // TODO FIX!!!
   // @gate experimental || www
   test(
     'mount a new Cache boundary in a sibling while simultaneously ' +
@@ -788,6 +1070,7 @@ describe('ReactCache', () => {
         'Cache miss! [B]',
         'Loading...',
       ]);
+      expect(root).toMatchRenderedOutput('Loading...');
 
       await act(async () => {
         // This will resolve the content in the first cache
@@ -804,7 +1087,9 @@ describe('ReactCache', () => {
         // requests are not dropped.
         'A [v1]',
         'B [v1]',
+        'Cache cleanup: A [v2]', // TODO FIX!!!
       ]);
+      expect(root).toMatchRenderedOutput('Loading... A [v1] B [v1]');
 
       // Now resolve the second tree
       await act(async () => {
@@ -812,9 +1097,21 @@ describe('ReactCache', () => {
       });
       expect(Scheduler).toHaveYielded(['A [v2]']);
       expect(root).toMatchRenderedOutput('A [v2] A [v1] B [v1]');
+
+      await act(async => {
+        root.render('Bye!');
+      });
+      expect(Scheduler).toHaveYielded([
+        // TODO FIX!!!
+        // 'Cache cleanup: A [v2]',
+        // 'Cache cleanup: A [v1]',
+        // 'Cache cleanup: B [v1]',
+      ]);
+      expect(root).toMatchRenderedOutput('Bye!');
     },
   );
 
+  // TODO unsure about this one
   // @gate experimental || www
   test('cache pool is cleared once transitions that depend on it commit their shell', async () => {
     function Child({text}) {
@@ -896,8 +1193,27 @@ describe('ReactCache', () => {
     });
     expect(Scheduler).toHaveYielded(['A [v1]', 'A [v1]', 'A [v2]']);
     expect(root).toMatchRenderedOutput('A [v1]A [v1]A [v2]');
+
+    await act(async => {
+      root.render('Bye!');
+    });
+    expect(Scheduler).toHaveYielded([
+      'Cache cleanup: A [v1]',
+      'Cache cleanup: A [v1] (cached)',
+      'Cache cleanup: A [v1] (cached)',
+      'Cache cleanup: A [v1] (cached)',
+      'Cache cleanup: A [v1] (cached)',
+      'Cache cleanup: A [v1] (cached)',
+      'Cache cleanup: A [v1] (cached)',
+      'Cache cleanup: A [v1] (cached)',
+      'Cache cleanup: A [v1] (cached)',
+      'Cache cleanup: A [v2]',
+      'Cache cleanup: A [v2] (cached)',
+    ]);
+    expect(root).toMatchRenderedOutput('Bye!');
   });
 
+  // TODO unsure about this one
   // @gate experimental || www
   test('cache pool is not cleared by arbitrary commits', async () => {
     function App() {
@@ -935,7 +1251,7 @@ describe('ReactCache', () => {
 
     const root = ReactNoop.createRoot();
     await act(async () => {
-      root.render(<App showMore={false} />);
+      root.render(<App />);
     });
     expect(Scheduler).toHaveYielded(['0']);
     expect(root).toMatchRenderedOutput('0');
@@ -953,6 +1269,8 @@ describe('ReactCache', () => {
     });
     expect(Scheduler).toHaveYielded([
       '1',
+      'Cache cleanup: A [v1]',
+      'Cache miss! [A]',
 
       // Happens to re-render the fallback. Doesn't need to, but not relevant
       // to this test.
@@ -965,5 +1283,14 @@ describe('ReactCache', () => {
     });
     expect(Scheduler).toHaveYielded(['A']);
     expect(root).toMatchRenderedOutput('A1');
+
+    await act(async => {
+      root.render('Bye!');
+    });
+    expect(Scheduler).toHaveYielded([
+      'Cache cleanup: A [v2]',
+      'Cache cleanup: A [v2] (cached)',
+    ]);
+    expect(root).toMatchRenderedOutput('Bye!');
   });
 });

From 17c2b44c34b92c3859c0dbd243ba636fde18e8ec Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Tue, 5 Oct 2021 09:06:43 -0700
Subject: [PATCH 11/25] existing tests cleanup as expected

---
 .../src/ReactFiberCacheComponent.new.js       | 33 ++++++++-----
 .../src/ReactFiberCommitWork.new.js           | 28 ++++++-----
 .../src/ReactFiberUnwindWork.new.js           |  9 ++--
 .../src/ReactFiberWorkLoop.new.js             |  3 --
 .../src/__tests__/ReactCache-test.js          | 49 ++++++++-----------
 5 files changed, 61 insertions(+), 61 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
index 0813b779696..246d1355ba3 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
@@ -18,11 +18,13 @@ import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols';
 import {isPrimaryRenderer} from './ReactFiberHostConfig';
 import {createCursor, push, pop} from './ReactFiberStack.new';
 import {pushProvider, popProvider} from './ReactFiberNewContext.new';
+import {string} from 'yargs';
 
 export type Cache = {|
   controller: AbortController,
   data: Map<() => mixed, mixed>,
   refCount: number,
+  key?: string,
 |};
 
 export type CacheComponentState = {|
@@ -76,7 +78,7 @@ export function createCache(): Cache {
   const cache: Cache = {
     controller: new AbortController(),
     data: new Map(),
-    refCount: 1,
+    refCount: 0,
   };
 
   // TODO: remove debugging code
@@ -91,11 +93,16 @@ export function createCache(): Cache {
 // When the returned cache instance is no longer reference it must be cleaned up by
 // calling `releaseCache(clone)`
 export function cloneCache(cache: Cache): Cache {
-  trace(`cloneCache #${cache.key} ${cache.refCount} -> ${cache.refCount + 1}`);
-  cache.refCount++;
+  // trace(`cloneCache #${cache.key} ${cache.refCount} -> ${cache.refCount + 1}`);
+  // cache.refCount++;
   return cache;
 }
 
+export function retainCache(cache: Cache) {
+  trace(`retainCache #${cache.key} ${cache.refCount} -> ${cache.refCount + 1}`);
+  cache.refCount++;
+}
+
 // Cleanup a cache instance, potentially freeing it if there are no more references
 export function releaseCache(cache: Cache) {
   trace(
@@ -104,9 +111,9 @@ export function releaseCache(cache: Cache) {
   cache.refCount--;
   if (__DEV__) {
     if (cache.refCount < 0) {
-      throw new Error(
-        'Error in React: cache reference count should not be negative',
-      );
+      // throw new Error(
+      //   'Error in React: cache reference count should not be negative',
+      // );
     }
   }
   if (cache.refCount === 0) {
@@ -117,13 +124,13 @@ export function releaseCache(cache: Cache) {
 }
 
 function trace(msg, levels = 2) {
-  console.log(
-    `${msg}:\n` +
-      new Error().stack
-        .split('\n')
-        .slice(3, 3 + Math.max(levels, 1))
-        .join('\n'),
-  );
+  // console.log(
+  //   `${msg}:\n` +
+  //     new Error().stack
+  //       .split('\n')
+  //       .slice(3, 3 + Math.max(levels, 1))
+  //       .join('\n'),
+  // );
 }
 
 export function pushCacheProvider(workInProgress: Fiber, cache: Cache) {
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
index b3b39acddbf..af60c6c436d 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
@@ -146,7 +146,7 @@ import {
 import {didWarnAboutReassigningProps} from './ReactFiberBeginWork.new';
 import {doesFiberContain} from './ReactFiberTreeReflection';
 import {invokeGuardedCallback, clearCaughtError} from 'shared/ReactErrorUtils';
-import {releaseCache} from './ReactFiberCacheComponent.new';
+import {releaseCache, retainCache} from './ReactFiberCacheComponent.new';
 
 let didWarnAboutUndefinedSnapshotBeforeUpdate: Set<mixed> | null = null;
 if (__DEV__) {
@@ -2628,12 +2628,18 @@ function commitPassiveMountOnFiber(
         // - retain next cache
         // - if prev cache non-null: release (or move to unmount phase?)
         // add comment that retain/release on child is technically not required
-        const previousCache: Cache =
+        const previousCache: ?Cache =
           finishedWork.alternate?.memoizedState.cache;
         const nextCache: Cache = finishedWork.memoizedState.cache;
-        if (nextCache !== previousCache && previousCache != null) {
-          console.log('update HostRoot cache');
-          releaseCache(previousCache);
+        const nextCacheRetained = (nextCache: any).retained;
+        if (nextCache !== previousCache) {
+          retainCache(nextCache);
+          if (previousCache != null) {
+            releaseCache(previousCache);
+          }
+        } else if (!nextCacheRetained) {
+          retainCache(nextCache);
+          (nextCache: any).retained = true;
         }
       }
       break;
@@ -2645,12 +2651,14 @@ function commitPassiveMountOnFiber(
         // - retain next cache
         // - if prev cache non-null: release (or move to unmount phase?)
         // add comment that retain/release on child is technically not required
-        const previousCache: Cache =
+        const previousCache: ?Cache =
           finishedWork.alternate?.memoizedState.cache;
         const nextCache: Cache = finishedWork.memoizedState.cache;
-        if (nextCache !== previousCache && previousCache != null) {
-          console.log('update CacheComponent cache');
-          releaseCache(previousCache);
+        if (nextCache !== previousCache) {
+          retainCache(nextCache);
+          if (previousCache != null) {
+            releaseCache(previousCache);
+          }
         }
       }
       break;
@@ -2864,7 +2872,6 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber(
     case HostRoot: {
       if (enableCache) {
         const cache = current.memoizedState.cache;
-        console.log('release HostRoot cache');
         releaseCache(cache);
       }
       break;
@@ -2872,7 +2879,6 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber(
     case CacheComponent: {
       if (enableCache) {
         const cache = current.memoizedState.cache;
-        console.log('release CacheComponent cache');
         releaseCache(cache);
       }
       break;
diff --git a/packages/react-reconciler/src/ReactFiberUnwindWork.new.js b/packages/react-reconciler/src/ReactFiberUnwindWork.new.js
index 1c096f3e207..97090f9e875 100644
--- a/packages/react-reconciler/src/ReactFiberUnwindWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberUnwindWork.new.js
@@ -11,11 +11,7 @@ import type {ReactContext} from 'shared/ReactTypes';
 import type {Fiber, FiberRoot} from './ReactInternalTypes';
 import type {Lanes} from './ReactFiberLane.new';
 import type {SuspenseState} from './ReactFiberSuspenseComponent.new';
-import {
-  Cache,
-  releaseCache,
-  SpawnedCachePool,
-} from './ReactFiberCacheComponent.new';
+import type {Cache, SpawnedCachePool} from './ReactFiberCacheComponent.new';
 
 import {resetWorkInProgressVersions as resetMutableSourceWorkInProgressVersions} from './ReactMutableSource.new';
 import {
@@ -54,6 +50,7 @@ import {
   popCachePool,
 } from './ReactFiberCacheComponent.new';
 import {transferActualDuration} from './ReactProfilerTimer.new';
+import {releaseCache} from './ReactFiberCacheComponent.new';
 
 import invariant from 'shared/invariant';
 
@@ -157,7 +154,7 @@ function unwindWork(workInProgress: Fiber, renderLanes: Lanes) {
       if (enableCache) {
         const cache: Cache = workInProgress.memoizedState.cache;
         popCacheProvider(workInProgress, cache);
-        releaseCache(cache);
+        // releaseCache(cache);
       }
       return null;
     default:
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
index 5960a68b03b..cdd1628fcc1 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -237,7 +237,6 @@ import {
   isDevToolsPresent,
 } from './ReactFiberDevToolsHook.new';
 import {onCommitRoot as onCommitRootTestSelector} from './ReactTestSelectors';
-import {releaseCache} from './ReactFiberCacheComponent.new';
 
 const ceil = Math.ceil;
 
@@ -2115,7 +2114,6 @@ function releaseRootPooledCache(root) {
   if (enableCache) {
     const pooledCache = root.pooledCache;
     if (pooledCache != null) {
-      releaseCache(pooledCache);
       root.pooledCache = null;
     }
   }
@@ -2160,7 +2158,6 @@ export function enqueuePendingPassiveProfilerEffect(fiber: Fiber): void {
 
 function flushPassiveEffectsImpl() {
   if (rootWithPendingPassiveEffects === null) {
-    console.log('flushPassiveEffectsImpl bailout');
     return false;
   }
 
diff --git a/packages/react-reconciler/src/__tests__/ReactCache-test.js b/packages/react-reconciler/src/__tests__/ReactCache-test.js
index 66c57e55853..6d17a44045a 100644
--- a/packages/react-reconciler/src/__tests__/ReactCache-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactCache-test.js
@@ -173,7 +173,6 @@ describe('ReactCache', () => {
     function Example(props) {
       return <Cache>{props.text}</Cache>;
     }
-    console.log('render: hi');
     await act(async () => {
       root.render(<Example text="Hi" />);
     });
@@ -580,14 +579,12 @@ describe('ReactCache', () => {
     expect(Scheduler).toHaveYielded([]);
     expect(root).toMatchRenderedOutput('(empty)');
 
-    console.log('*** show cache boundaries ***');
     await act(async () => {
       root.render(<App showMore={true} />);
     });
     expect(Scheduler).toHaveYielded(['Cache miss! [A]', 'Loading shell...']);
     expect(root).toMatchRenderedOutput('Loading shell...');
 
-    console.log('*** resolve A ***');
     await act(async () => {
       resolveMostRecentTextCache('A');
     });
@@ -605,7 +602,6 @@ describe('ReactCache', () => {
       </>,
     );
 
-    console.log('*** resolve B ***');
     await act(async () => {
       resolveMostRecentTextCache('B');
     });
@@ -617,7 +613,6 @@ describe('ReactCache', () => {
       </>,
     );
 
-    console.log('*** unmount children ***');
     await act(async () => {
       root.render('Bye');
     });
@@ -730,8 +725,8 @@ describe('ReactCache', () => {
     // Note that the version has updated, and the previous cache is cleared
     expect(Scheduler).toHaveYielded([
       'A [v2]',
-      'Cache cleanup: A [v1]', // initial suspended render
-      'Cache cleanup: A [v1] (cached)', // resumption
+      'Cache cleanup: A [v1]', // from initial suspended render
+      'Cache cleanup: A [v1] (cached)', // from resumption
     ]);
     expect(root).toMatchRenderedOutput('A [v2]');
 
@@ -854,9 +849,8 @@ describe('ReactCache', () => {
     expect(root).toMatchRenderedOutput('Bye');
   });
 
-  // TODO FIX!!!
   // @gate experimental || www
-  test.only('refreshing a parent cache also refreshes its children', async () => {
+  test('refreshing a parent cache also refreshes its children', async () => {
     let refreshShell;
     function RefreshShell() {
       refreshShell = useCacheRefresh();
@@ -882,7 +876,6 @@ describe('ReactCache', () => {
     }
 
     const root = ReactNoop.createRoot();
-    console.log('*** initial render ***');
     await act(async () => {
       seedNextTextCache('A');
       root.render(<App showMore={false} />);
@@ -891,7 +884,6 @@ describe('ReactCache', () => {
     expect(root).toMatchRenderedOutput('A [v1]');
 
     // Add a new cache boundary
-    console.log('*** new cache boundary ***');
     await act(async () => {
       seedNextTextCache('A');
       root.render(<App showMore={true} />);
@@ -905,7 +897,6 @@ describe('ReactCache', () => {
 
     // Now refresh the shell. This should also cause the "Show More" contents to
     // refresh, since its cache is nested inside the outer one.
-    console.log('*** refresh shell ***');
     await act(async () => {
       startTransition(() => refreshShell());
     });
@@ -916,28 +907,30 @@ describe('ReactCache', () => {
     ]);
     expect(root).toMatchRenderedOutput('A [v1]A [v2]');
 
-    console.log('*** resolve text ***');
     await act(async () => {
       resolveMostRecentTextCache('A');
     });
     expect(Scheduler).toHaveYielded([
       'A [v3]',
       'A [v3]',
+      // once the refresh completes the inner showMore boundary frees its previous
+      // cache instance, since it is now using the refreshed parent instance.
+      // note that the entry is "(cached)" because the cache was seeded w a value.
       'Cache cleanup: A [v2] (cached)',
     ]);
     expect(root).toMatchRenderedOutput('A [v3]A [v3]');
 
-    // Replace all the children: this should clear *both* cache instances:
-    // the root doesn't have a cache instance (since it wasn't accessed
-    // during the initial render, and all subsequent cache accesses were within
-    // a fresh boundary). Therefore this causes cleanup for both the fresh cache
-    // instance in the refreshed first boundary and cleanup for the non-refreshed
-    // sibling boundary.
-    console.log('*** remove children ***');
     await act(async => {
       root.render('Bye!');
     });
-    expect(Scheduler).toHaveYielded([]);
+    // Unmounting children releases the refreshed cache instance only; the root
+    // still retains the original cache instance used for the first render
+    expect(Scheduler).toHaveYielded([
+      'Cache cleanup: A [v3]',
+      'Cache cleanup: A [v3] (cached)',
+      'Cache cleanup: A [v3] (cached)',
+      'Cache cleanup: A [v3] (cached)',
+    ]);
     expect(root).toMatchRenderedOutput('Bye!');
   });
 
@@ -1034,7 +1027,6 @@ describe('ReactCache', () => {
     },
   );
 
-  // TODO FIX!!!
   // @gate experimental || www
   test(
     'mount a new Cache boundary in a sibling while simultaneously ' +
@@ -1087,7 +1079,6 @@ describe('ReactCache', () => {
         // requests are not dropped.
         'A [v1]',
         'B [v1]',
-        'Cache cleanup: A [v2]', // TODO FIX!!!
       ]);
       expect(root).toMatchRenderedOutput('Loading... A [v1] B [v1]');
 
@@ -1101,11 +1092,12 @@ describe('ReactCache', () => {
       await act(async => {
         root.render('Bye!');
       });
+      // Unmounting children releases both cache boundaries, but the original
+      // cache instance (used by second boundary) is still referenced by the root.
+      // only the second cache instance is freed.
       expect(Scheduler).toHaveYielded([
-        // TODO FIX!!!
-        // 'Cache cleanup: A [v2]',
-        // 'Cache cleanup: A [v1]',
-        // 'Cache cleanup: B [v1]',
+        'Cache cleanup: A [v2]',
+        'Cache cleanup: A [v2] (cached)',
       ]);
       expect(root).toMatchRenderedOutput('Bye!');
     },
@@ -1269,7 +1261,6 @@ describe('ReactCache', () => {
     });
     expect(Scheduler).toHaveYielded([
       '1',
-      'Cache cleanup: A [v1]',
       'Cache miss! [A]',
 
       // Happens to re-render the fallback. Doesn't need to, but not relevant
@@ -1287,6 +1278,8 @@ describe('ReactCache', () => {
     await act(async => {
       root.render('Bye!');
     });
+    // the cache boundary's fresh cache is freed, but the original root cache
+    // is still retained by the root
     expect(Scheduler).toHaveYielded([
       'Cache cleanup: A [v2]',
       'Cache cleanup: A [v2] (cached)',

From e5c2575883bc7705cffe7d613ab955513f0af524 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Tue, 5 Oct 2021 09:20:40 -0700
Subject: [PATCH 12/25] cleanup, lint, sync fork

---
 .../src/ReactFiberBeginWork.new.js            |  3 +-
 .../src/ReactFiberCacheComponent.new.js       | 72 +++++++++----------
 .../src/ReactFiberCacheComponent.old.js       | 64 +++++++++++++----
 .../src/ReactFiberCommitWork.old.js           | 62 ++++++++++++++++
 .../src/ReactFiberCompleteWork.old.js         |  3 +
 .../src/ReactFiberRoot.new.js                 |  4 +-
 .../src/ReactFiberUnwindWork.new.js           |  2 -
 .../src/ReactFiberWorkLoop.old.js             | 20 +++---
 8 files changed, 163 insertions(+), 67 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js
index d70bddf476d..6ad63ae763c 100644
--- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js
@@ -223,7 +223,6 @@ import {
 } from './ReactFiberWorkLoop.new';
 import {setWorkInProgressVersion} from './ReactMutableSource.new';
 import {
-  cloneCache,
   requestCacheFromPool,
   pushCacheProvider,
   pushRootCachePool,
@@ -850,7 +849,7 @@ function updateCacheComponent(
       // Refresh in parent. Update the parent.
       const derivedState: CacheComponentState = {
         parent: parentCache,
-        cache: cloneCache(parentCache),
+        cache: parentCache,
       };
 
       // Copied from getDerivedStateFromProps implementation. Once the update
diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
index 246d1355ba3..b2d72f7e955 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
@@ -18,7 +18,6 @@ import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols';
 import {isPrimaryRenderer} from './ReactFiberHostConfig';
 import {createCursor, push, pop} from './ReactFiberStack.new';
 import {pushProvider, popProvider} from './ReactFiberNewContext.new';
-import {string} from 'yargs';
 
 export type Cache = {|
   controller: AbortController,
@@ -63,16 +62,9 @@ let pooledCache: Cache | null = null;
 // cache from the render that suspended.
 const prevFreshCacheOnStack: StackCursor<Cache | null> = createCursor(null);
 
-// Creates a new empty Cache instance with a ref-count of 1: the caller is
-// the implicit "owner" of the cache instance. The developer is responsible
-// for calling `releaseCache(cache)` when the reference to this cache is no
-// longer reachable.
-//
-// To create additional references to a Cache (ie to share it across the root,
-// multiple <Cache> components, etc), create a new "clone" with
-// eg `clone = cloneCache(cache)`. The developer is then calling
-// `releaseCache(clone)` for each clone created w cloneCache, when that clone
-// is no longer reachable.
+// Creates a new empty Cache instance with a ref-count of 0. The caller is responsible
+// for retaining the cache once it is in use (retainCache), and releasing the cache
+// once it is no longer needed (releaseCache).
 let _cacheIndex = 0;
 export function createCache(): Cache {
   const cache: Cache = {
@@ -82,38 +74,38 @@ export function createCache(): Cache {
   };
 
   // TODO: remove debugging code
-  const key = _cacheIndex++;
-  (cache: any).key = String(key);
-  trace(`createCache #${cache.key}`);
-
-  return cache;
-}
+  if (__DEV__) {
+    cache.key = '' + _cacheIndex++;
+    trace(`createCache #${cache.key ?? ''}`);
+  }
 
-// Creates a lightweight clone of the given Cache instance that *shares the same data*.
-// When the returned cache instance is no longer reference it must be cleaned up by
-// calling `releaseCache(clone)`
-export function cloneCache(cache: Cache): Cache {
-  // trace(`cloneCache #${cache.key} ${cache.refCount} -> ${cache.refCount + 1}`);
-  // cache.refCount++;
   return cache;
 }
 
 export function retainCache(cache: Cache) {
-  trace(`retainCache #${cache.key} ${cache.refCount} -> ${cache.refCount + 1}`);
+  if (__DEV__) {
+    trace(
+      `retainCache #${cache.key ?? ''} ${cache.refCount} -> ${cache.refCount +
+        1}`,
+    );
+  }
   cache.refCount++;
 }
 
 // Cleanup a cache instance, potentially freeing it if there are no more references
 export function releaseCache(cache: Cache) {
-  trace(
-    `releaseCache #${cache.key} ${cache.refCount} -> ${cache.refCount - 1}`,
-  );
+  if (__DEV__) {
+    trace(
+      `releaseCache #${cache.key ?? ''} ${cache.refCount} -> ${cache.refCount -
+        1}`,
+    );
+  }
   cache.refCount--;
   if (__DEV__) {
     if (cache.refCount < 0) {
-      // throw new Error(
-      //   'Error in React: cache reference count should not be negative',
-      // );
+      throw new Error(
+        'Error in React: cache reference count should not be negative',
+      );
     }
   }
   if (cache.refCount === 0) {
@@ -124,13 +116,15 @@ export function releaseCache(cache: Cache) {
 }
 
 function trace(msg, levels = 2) {
-  // console.log(
-  //   `${msg}:\n` +
-  //     new Error().stack
-  //       .split('\n')
-  //       .slice(3, 3 + Math.max(levels, 1))
-  //       .join('\n'),
-  // );
+  if (__DEV__) {
+    // console.log(
+    //   `${msg}:\n` +
+    //     new Error().stack
+    //       .split('\n')
+    //       .slice(3, 3 + Math.max(levels, 1))
+    //       .join('\n'),
+    // );
+  }
 }
 
 export function pushCacheProvider(workInProgress: Fiber, cache: Cache) {
@@ -152,11 +146,11 @@ export function requestCacheFromPool(renderLanes: Lanes): Cache {
     return (null: any);
   }
   if (pooledCache !== null) {
-    return cloneCache(pooledCache);
+    return pooledCache;
   }
   // Create a fresh cache.
   pooledCache = createCache();
-  return cloneCache(pooledCache);
+  return pooledCache;
 }
 
 export function pushRootCachePool(root: FiberRoot) {
diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.old.js b/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
index df5a49e4aae..932c02eb859 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
@@ -23,6 +23,7 @@ export type Cache = {|
   controller: AbortController,
   data: Map<() => mixed, mixed>,
   refCount: number,
+  key?: string,
 |};
 
 export type CacheComponentState = {|
@@ -61,27 +62,44 @@ let pooledCache: Cache | null = null;
 // cache from the render that suspended.
 const prevFreshCacheOnStack: StackCursor<Cache | null> = createCursor(null);
 
-// Creates a new empty Cache instance with a ref-count of 1: the caller is
-// the implicit "owner". Note that retain/release must be called to ensure
-// that the ref count is accurate:
-// * Call retainCache() when a new reference to the cache instance is created.
-//   This *excludes* the original reference created w createCache().
-// * Call releaseCache() when any reference to the cache is "released" (ie
-//   when the reference is no longer reachable). This *includes* the original
-//   reference created w createCache().
+// Creates a new empty Cache instance with a ref-count of 0. The caller is responsible
+// for retaining the cache once it is in use (retainCache), and releasing the cache
+// once it is no longer needed (releaseCache).
+let _cacheIndex = 0;
 export function createCache(): Cache {
-  return {
+  const cache: Cache = {
     controller: new AbortController(),
     data: new Map(),
-    refCount: 1,
+    refCount: 0,
   };
+
+  // TODO: remove debugging code
+  if (__DEV__) {
+    cache.key = '' + _cacheIndex++;
+    trace(`createCache #${cache.key ?? ''}`);
+  }
+
+  return cache;
 }
 
 export function retainCache(cache: Cache) {
+  if (__DEV__) {
+    trace(
+      `retainCache #${cache.key ?? ''} ${cache.refCount} -> ${cache.refCount +
+        1}`,
+    );
+  }
   cache.refCount++;
 }
 
+// Cleanup a cache instance, potentially freeing it if there are no more references
 export function releaseCache(cache: Cache) {
+  if (__DEV__) {
+    trace(
+      `releaseCache #${cache.key ?? ''} ${cache.refCount} -> ${cache.refCount -
+        1}`,
+    );
+  }
   cache.refCount--;
   if (__DEV__) {
     if (cache.refCount < 0) {
@@ -91,12 +109,24 @@ export function releaseCache(cache: Cache) {
     }
   }
   if (cache.refCount === 0) {
-    // TODO: considering scheduling and error handling for any
-    // event listeners that get triggered.
+    // TODO: considering scheduling this call, and adding error handling for
+    // any event listeners that get triggered.
     cache.controller.abort();
   }
 }
 
+function trace(msg, levels = 2) {
+  if (__DEV__) {
+    // console.log(
+    //   `${msg}:\n` +
+    //     new Error().stack
+    //       .split('\n')
+    //       .slice(3, 3 + Math.max(levels, 1))
+    //       .join('\n'),
+    // );
+  }
+}
+
 export function pushCacheProvider(workInProgress: Fiber, cache: Cache) {
   if (!enableCache) {
     return;
@@ -131,7 +161,14 @@ export function pushRootCachePool(root: FiberRoot) {
   // from `root.pooledCache`. If it's currently `null`, we will lazily
   // initialize it the first type it's requested. However, we only mutate
   // the root itself during the complete/unwind phase of the HostRoot.
-  pooledCache = root.pooledCache;
+  const rootCache = root.pooledCache;
+  if (rootCache != null) {
+    trace('pushRootCachePool');
+    pooledCache = rootCache;
+    root.pooledCache = null;
+  } else {
+    pooledCache = null;
+  }
 }
 
 export function popRootCachePool(root: FiberRoot, renderLanes: Lanes) {
@@ -145,6 +182,7 @@ export function popRootCachePool(root: FiberRoot, renderLanes: Lanes) {
   // on it (which we track with `pooledCacheLanes`) have committed.
   root.pooledCache = pooledCache;
   if (pooledCache !== null) {
+    trace('popRootCachePool');
     root.pooledCacheLanes |= renderLanes;
   }
   // set to null, conceptually we are moving ownership to the root
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js
index 53294e67498..03588a8bb8f 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js
@@ -24,6 +24,7 @@ import type {FunctionComponentUpdateQueue} from './ReactFiberHooks.old';
 import type {Wakeable} from 'shared/ReactTypes';
 import type {OffscreenState} from './ReactFiberOffscreenComponent';
 import type {HookFlags} from './ReactHookEffectTags';
+import type {Cache} from './ReactFiberCacheComponent.old';
 
 import {
   enableCreateEventHandleAPI,
@@ -38,6 +39,7 @@ import {
   enableSuspenseLayoutEffectSemantics,
   enableUpdaterTracking,
   warnAboutCallbackRefReturningFunction,
+  enableCache,
 } from 'shared/ReactFeatureFlags';
 import {
   FunctionComponent,
@@ -57,6 +59,7 @@ import {
   ScopeComponent,
   OffscreenComponent,
   LegacyHiddenComponent,
+  CacheComponent,
 } from './ReactWorkTags';
 import {detachDeletedInstance} from './ReactFiberHostConfig';
 import {
@@ -143,6 +146,7 @@ import {
 import {didWarnAboutReassigningProps} from './ReactFiberBeginWork.old';
 import {doesFiberContain} from './ReactFiberTreeReflection';
 import {invokeGuardedCallback, clearCaughtError} from 'shared/ReactErrorUtils';
+import {releaseCache, retainCache} from './ReactFiberCacheComponent.old';
 
 let didWarnAboutUndefinedSnapshotBeforeUpdate: Set<mixed> | null = null;
 if (__DEV__) {
@@ -2594,6 +2598,8 @@ function commitPassiveMountEffects_complete(
 function commitPassiveMountOnFiber(
   finishedRoot: FiberRoot,
   finishedWork: Fiber,
+  // maybe thread through previous fiber or cache from previous fiber
+  //
 ): void {
   switch (finishedWork.tag) {
     case FunctionComponent:
@@ -2615,6 +2621,48 @@ function commitPassiveMountOnFiber(
       }
       break;
     }
+    case HostRoot: {
+      if (enableCache) {
+        // todo compare caches here
+        // if caches not same
+        // - retain next cache
+        // - if prev cache non-null: release (or move to unmount phase?)
+        // add comment that retain/release on child is technically not required
+        const previousCache: ?Cache =
+          finishedWork.alternate?.memoizedState.cache;
+        const nextCache: Cache = finishedWork.memoizedState.cache;
+        const nextCacheRetained = (nextCache: any).retained;
+        if (nextCache !== previousCache) {
+          retainCache(nextCache);
+          if (previousCache != null) {
+            releaseCache(previousCache);
+          }
+        } else if (!nextCacheRetained) {
+          retainCache(nextCache);
+          (nextCache: any).retained = true;
+        }
+      }
+      break;
+    }
+    case CacheComponent: {
+      if (enableCache) {
+        // todo compare caches here
+        // if caches not same
+        // - retain next cache
+        // - if prev cache non-null: release (or move to unmount phase?)
+        // add comment that retain/release on child is technically not required
+        const previousCache: ?Cache =
+          finishedWork.alternate?.memoizedState.cache;
+        const nextCache: Cache = finishedWork.memoizedState.cache;
+        if (nextCache !== previousCache) {
+          retainCache(nextCache);
+          if (previousCache != null) {
+            releaseCache(previousCache);
+          }
+        }
+      }
+      break;
+    }
   }
 }
 
@@ -2821,6 +2869,20 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber(
       }
       break;
     }
+    case HostRoot: {
+      if (enableCache) {
+        const cache = current.memoizedState.cache;
+        releaseCache(cache);
+      }
+      break;
+    }
+    case CacheComponent: {
+      if (enableCache) {
+        const cache = current.memoizedState.cache;
+        releaseCache(cache);
+      }
+      break;
+    }
   }
 }
 
diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.old.js b/packages/react-reconciler/src/ReactFiberCompleteWork.old.js
index 1155a817d61..e7766864fea 100644
--- a/packages/react-reconciler/src/ReactFiberCompleteWork.old.js
+++ b/packages/react-reconciler/src/ReactFiberCompleteWork.old.js
@@ -72,6 +72,7 @@ import {
   ChildDeletion,
   StaticMask,
   MutationMask,
+  Passive,
 } from './ReactFiberFlags';
 import invariant from 'shared/invariant';
 
@@ -847,6 +848,7 @@ function completeWork(
     case HostRoot: {
       const fiberRoot = (workInProgress.stateNode: FiberRoot);
       if (enableCache) {
+        workInProgress.flags |= Passive;
         popRootCachePool(fiberRoot, renderLanes);
 
         const cache: Cache = workInProgress.memoizedState.cache;
@@ -1472,6 +1474,7 @@ function completeWork(
     case CacheComponent: {
       if (enableCache) {
         const cache: Cache = workInProgress.memoizedState.cache;
+        workInProgress.flags |= Passive;
         popCacheProvider(workInProgress, cache);
         bubbleProperties(workInProgress);
         return null;
diff --git a/packages/react-reconciler/src/ReactFiberRoot.new.js b/packages/react-reconciler/src/ReactFiberRoot.new.js
index 1b49e45289c..75a621b610f 100644
--- a/packages/react-reconciler/src/ReactFiberRoot.new.js
+++ b/packages/react-reconciler/src/ReactFiberRoot.new.js
@@ -28,7 +28,7 @@ import {
 } from 'shared/ReactFeatureFlags';
 import {initializeUpdateQueue} from './ReactUpdateQueue.new';
 import {LegacyRoot, ConcurrentRoot} from './ReactRootTags';
-import {cloneCache, createCache} from './ReactFiberCacheComponent.new';
+import {createCache} from './ReactFiberCacheComponent.new';
 
 function FiberRootNode(containerInfo, tag, hydrate) {
   this.tag = tag;
@@ -119,7 +119,7 @@ export function createFiberRoot(
 
   if (enableCache) {
     const initialCache = createCache();
-    root.pooledCache = cloneCache(initialCache);
+    root.pooledCache = initialCache;
     const initialState = {
       element: null,
       cache: initialCache,
diff --git a/packages/react-reconciler/src/ReactFiberUnwindWork.new.js b/packages/react-reconciler/src/ReactFiberUnwindWork.new.js
index 97090f9e875..152837286f5 100644
--- a/packages/react-reconciler/src/ReactFiberUnwindWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberUnwindWork.new.js
@@ -50,7 +50,6 @@ import {
   popCachePool,
 } from './ReactFiberCacheComponent.new';
 import {transferActualDuration} from './ReactProfilerTimer.new';
-import {releaseCache} from './ReactFiberCacheComponent.new';
 
 import invariant from 'shared/invariant';
 
@@ -154,7 +153,6 @@ function unwindWork(workInProgress: Fiber, renderLanes: Lanes) {
       if (enableCache) {
         const cache: Cache = workInProgress.memoizedState.cache;
         popCacheProvider(workInProgress, cache);
-        // releaseCache(cache);
       }
       return null;
     default:
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
index 9b1ccdd3dcb..3f61794a9e0 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
@@ -237,7 +237,6 @@ import {
   isDevToolsPresent,
 } from './ReactFiberDevToolsHook.old';
 import {onCommitRoot as onCommitRootTestSelector} from './ReactTestSelectors';
-import {releaseCache} from './ReactFiberCacheComponent.old';
 
 const ceil = Math.ceil;
 
@@ -2053,7 +2052,7 @@ function commitRootImpl(root, renderPriorityLevel) {
     hasUncaughtError = false;
     const error = firstUncaughtError;
     firstUncaughtError = null;
-    // TODO: how to handle cache cleanup here?
+    releaseRootPooledCache(root);
     throw error;
   }
 
@@ -2096,13 +2095,7 @@ function commitRootImpl(root, renderPriorityLevel) {
 
   // Now that effects have run - giving Cache boundaries a chance to retain the
   // cache instance - release the root's cache in case since the render is complete
-  if (enableCache) {
-    const pooledCache = root.pooledCache;
-    if (pooledCache != null) {
-      releaseCache(pooledCache);
-      root.pooledCache = null;
-    }
-  }
+  releaseRootPooledCache(root);
 
   if (__DEV__) {
     if (enableDebugTracing) {
@@ -2117,6 +2110,15 @@ function commitRootImpl(root, renderPriorityLevel) {
   return null;
 }
 
+function releaseRootPooledCache(root) {
+  if (enableCache) {
+    const pooledCache = root.pooledCache;
+    if (pooledCache != null) {
+      root.pooledCache = null;
+    }
+  }
+}
+
 export function flushPassiveEffects(): boolean {
   // Returns whether passive effects were flushed.
   // TODO: Combine this check with the one in flushPassiveEFfectsImpl. We should

From 650f1089c8b2aa7eeee7b5a78e2230b6e442945b Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Tue, 5 Oct 2021 09:26:13 -0700
Subject: [PATCH 13/25] cleanup test

---
 .../src/__tests__/ReactCache-test.js           | 18 ------------------
 1 file changed, 18 deletions(-)

diff --git a/packages/react-reconciler/src/__tests__/ReactCache-test.js b/packages/react-reconciler/src/__tests__/ReactCache-test.js
index 6d17a44045a..e3e4ed1cd97 100644
--- a/packages/react-reconciler/src/__tests__/ReactCache-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactCache-test.js
@@ -1,5 +1,3 @@
-const {test} = require('jest-snapshot-serializer-raw');
-
 let React;
 let ReactNoop;
 let Cache;
@@ -166,7 +164,6 @@ describe('ReactCache', () => {
     }
   }
 
-  // TODO OK
   // @gate experimental || www
   test('render Cache component', async () => {
     const root = ReactNoop.createRoot();
@@ -179,7 +176,6 @@ describe('ReactCache', () => {
     expect(root).toMatchRenderedOutput('Hi');
   });
 
-  // TODO OK
   // @gate experimental || www
   test('mount new data', async () => {
     const root = ReactNoop.createRoot();
@@ -209,7 +205,6 @@ describe('ReactCache', () => {
     expect(root).toMatchRenderedOutput('Bye');
   });
 
-  // TODO OK
   // @gate experimental || www
   test('root acts as implicit cache boundary', async () => {
     const root = ReactNoop.createRoot();
@@ -237,7 +232,6 @@ describe('ReactCache', () => {
     expect(root).toMatchRenderedOutput('Bye');
   });
 
-  // TODO OK
   // @gate experimental || www
   test('multiple new Cache boundaries in the same update share the same, fresh cache', async () => {
     function App({showMore}) {
@@ -299,7 +293,6 @@ describe('ReactCache', () => {
     expect(root).toMatchRenderedOutput('Bye');
   });
 
-  // TODO OK
   // @gate experimental || www
   test(
     'nested cache boundaries share the same cache as the root during ' +
@@ -340,7 +333,6 @@ describe('ReactCache', () => {
     },
   );
 
-  // TODO OK
   // @gate experimental || www
   test('new content inside an existing Cache boundary should re-use already cached data', async () => {
     function App({showMore}) {
@@ -385,7 +377,6 @@ describe('ReactCache', () => {
     expect(root).toMatchRenderedOutput('Bye');
   });
 
-  // TODO OK
   // @gate experimental || www
   test('a new Cache boundary uses fresh cache', async () => {
     // The only difference from the previous test is that the "Show More"
@@ -447,7 +438,6 @@ describe('ReactCache', () => {
     expect(root).toMatchRenderedOutput('Bye!');
   });
 
-  // TODO OK
   // @gate experimental || www
   test('inner/outer cache boundaries uses the same cache instance on initial render', async () => {
     const root = ReactNoop.createRoot();
@@ -530,7 +520,6 @@ describe('ReactCache', () => {
     expect(root).toMatchRenderedOutput('Bye');
   });
 
-  // TODO OK
   // @gate experimental || www
   test('inner/ outer cache boundaries added in the same update use the same cache instance', async () => {
     const root = ReactNoop.createRoot();
@@ -630,7 +619,6 @@ describe('ReactCache', () => {
     expect(root).toMatchRenderedOutput('Bye');
   });
 
-  // TODO OK
   // @gate experimental || www
   test('refresh a cache boundary', async () => {
     let refresh;
@@ -685,7 +673,6 @@ describe('ReactCache', () => {
     expect(root).toMatchRenderedOutput('Bye');
   });
 
-  // TODO OK
   // @gate experimental || www
   test('refresh the root cache', async () => {
     let refresh;
@@ -738,7 +725,6 @@ describe('ReactCache', () => {
     expect(root).toMatchRenderedOutput('Bye');
   });
 
-  // TODO OK
   // @gate experimental || www
   test('refresh the root cache without a transition', async () => {
     let refresh;
@@ -794,7 +780,6 @@ describe('ReactCache', () => {
     expect(root).toMatchRenderedOutput('Bye');
   });
 
-  // TODO OK
   // @gate experimental || www
   test('refresh a cache with seed data', async () => {
     let refresh;
@@ -934,7 +919,6 @@ describe('ReactCache', () => {
     expect(root).toMatchRenderedOutput('Bye!');
   });
 
-  // TODO OK
   // @gate experimental || www
   test(
     'refreshing a cache boundary does not refresh the other boundaries ' +
@@ -1103,7 +1087,6 @@ describe('ReactCache', () => {
     },
   );
 
-  // TODO unsure about this one
   // @gate experimental || www
   test('cache pool is cleared once transitions that depend on it commit their shell', async () => {
     function Child({text}) {
@@ -1205,7 +1188,6 @@ describe('ReactCache', () => {
     expect(root).toMatchRenderedOutput('Bye!');
   });
 
-  // TODO unsure about this one
   // @gate experimental || www
   test('cache pool is not cleared by arbitrary commits', async () => {
     function App() {

From 0bf386c8ea4e547e7dd0073f8918a5871d3a99dc Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Tue, 5 Oct 2021 15:22:33 -0700
Subject: [PATCH 14/25] update snapshots

---
 .../SchedulingProfiler-test.internal.js       | 322 ++++++++++--------
 .../SchedulingProfilerLabels-test.internal.js | 100 +++---
 2 files changed, 229 insertions(+), 193 deletions(-)

diff --git a/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js b/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js
index 5b58342ee0b..e74d7885878 100644
--- a/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js
+++ b/packages/react-reconciler/src/__tests__/SchedulingProfiler-test.internal.js
@@ -409,30 +409,36 @@ describe('SchedulingProfiler', () => {
 
     if (gate(flags => flags.enableSchedulingProfiler)) {
       expect(getMarks()).toMatchInlineSnapshot(`
-              Array [
-                "--render-start-16",
-                "--component-render-start-Example",
-                "--component-render-stop",
-                "--render-stop",
-                "--commit-start-16",
-                "--react-version-17.0.3",
-                "--profiler-version-1",
-                "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
-                "--layout-effects-start-16",
-                "--schedule-state-update-1-Example",
-                "--layout-effects-stop",
-                "--render-start-1",
-                "--component-render-start-Example",
-                "--component-render-stop",
-                "--render-stop",
-                "--commit-start-1",
-                "--react-version-17.0.3",
-                "--profiler-version-1",
-                "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
-                "--commit-stop",
-                "--commit-stop",
-              ]
-          `);
+        Array [
+          "--render-start-16",
+          "--component-render-start-Example",
+          "--component-render-stop",
+          "--render-stop",
+          "--commit-start-16",
+          "--react-version-17.0.3",
+          "--profiler-version-1",
+          "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
+          "--layout-effects-start-16",
+          "--schedule-state-update-1-Example",
+          "--layout-effects-stop",
+          "--passive-effects-start-16",
+          "--passive-effects-stop",
+          "--render-start-1",
+          "--component-render-start-Example",
+          "--component-render-stop",
+          "--render-stop",
+          "--commit-start-1",
+          "--react-version-17.0.3",
+          "--profiler-version-1",
+          "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
+          "--layout-effects-start-1",
+          "--layout-effects-stop",
+          "--passive-effects-start-1",
+          "--passive-effects-stop",
+          "--commit-stop",
+          "--commit-stop",
+        ]
+      `);
     }
   });
 
@@ -462,30 +468,36 @@ describe('SchedulingProfiler', () => {
 
     if (gate(flags => flags.enableSchedulingProfiler)) {
       expect(getMarks()).toMatchInlineSnapshot(`
-              Array [
-                "--render-start-16",
-                "--component-render-start-Example",
-                "--component-render-stop",
-                "--render-stop",
-                "--commit-start-16",
-                "--react-version-17.0.3",
-                "--profiler-version-1",
-                "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
-                "--layout-effects-start-16",
-                "--schedule-forced-update-1-Example",
-                "--layout-effects-stop",
-                "--render-start-1",
-                "--component-render-start-Example",
-                "--component-render-stop",
-                "--render-stop",
-                "--commit-start-1",
-                "--react-version-17.0.3",
-                "--profiler-version-1",
-                "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
-                "--commit-stop",
-                "--commit-stop",
-              ]
-          `);
+        Array [
+          "--render-start-16",
+          "--component-render-start-Example",
+          "--component-render-stop",
+          "--render-stop",
+          "--commit-start-16",
+          "--react-version-17.0.3",
+          "--profiler-version-1",
+          "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
+          "--layout-effects-start-16",
+          "--schedule-forced-update-1-Example",
+          "--layout-effects-stop",
+          "--passive-effects-start-16",
+          "--passive-effects-stop",
+          "--render-start-1",
+          "--component-render-start-Example",
+          "--component-render-stop",
+          "--render-stop",
+          "--commit-start-1",
+          "--react-version-17.0.3",
+          "--profiler-version-1",
+          "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
+          "--layout-effects-start-1",
+          "--layout-effects-stop",
+          "--passive-effects-start-1",
+          "--passive-effects-stop",
+          "--commit-stop",
+          "--commit-stop",
+        ]
+      `);
     }
   });
 
@@ -608,30 +620,36 @@ describe('SchedulingProfiler', () => {
 
     if (gate(flags => flags.enableSchedulingProfiler)) {
       expect(getMarks()).toMatchInlineSnapshot(`
-              Array [
-                "--render-start-16",
-                "--component-render-start-Example",
-                "--component-render-stop",
-                "--render-stop",
-                "--commit-start-16",
-                "--react-version-17.0.3",
-                "--profiler-version-1",
-                "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
-                "--layout-effects-start-16",
-                "--schedule-state-update-1-Example",
-                "--layout-effects-stop",
-                "--render-start-1",
-                "--component-render-start-Example",
-                "--component-render-stop",
-                "--render-stop",
-                "--commit-start-1",
-                "--react-version-17.0.3",
-                "--profiler-version-1",
-                "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
-                "--commit-stop",
-                "--commit-stop",
-              ]
-          `);
+        Array [
+          "--render-start-16",
+          "--component-render-start-Example",
+          "--component-render-stop",
+          "--render-stop",
+          "--commit-start-16",
+          "--react-version-17.0.3",
+          "--profiler-version-1",
+          "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
+          "--layout-effects-start-16",
+          "--schedule-state-update-1-Example",
+          "--layout-effects-stop",
+          "--passive-effects-start-16",
+          "--passive-effects-stop",
+          "--render-start-1",
+          "--component-render-start-Example",
+          "--component-render-stop",
+          "--render-stop",
+          "--commit-start-1",
+          "--react-version-17.0.3",
+          "--profiler-version-1",
+          "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
+          "--layout-effects-start-1",
+          "--layout-effects-stop",
+          "--passive-effects-start-1",
+          "--passive-effects-stop",
+          "--commit-stop",
+          "--commit-stop",
+        ]
+      `);
     }
   });
 
@@ -652,33 +670,37 @@ describe('SchedulingProfiler', () => {
 
     if (gate(flags => flags.enableSchedulingProfiler)) {
       expect(getMarks()).toMatchInlineSnapshot(`
-              Array [
-                "--schedule-render-16",
-                "--render-start-16",
-                "--component-render-start-Example",
-                "--component-render-stop",
-                "--render-stop",
-                "--commit-start-16",
-                "--react-version-17.0.3",
-                "--profiler-version-1",
-                "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
-                "--layout-effects-start-16",
-                "--layout-effects-stop",
-                "--commit-stop",
-                "--passive-effects-start-16",
-                "--schedule-state-update-16-Example",
-                "--passive-effects-stop",
-                "--render-start-16",
-                "--component-render-start-Example",
-                "--component-render-stop",
-                "--render-stop",
-                "--commit-start-16",
-                "--react-version-17.0.3",
-                "--profiler-version-1",
-                "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
-                "--commit-stop",
-              ]
-          `);
+        Array [
+          "--schedule-render-16",
+          "--render-start-16",
+          "--component-render-start-Example",
+          "--component-render-stop",
+          "--render-stop",
+          "--commit-start-16",
+          "--react-version-17.0.3",
+          "--profiler-version-1",
+          "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
+          "--layout-effects-start-16",
+          "--layout-effects-stop",
+          "--commit-stop",
+          "--passive-effects-start-16",
+          "--schedule-state-update-16-Example",
+          "--passive-effects-stop",
+          "--render-start-16",
+          "--component-render-start-Example",
+          "--component-render-stop",
+          "--render-stop",
+          "--commit-start-16",
+          "--react-version-17.0.3",
+          "--profiler-version-1",
+          "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
+          "--layout-effects-start-16",
+          "--layout-effects-stop",
+          "--commit-stop",
+          "--passive-effects-start-16",
+          "--passive-effects-stop",
+        ]
+      `);
     }
   });
 
@@ -697,22 +719,24 @@ describe('SchedulingProfiler', () => {
 
     if (gate(flags => flags.enableSchedulingProfiler)) {
       expect(getMarks()).toMatchInlineSnapshot(`
-              Array [
-                "--schedule-render-16",
-                "--render-start-16",
-                "--component-render-start-Example",
-                "--schedule-state-update-16-Example",
-                "--component-render-stop",
-                "--render-stop",
-                "--commit-start-16",
-                "--react-version-17.0.3",
-                "--profiler-version-1",
-                "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
-                "--layout-effects-start-16",
-                "--layout-effects-stop",
-                "--commit-stop",
-              ]
-          `);
+        Array [
+          "--schedule-render-16",
+          "--render-start-16",
+          "--component-render-start-Example",
+          "--schedule-state-update-16-Example",
+          "--component-render-stop",
+          "--render-stop",
+          "--commit-start-16",
+          "--react-version-17.0.3",
+          "--profiler-version-1",
+          "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
+          "--layout-effects-start-16",
+          "--layout-effects-stop",
+          "--commit-stop",
+          "--passive-effects-start-16",
+          "--passive-effects-stop",
+        ]
+      `);
     }
   });
 
@@ -742,35 +766,39 @@ describe('SchedulingProfiler', () => {
 
     if (gate(flags => flags.enableSchedulingProfiler)) {
       expect(getMarks()).toMatchInlineSnapshot(`
-              Array [
-                "--schedule-render-1",
-                "--render-start-1",
-                "--component-render-start-ErrorBoundary",
-                "--component-render-stop",
-                "--component-render-start-ExampleThatThrows",
-                "--component-render-start-ExampleThatThrows",
-                "--component-render-stop",
-                "--error-ExampleThatThrows-mount-Expected error",
-                "--render-stop",
-                "--commit-start-1",
-                "--react-version-17.0.3",
-                "--profiler-version-1",
-                "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
-                "--layout-effects-start-1",
-                "--schedule-state-update-1-ErrorBoundary",
-                "--layout-effects-stop",
-                "--commit-stop",
-                "--render-start-1",
-                "--component-render-start-ErrorBoundary",
-                "--component-render-stop",
-                "--render-stop",
-                "--commit-start-1",
-                "--react-version-17.0.3",
-                "--profiler-version-1",
-                "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
-                "--commit-stop",
-              ]
-          `);
+        Array [
+          "--schedule-render-1",
+          "--render-start-1",
+          "--component-render-start-ErrorBoundary",
+          "--component-render-stop",
+          "--component-render-start-ExampleThatThrows",
+          "--component-render-start-ExampleThatThrows",
+          "--component-render-stop",
+          "--error-ExampleThatThrows-mount-Expected error",
+          "--render-stop",
+          "--commit-start-1",
+          "--react-version-17.0.3",
+          "--profiler-version-1",
+          "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
+          "--layout-effects-start-1",
+          "--schedule-state-update-1-ErrorBoundary",
+          "--layout-effects-stop",
+          "--commit-stop",
+          "--passive-effects-start-1",
+          "--passive-effects-stop",
+          "--render-start-1",
+          "--component-render-start-ErrorBoundary",
+          "--component-render-stop",
+          "--render-stop",
+          "--commit-start-1",
+          "--react-version-17.0.3",
+          "--profiler-version-1",
+          "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
+          "--layout-effects-start-1",
+          "--layout-effects-stop",
+          "--commit-stop",
+        ]
+      `);
     }
   });
 
@@ -840,6 +868,8 @@ describe('SchedulingProfiler', () => {
           "--layout-effects-start-16",
           "--schedule-state-update-1-ErrorBoundary",
           "--layout-effects-stop",
+          "--passive-effects-start-16",
+          "--passive-effects-stop",
           "--render-start-1",
           "--component-render-start-ErrorBoundary",
           "--component-render-stop",
@@ -848,6 +878,10 @@ describe('SchedulingProfiler', () => {
           "--react-version-17.0.3",
           "--profiler-version-1",
           "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
+          "--layout-effects-start-1",
+          "--layout-effects-stop",
+          "--passive-effects-start-1",
+          "--passive-effects-stop",
           "--commit-stop",
           "--commit-stop",
         ]
diff --git a/packages/react-reconciler/src/__tests__/SchedulingProfilerLabels-test.internal.js b/packages/react-reconciler/src/__tests__/SchedulingProfilerLabels-test.internal.js
index 09fdb95b01e..28e0c162512 100644
--- a/packages/react-reconciler/src/__tests__/SchedulingProfilerLabels-test.internal.js
+++ b/packages/react-reconciler/src/__tests__/SchedulingProfilerLabels-test.internal.js
@@ -87,20 +87,20 @@ describe('SchedulingProfiler labels', () => {
 
     if (gate(flags => flags.enableSchedulingProfiler)) {
       expect(clearedMarks).toMatchInlineSnapshot(`
-      Array [
-        "__v3",
-        "--schedule-render-1",
-        "--render-start-1",
-        "--render-stop",
-        "--commit-start-1",
-        "--react-version-17.0.3",
-        "--profiler-version-1",
-        "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
-        "--layout-effects-start-1",
-        "--layout-effects-stop",
-        "--commit-stop",
-      ]
-    `);
+              Array [
+                "__v3",
+                "--schedule-render-1",
+                "--render-start-1",
+                "--render-stop",
+                "--commit-start-1",
+                "--react-version-17.0.3",
+                "--profiler-version-1",
+                "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
+                "--layout-effects-start-1",
+                "--layout-effects-stop",
+                "--commit-stop",
+              ]
+          `);
     }
   });
 
@@ -112,11 +112,11 @@ describe('SchedulingProfiler labels', () => {
 
         root.render(<div />);
         expect(clearedMarks).toMatchInlineSnapshot(`
-        Array [
-          "__v3",
-          "--schedule-render-16",
-        ]
-      `);
+                  Array [
+                    "__v3",
+                    "--schedule-render-16",
+                  ]
+              `);
       });
     }
   });
@@ -150,21 +150,21 @@ describe('SchedulingProfiler labels', () => {
       });
 
       expect(clearedMarks).toMatchInlineSnapshot(`
-      Array [
-        "--schedule-state-update-1-App",
-        "--render-start-1",
-        "--component-render-start-App",
-        "--component-render-stop",
-        "--render-stop",
-        "--commit-start-1",
-        "--react-version-17.0.3",
-        "--profiler-version-1",
-        "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
-        "--layout-effects-start-1",
-        "--layout-effects-stop",
-        "--commit-stop",
-      ]
-    `);
+              Array [
+                "--schedule-state-update-1-App",
+                "--render-start-1",
+                "--component-render-start-App",
+                "--component-render-stop",
+                "--render-stop",
+                "--commit-start-1",
+                "--react-version-17.0.3",
+                "--profiler-version-1",
+                "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
+                "--layout-effects-start-1",
+                "--layout-effects-stop",
+                "--commit-stop",
+              ]
+          `);
     }
   });
 
@@ -194,21 +194,23 @@ describe('SchedulingProfiler labels', () => {
       });
 
       expect(clearedMarks).toMatchInlineSnapshot(`
-      Array [
-        "--schedule-state-update-4-App",
-        "--render-start-4",
-        "--component-render-start-App",
-        "--component-render-stop",
-        "--render-stop",
-        "--commit-start-4",
-        "--react-version-17.0.3",
-        "--profiler-version-1",
-        "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
-        "--layout-effects-start-4",
-        "--layout-effects-stop",
-        "--commit-stop",
-      ]
-    `);
+        Array [
+          "--schedule-state-update-4-App",
+          "--render-start-4",
+          "--component-render-start-App",
+          "--component-render-stop",
+          "--render-stop",
+          "--commit-start-4",
+          "--react-version-17.0.3",
+          "--profiler-version-1",
+          "--react-lane-labels-Sync,InputContinuousHydration,InputContinuous,DefaultHydration,Default,TransitionHydration,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Transition,Retry,Retry,Retry,Retry,Retry,SelectiveHydration,IdleHydration,Idle,Offscreen",
+          "--layout-effects-start-4",
+          "--layout-effects-stop",
+          "--commit-stop",
+          "--passive-effects-start-4",
+          "--passive-effects-stop",
+        ]
+      `);
     }
   });
 });

From ac7de0500cd4dda7b8b35707758f2bf24190ff13 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Wed, 6 Oct 2021 17:30:49 -0700
Subject: [PATCH 15/25] put back accidentally removed logic, handle suspense,
 more tests

---
 .../src/ReactFiberCacheComponent.new.js       |  26 +-
 .../src/ReactFiberCommitWork.new.js           |  56 +++-
 .../src/ReactFiberCompleteWork.new.js         |  10 +
 .../src/ReactFiberRoot.new.js                 |   3 +-
 .../src/ReactFiberWorkLoop.new.js             |  12 +-
 .../src/__tests__/ReactCache-test.js          | 313 +++++++++++++++++-
 6 files changed, 374 insertions(+), 46 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
index b2d72f7e955..9472c49e96f 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
@@ -115,15 +115,15 @@ export function releaseCache(cache: Cache) {
   }
 }
 
-function trace(msg, levels = 2) {
-  if (__DEV__) {
-    // console.log(
-    //   `${msg}:\n` +
-    //     new Error().stack
-    //       .split('\n')
-    //       .slice(3, 3 + Math.max(levels, 1))
-    //       .join('\n'),
-    // );
+export function trace(msg: string, levels: number = 2) {
+  if (__DEV__ && false) {
+    console.log(
+      `${msg}:\n` +
+        new Error().stack
+          .split('\n')
+          .slice(3, 3 + Math.max(levels, 0))
+          .join('\n'),
+    );
   }
 }
 
@@ -202,6 +202,7 @@ export function restoreSpawnedCachePool(
   if (nextParentCache !== prevCachePool.parent) {
     // There was a refresh. Don't bother restoring anything since the refresh
     // will override it.
+    trace('restoreSpawnedCachePool refresh');
     return null;
   } else {
     // No refresh. Resume with the previous cache. This will override the cache
@@ -212,6 +213,7 @@ export function restoreSpawnedCachePool(
 
     // 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.
+    trace('restoreSpawnedCachePool hasCache=' + (pooledCache != null));
     return prevCachePool;
   }
 }
@@ -226,6 +228,7 @@ export function popCachePool(workInProgress: Fiber) {
   if (!enableCache) {
     return;
   }
+  trace('popCachePool');
   _suspendedPooledCache = pooledCache;
   pooledCache = prevFreshCacheOnStack.current;
   pop(prevFreshCacheOnStack, workInProgress);
@@ -235,6 +238,10 @@ export function getSuspendedCachePool(): SpawnedCachePool | null {
   if (!enableCache) {
     return null;
   }
+  trace(
+    'getSuspendedCachePool hasCache=' +
+      ((pooledCache ?? _suspendedPooledCache) != null),
+  );
 
   // We check the cache on the stack first, since that's the one any new Caches
   // would have accessed.
@@ -271,6 +278,7 @@ export function getOffscreenDeferredCachePool(): SpawnedCachePool | null {
     // There's no deferred cache pool.
     return null;
   }
+  trace('getOffscreenDeferredCachePool');
 
   return {
     // We must also store the parent, so that when we resume we can detect
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
index af60c6c436d..8f3a57880bf 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
@@ -26,6 +26,7 @@ import type {OffscreenState} from './ReactFiberOffscreenComponent';
 import type {HookFlags} from './ReactHookEffectTags';
 import type {Cache} from './ReactFiberCacheComponent.new';
 
+import {trace} from './ReactFiberCacheComponent.new';
 import {
   enableCreateEventHandleAPI,
   enableProfilerTimer,
@@ -2623,34 +2624,63 @@ function commitPassiveMountOnFiber(
     }
     case HostRoot: {
       if (enableCache) {
-        // todo compare caches here
-        // if caches not same
-        // - retain next cache
-        // - if prev cache non-null: release (or move to unmount phase?)
-        // add comment that retain/release on child is technically not required
         const previousCache: ?Cache =
           finishedWork.alternate?.memoizedState.cache;
         const nextCache: Cache = finishedWork.memoizedState.cache;
-        const nextCacheRetained = (nextCache: any).retained;
+        // Retain/release the root cache.
+        // Note that on initial mount, previousCache and nextCache will be the same,
+        // this retain won't occur. To counter this, we instead retain the HostRoot's
+        // initial cache when creating the root itself (see createFiberRoot() in
+        // ReactFiberRoot.js). Subsequent updates that change the cache are reflected
+        // here, such that previous/next caches are retained correctly.
         if (nextCache !== previousCache) {
           retainCache(nextCache);
           if (previousCache != null) {
             releaseCache(previousCache);
           }
-        } else if (!nextCacheRetained) {
+        }
+      }
+      break;
+    }
+    case SuspenseComponent: {
+      if (enableCache) {
+        const isHidden = finishedWork.memoizedState !== null;
+        if (isHidden) {
+          const offscreen = finishedWork.child;
+          if (offscreen != null && offscreen.tag === OffscreenComponent) {
+            const previousCache: ?Cache =
+              offscreen.alternate?.memoizedState?.cachePool?.pool;
+            const nextCache: Cache = offscreen.memoizedState?.cachePool?.pool;
+            if (nextCache !== previousCache) {
+              if (nextCache != null) {
+                retainCache(nextCache);
+              }
+              if (previousCache != null) {
+                releaseCache(previousCache);
+              }
+            }
+          }
+        }
+      }
+      break;
+    }
+    case LegacyHiddenComponent:
+    case OffscreenComponent: {
+      const previousCache: ?Cache =
+        finishedWork.alternate?.memoizedState?.cachePool?.pool;
+      const nextCache: Cache = finishedWork.memoizedState?.cachePool?.pool;
+      if (nextCache !== previousCache) {
+        if (nextCache != null) {
           retainCache(nextCache);
-          (nextCache: any).retained = true;
+        }
+        if (previousCache != null) {
+          releaseCache(previousCache);
         }
       }
       break;
     }
     case CacheComponent: {
       if (enableCache) {
-        // todo compare caches here
-        // if caches not same
-        // - retain next cache
-        // - if prev cache non-null: release (or move to unmount phase?)
-        // add comment that retain/release on child is technically not required
         const previousCache: ?Cache =
           finishedWork.alternate?.memoizedState.cache;
         const nextCache: Cache = finishedWork.memoizedState.cache;
diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js
index 99af01ff38e..20601b4cdd2 100644
--- a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js
@@ -1005,6 +1005,13 @@ function completeWork(
       popSuspenseContext(workInProgress);
       const nextState: null | SuspenseState = workInProgress.memoizedState;
 
+      if (enableCache) {
+        // Suspense components may need to retain/release the cache stored on their child
+        // Offscreen component during the passive effects stage (running passive effects
+        // on behalf of that offscreen component).
+        workInProgress.flags |= Passive;
+      }
+
       if (enableSuspenseServerRenderer) {
         if (nextState !== null && nextState.dehydrated !== null) {
           if (current === null) {
@@ -1463,6 +1470,8 @@ function completeWork(
       }
 
       if (enableCache) {
+        // Run passive effects to retain/release the cache.
+        workInProgress.flags |= Passive;
         const spawnedCachePool: SpawnedCachePool | null = (workInProgress.updateQueue: any);
         if (spawnedCachePool !== null) {
           popCachePool(workInProgress);
@@ -1474,6 +1483,7 @@ function completeWork(
     case CacheComponent: {
       if (enableCache) {
         const cache: Cache = workInProgress.memoizedState.cache;
+        // Run passive effects to retain/release the cache.
         workInProgress.flags |= Passive;
         popCacheProvider(workInProgress, cache);
         bubbleProperties(workInProgress);
diff --git a/packages/react-reconciler/src/ReactFiberRoot.new.js b/packages/react-reconciler/src/ReactFiberRoot.new.js
index 75a621b610f..73a83b47415 100644
--- a/packages/react-reconciler/src/ReactFiberRoot.new.js
+++ b/packages/react-reconciler/src/ReactFiberRoot.new.js
@@ -28,7 +28,7 @@ import {
 } from 'shared/ReactFeatureFlags';
 import {initializeUpdateQueue} from './ReactUpdateQueue.new';
 import {LegacyRoot, ConcurrentRoot} from './ReactRootTags';
-import {createCache} from './ReactFiberCacheComponent.new';
+import {createCache, retainCache} from './ReactFiberCacheComponent.new';
 
 function FiberRootNode(containerInfo, tag, hydrate) {
   this.tag = tag;
@@ -119,6 +119,7 @@ export function createFiberRoot(
 
   if (enableCache) {
     const initialCache = createCache();
+    retainCache(initialCache);
     root.pooledCache = initialCache;
     const initialState = {
       element: null,
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
index cdd1628fcc1..3c051fbfeca 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -2052,7 +2052,7 @@ function commitRootImpl(root, renderPriorityLevel) {
     hasUncaughtError = false;
     const error = firstUncaughtError;
     firstUncaughtError = null;
-    releaseRootPooledCache(root);
+    releaseRootPooledCache(root, remainingLanes);
     throw error;
   }
 
@@ -2095,7 +2095,7 @@ function commitRootImpl(root, renderPriorityLevel) {
 
   // Now that effects have run - giving Cache boundaries a chance to retain the
   // cache instance - release the root's cache in case since the render is complete
-  releaseRootPooledCache(root);
+  releaseRootPooledCache(root, remainingLanes);
 
   if (__DEV__) {
     if (enableDebugTracing) {
@@ -2110,10 +2110,12 @@ function commitRootImpl(root, renderPriorityLevel) {
   return null;
 }
 
-function releaseRootPooledCache(root) {
+function releaseRootPooledCache(root: FiberRoot, remainingLanes: Lanes) {
   if (enableCache) {
-    const pooledCache = root.pooledCache;
-    if (pooledCache != null) {
+    const pooledCacheLanes = (root.pooledCacheLanes &= remainingLanes);
+    if (pooledCacheLanes === NoLanes) {
+      // None of the remaining work relies on the cache pool. Clear it so
+      // subsequent requests get a new cache
       root.pooledCache = null;
     }
   }
diff --git a/packages/react-reconciler/src/__tests__/ReactCache-test.js b/packages/react-reconciler/src/__tests__/ReactCache-test.js
index e3e4ed1cd97..db95b01cb68 100644
--- a/packages/react-reconciler/src/__tests__/ReactCache-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactCache-test.js
@@ -260,7 +260,7 @@ describe('ReactCache', () => {
     expect(Scheduler).toHaveYielded([]);
     expect(root).toMatchRenderedOutput('(empty)');
 
-    await act(async => {
+    await act(async () => {
       root.render(<App showMore={true} />);
     });
     // Even though there are two new <Cache /> trees, they should share the same
@@ -426,7 +426,7 @@ describe('ReactCache', () => {
     // Replace all the children: this should retain the root Cache instance,
     // but cleanup the separate cache instance created for the fresh cache
     // boundary
-    await act(async => {
+    await act(async () => {
       root.render('Bye!');
     });
     // Cleanup occurs for the *second* cache instance: the first is still
@@ -760,8 +760,8 @@ describe('ReactCache', () => {
       'Loading...',
 
       // cleanup occurs immediately bc the root commits w a new cache
-      'Cache cleanup: A [v1]',
-      'Cache cleanup: A [v1] (cached)',
+      // 'Cache cleanup: A [v1]',
+      // 'Cache cleanup: A [v1] (cached)',
     ]);
     expect(root).toMatchRenderedOutput('Loading...');
 
@@ -769,7 +769,11 @@ describe('ReactCache', () => {
       resolveMostRecentTextCache('A');
     });
     // Note that the version has updated, and the previous cache is cleared
-    expect(Scheduler).toHaveYielded(['A [v2]']);
+    expect(Scheduler).toHaveYielded([
+      'A [v2]',
+      'Cache cleanup: A [v1]',
+      'Cache cleanup: A [v1] (cached)',
+    ]);
     expect(root).toMatchRenderedOutput('A [v2]');
 
     await act(async () => {
@@ -905,7 +909,7 @@ describe('ReactCache', () => {
     ]);
     expect(root).toMatchRenderedOutput('A [v3]A [v3]');
 
-    await act(async => {
+    await act(async () => {
       root.render('Bye!');
     });
     // Unmounting children releases the refreshed cache instance only; the root
@@ -996,7 +1000,7 @@ describe('ReactCache', () => {
       // a fresh boundary). Therefore this causes cleanup for both the fresh cache
       // instance in the refreshed first boundary and cleanup for the non-refreshed
       // sibling boundary.
-      await act(async => {
+      await act(async () => {
         root.render('Bye!');
       });
       expect(Scheduler).toHaveYielded([
@@ -1073,7 +1077,7 @@ describe('ReactCache', () => {
       expect(Scheduler).toHaveYielded(['A [v2]']);
       expect(root).toMatchRenderedOutput('A [v2] A [v1] B [v1]');
 
-      await act(async => {
+      await act(async () => {
         root.render('Bye!');
       });
       // Unmounting children releases both cache boundaries, but the original
@@ -1169,7 +1173,7 @@ describe('ReactCache', () => {
     expect(Scheduler).toHaveYielded(['A [v1]', 'A [v1]', 'A [v2]']);
     expect(root).toMatchRenderedOutput('A [v1]A [v1]A [v2]');
 
-    await act(async => {
+    await act(async () => {
       root.render('Bye!');
     });
     expect(Scheduler).toHaveYielded([
@@ -1208,7 +1212,7 @@ describe('ReactCache', () => {
           <Suspense fallback={<Text text="Loading..." />}>
             {shouldShow ? (
               <Cache>
-                <AsyncText text="A" />
+                <AsyncText showVersion={true} text="A" />
               </Cache>
             ) : null}
           </Suspense>
@@ -1243,7 +1247,6 @@ describe('ReactCache', () => {
     });
     expect(Scheduler).toHaveYielded([
       '1',
-      'Cache miss! [A]',
 
       // Happens to re-render the fallback. Doesn't need to, but not relevant
       // to this test.
@@ -1254,17 +1257,291 @@ describe('ReactCache', () => {
     await act(async () => {
       resolveMostRecentTextCache('A');
     });
-    expect(Scheduler).toHaveYielded(['A']);
-    expect(root).toMatchRenderedOutput('A1');
+    expect(Scheduler).toHaveYielded(['A [v1]']);
+    expect(root).toMatchRenderedOutput('A [v1]1');
 
-    await act(async => {
+    await act(async () => {
       root.render('Bye!');
     });
-    // the cache boundary's fresh cache is freed, but the original root cache
-    // is still retained by the root
+    // The root's cache is never accessed, so the v1 cache is for the fresh Cache boundary
+    // added during the transition. This is freed when children are unmounted
     expect(Scheduler).toHaveYielded([
-      'Cache cleanup: A [v2]',
-      'Cache cleanup: A [v2] (cached)',
+      'Cache cleanup: A [v1]',
+      'Cache cleanup: A [v1] (cached)',
+      'Cache cleanup: A [v1] (cached)',
+    ]);
+    expect(root).toMatchRenderedOutput('Bye!');
+  });
+
+  // @gate experimental || www
+  test('cache boundary uses a fresh cache when its key changes', async () => {
+    const root = ReactNoop.createRoot();
+    seedNextTextCache('A');
+    await act(async () => {
+      root.render(
+        <Suspense fallback="Loading...">
+          <Cache key="A">
+            <AsyncText showVersion={true} text="A" />
+          </Cache>
+        </Suspense>,
+      );
+    });
+    expect(Scheduler).toHaveYielded(['A [v1]']);
+    expect(root).toMatchRenderedOutput('A [v1]');
+
+    seedNextTextCache('B');
+    await act(async () => {
+      root.render(
+        <Suspense fallback="Loading...">
+          <Cache key="B">
+            <AsyncText showVersion={true} text="B" />
+          </Cache>
+        </Suspense>,
+      );
+    });
+    // the v1 cache (with A) does not cleanup bc its still retained by the root
+    expect(Scheduler).toHaveYielded(['B [v2]']);
+    expect(root).toMatchRenderedOutput('B [v2]');
+
+    await act(async () => {
+      root.render('Bye!');
+    });
+    expect(Scheduler).toHaveYielded(['Cache cleanup: B [v2] (cached)']);
+    expect(root).toMatchRenderedOutput('Bye!');
+  });
+
+  // @gate experimental || www
+  test('overlapping transitions share the same cache', async () => {
+    const root = ReactNoop.createRoot();
+    await act(async () => {
+      root.render(
+        <Suspense fallback="Loading...">
+          <Cache key="A">
+            <AsyncText showVersion={true} text="A" />
+          </Cache>
+        </Suspense>,
+      );
+    });
+    expect(Scheduler).toHaveYielded(['Cache miss! [A]']);
+    expect(root).toMatchRenderedOutput('Loading...');
+
+    await act(async () => {
+      startTransition(() => {
+        root.render(
+          <Suspense fallback="Loading...">
+            <Cache key="B">
+              <AsyncText showVersion={true} text="B" />
+            </Cache>
+          </Suspense>,
+        );
+      });
+    });
+    expect(Scheduler).toHaveYielded(['Cache miss! [B]']);
+    expect(root).toMatchRenderedOutput('Loading...');
+
+    await act(async () => {
+      startTransition(() => {
+        root.render(
+          <Suspense fallback="Loading...">
+            <Cache key="C">
+              <AsyncText showVersion={true} text="C" />
+            </Cache>
+          </Suspense>,
+        );
+      });
+    });
+    expect(Scheduler).toHaveYielded(['Cache miss! [C]']);
+    expect(root).toMatchRenderedOutput('Loading...');
+
+    await act(async () => {
+      resolveMostRecentTextCache('C');
+    });
+    expect(Scheduler).toHaveYielded(['C [v1]']);
+    expect(root).toMatchRenderedOutput('C [v1]');
+
+    await act(async () => {
+      root.render('Bye!');
+    });
+    // The root's cache is never accessed, so the v1 cache is for the fresh Cache boundary
+    // added during the transition. This is freed when children are unmounted
+    expect(Scheduler).toHaveYielded([]);
+    expect(root).toMatchRenderedOutput('Bye!');
+  });
+
+  // @gate experimental || www
+  test('overlapping transitions after an initial mount use the same fresh cache', async () => {
+    const root = ReactNoop.createRoot();
+    await act(async () => {
+      root.render(
+        <Suspense fallback="Loading...">
+          <Cache key="A">
+            <AsyncText showVersion={true} text="A" />
+          </Cache>
+        </Suspense>,
+      );
+    });
+    expect(Scheduler).toHaveYielded(['Cache miss! [A]']);
+    expect(root).toMatchRenderedOutput('Loading...');
+
+    await act(async () => {
+      resolveMostRecentTextCache('A');
+    });
+    expect(Scheduler).toHaveYielded(['A [v1]']);
+    expect(root).toMatchRenderedOutput('A [v1]');
+
+    await act(async () => {
+      startTransition(() => {
+        root.render(
+          <Suspense fallback="Loading...">
+            <Cache key="B">
+              <AsyncText showVersion={true} text="B" />
+            </Cache>
+          </Suspense>,
+        );
+      });
+    });
+    expect(Scheduler).toHaveYielded(['Cache miss! [B]']);
+    expect(root).toMatchRenderedOutput('A [v1]');
+
+    await act(async () => {
+      startTransition(() => {
+        root.render(
+          <Suspense fallback="Loading...">
+            <Cache key="C">
+              <AsyncText showVersion={true} text="C" />
+            </Cache>
+          </Suspense>,
+        );
+      });
+    });
+    expect(Scheduler).toHaveYielded(['Cache miss! [C]']);
+    expect(root).toMatchRenderedOutput('A [v1]');
+
+    await act(async () => {
+      resolveMostRecentTextCache('C');
+    });
+    expect(Scheduler).toHaveYielded(['C [v2]']);
+    expect(root).toMatchRenderedOutput('C [v2]');
+
+    await act(async () => {
+      startTransition(() => {
+        root.render(
+          <Suspense fallback="Loading...">
+            <Cache key="D">
+              <AsyncText showVersion={true} text="D" />
+            </Cache>
+          </Suspense>,
+        );
+      });
+    });
+    expect(Scheduler).toHaveYielded(['Cache miss! [D]']);
+    expect(root).toMatchRenderedOutput('C [v2]');
+
+    await act(async () => {
+      resolveMostRecentTextCache('D');
+    });
+    expect(Scheduler).toHaveYielded([
+      'D [v3]',
+      'Cache cleanup: B [v2]',
+      'Cache cleanup: C [v2]',
+      'Cache cleanup: C [v2] (cached)',
+    ]);
+    expect(root).toMatchRenderedOutput('D [v3]');
+
+    await act(async () => {
+      root.render('Bye!');
+    });
+    // The root's cache is never accessed, so the v1 cache is for the fresh Cache boundary
+    // added during the transition. This is freed when children are unmounted
+    expect(Scheduler).toHaveYielded([
+      'Cache cleanup: D [v3]',
+      'Cache cleanup: D [v3] (cached)',
+    ]);
+    expect(root).toMatchRenderedOutput('Bye!');
+  });
+
+  // @gate experimental || www
+  test('overlapping updates after an initial mount use the same fresh cache', async () => {
+    const root = ReactNoop.createRoot();
+    await act(async () => {
+      root.render(
+        <Suspense fallback="Loading...">
+          <Cache key="A">
+            <AsyncText showVersion={true} text="A" />
+          </Cache>
+        </Suspense>,
+      );
+    });
+    expect(Scheduler).toHaveYielded(['Cache miss! [A]']);
+    expect(root).toMatchRenderedOutput('Loading...');
+
+    await act(async () => {
+      resolveMostRecentTextCache('A');
+    });
+    expect(Scheduler).toHaveYielded(['A [v1]']);
+    expect(root).toMatchRenderedOutput('A [v1]');
+
+    await act(async () => {
+      root.render(
+        <Suspense fallback="Loading...">
+          <Cache key="B">
+            <AsyncText showVersion={true} text="B" />
+          </Cache>
+        </Suspense>,
+      );
+    });
+    expect(Scheduler).toHaveYielded(['Cache miss! [B]']);
+    expect(root).toMatchRenderedOutput('Loading...');
+
+    await act(async () => {
+      root.render(
+        <Suspense fallback="Loading...">
+          <Cache key="C">
+            <AsyncText showVersion={true} text="C" />
+          </Cache>
+        </Suspense>,
+      );
+    });
+    expect(Scheduler).toHaveYielded(['Cache miss! [C]']);
+    expect(root).toMatchRenderedOutput('Loading...');
+
+    await act(async () => {
+      resolveMostRecentTextCache('C');
+    });
+    expect(Scheduler).toHaveYielded(['C [v2]']);
+    expect(root).toMatchRenderedOutput('C [v2]');
+
+    await act(async () => {
+      root.render(
+        <Suspense fallback="Loading...">
+          <Cache key="D">
+            <AsyncText showVersion={true} text="D" />
+          </Cache>
+        </Suspense>,
+      );
+    });
+    expect(Scheduler).toHaveYielded(['Cache miss! [D]']);
+    expect(root).toMatchRenderedOutput('Loading...');
+
+    await act(async () => {
+      resolveMostRecentTextCache('D');
+    });
+    expect(Scheduler).toHaveYielded([
+      'D [v3]',
+      'Cache cleanup: B [v2]',
+      'Cache cleanup: C [v2]',
+      'Cache cleanup: C [v2] (cached)',
+    ]);
+    expect(root).toMatchRenderedOutput('D [v3]');
+
+    await act(async () => {
+      root.render('Bye!');
+    });
+    // The root's cache is never accessed, so the v1 cache is for the fresh Cache boundary
+    // added during the transition. This is freed when children are unmounted
+    expect(Scheduler).toHaveYielded([
+      'Cache cleanup: D [v3]',
+      'Cache cleanup: D [v3] (cached)',
     ]);
     expect(root).toMatchRenderedOutput('Bye!');
   });

From d4b62c2282ff32d2b28cee904f74798f37131f2c Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Thu, 7 Oct 2021 09:14:56 -0700
Subject: [PATCH 16/25] more cleanup, sync forks

---
 .../src/ReactFiberCacheComponent.new.js       | 16 ++--
 .../src/ReactFiberCacheComponent.old.js       | 12 ++-
 .../src/ReactFiberCommitWork.new.js           | 19 ++++-
 .../src/ReactFiberCommitWork.old.js           | 73 +++++++++++++++----
 .../src/ReactFiberCompleteWork.old.js         | 10 +++
 .../src/ReactFiberRoot.old.js                 |  3 +-
 .../src/ReactFiberWorkLoop.old.js             | 12 +--
 7 files changed, 115 insertions(+), 30 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
index 9472c49e96f..5cf5d6b0dad 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
@@ -116,14 +116,14 @@ export function releaseCache(cache: Cache) {
 }
 
 export function trace(msg: string, levels: number = 2) {
-  if (__DEV__ && false) {
-    console.log(
-      `${msg}:\n` +
-        new Error().stack
-          .split('\n')
-          .slice(3, 3 + Math.max(levels, 0))
-          .join('\n'),
-    );
+  if (__DEV__) {
+    // console.log(
+    //   `${msg}:\n` +
+    //     new Error().stack
+    //       .split('\n')
+    //       .slice(3, 3 + Math.max(levels, 0))
+    //       .join('\n'),
+    // );
   }
 }
 
diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.old.js b/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
index 932c02eb859..60fb6c0b5e4 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
@@ -115,13 +115,13 @@ export function releaseCache(cache: Cache) {
   }
 }
 
-function trace(msg, levels = 2) {
+export function trace(msg: string, levels: number = 2) {
   if (__DEV__) {
     // console.log(
     //   `${msg}:\n` +
     //     new Error().stack
     //       .split('\n')
-    //       .slice(3, 3 + Math.max(levels, 1))
+    //       .slice(3, 3 + Math.max(levels, 0))
     //       .join('\n'),
     // );
   }
@@ -202,6 +202,7 @@ export function restoreSpawnedCachePool(
   if (nextParentCache !== prevCachePool.parent) {
     // There was a refresh. Don't bother restoring anything since the refresh
     // will override it.
+    trace('restoreSpawnedCachePool refresh');
     return null;
   } else {
     // No refresh. Resume with the previous cache. This will override the cache
@@ -212,6 +213,7 @@ export function restoreSpawnedCachePool(
 
     // 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.
+    trace('restoreSpawnedCachePool hasCache=' + (pooledCache != null));
     return prevCachePool;
   }
 }
@@ -226,6 +228,7 @@ export function popCachePool(workInProgress: Fiber) {
   if (!enableCache) {
     return;
   }
+  trace('popCachePool');
   _suspendedPooledCache = pooledCache;
   pooledCache = prevFreshCacheOnStack.current;
   pop(prevFreshCacheOnStack, workInProgress);
@@ -235,6 +238,10 @@ export function getSuspendedCachePool(): SpawnedCachePool | null {
   if (!enableCache) {
     return null;
   }
+  trace(
+    'getSuspendedCachePool hasCache=' +
+      ((pooledCache ?? _suspendedPooledCache) != null),
+  );
 
   // We check the cache on the stack first, since that's the one any new Caches
   // would have accessed.
@@ -271,6 +278,7 @@ export function getOffscreenDeferredCachePool(): SpawnedCachePool | null {
     // There's no deferred cache pool.
     return null;
   }
+  trace('getOffscreenDeferredCachePool');
 
   return {
     // We must also store the parent, so that when we resume we can detect
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
index 8f3a57880bf..edad51f7ecf 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
@@ -26,7 +26,6 @@ import type {OffscreenState} from './ReactFiberOffscreenComponent';
 import type {HookFlags} from './ReactHookEffectTags';
 import type {Cache} from './ReactFiberCacheComponent.new';
 
-import {trace} from './ReactFiberCacheComponent.new';
 import {
   enableCreateEventHandleAPI,
   enableProfilerTimer,
@@ -2645,6 +2644,15 @@ function commitPassiveMountOnFiber(
     case SuspenseComponent: {
       if (enableCache) {
         const isHidden = finishedWork.memoizedState !== null;
+        // Retain/release the cache used for pending (suspended) children.
+        // When a tree is suspended we don't want to throw away the fresh
+        // cache instance being used for any new cache boundaries within it.
+        // This cache instance is stored on the OffscreenComponent that is
+        // a child of a Suspense component. However, passive effects don't
+        // run for children of a hidden suspense component. So for hidden
+        // suspense components we retain/release the next/previous cache
+        // on behalf of the hidden offscreen child here. For the non-hidden
+        // case, the component is handled normally (see below).
         if (isHidden) {
           const offscreen = finishedWork.child;
           if (offscreen != null && offscreen.tag === OffscreenComponent) {
@@ -2669,6 +2677,10 @@ function commitPassiveMountOnFiber(
       const previousCache: ?Cache =
         finishedWork.alternate?.memoizedState?.cachePool?.pool;
       const nextCache: Cache = finishedWork.memoizedState?.cachePool?.pool;
+      // Retain/release the cache used for pending (suspended) nodes.
+      // Note that this is only reached in the non-suspended/visible case:
+      // when the content is suspended/hidden, the retain/release occurs
+      // via the parent Suspense component (see case above).
       if (nextCache !== previousCache) {
         if (nextCache != null) {
           retainCache(nextCache);
@@ -2684,6 +2696,11 @@ function commitPassiveMountOnFiber(
         const previousCache: ?Cache =
           finishedWork.alternate?.memoizedState.cache;
         const nextCache: Cache = finishedWork.memoizedState.cache;
+        // Retain/release the cache. In theory the cache component
+        // could be "borrowing" a cache instance owned by some parent,
+        // in which case we could avoid retaining/releasing. But it
+        // is non-trivial to determine when that is the case, so we
+        // always retain/release.
         if (nextCache !== previousCache) {
           retainCache(nextCache);
           if (previousCache != null) {
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js
index 03588a8bb8f..95486896b5b 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js
@@ -2623,37 +2623,84 @@ function commitPassiveMountOnFiber(
     }
     case HostRoot: {
       if (enableCache) {
-        // todo compare caches here
-        // if caches not same
-        // - retain next cache
-        // - if prev cache non-null: release (or move to unmount phase?)
-        // add comment that retain/release on child is technically not required
         const previousCache: ?Cache =
           finishedWork.alternate?.memoizedState.cache;
         const nextCache: Cache = finishedWork.memoizedState.cache;
-        const nextCacheRetained = (nextCache: any).retained;
+        // Retain/release the root cache.
+        // Note that on initial mount, previousCache and nextCache will be the same,
+        // this retain won't occur. To counter this, we instead retain the HostRoot's
+        // initial cache when creating the root itself (see createFiberRoot() in
+        // ReactFiberRoot.js). Subsequent updates that change the cache are reflected
+        // here, such that previous/next caches are retained correctly.
         if (nextCache !== previousCache) {
           retainCache(nextCache);
           if (previousCache != null) {
             releaseCache(previousCache);
           }
-        } else if (!nextCacheRetained) {
+        }
+      }
+      break;
+    }
+    case SuspenseComponent: {
+      if (enableCache) {
+        const isHidden = finishedWork.memoizedState !== null;
+        // Retain/release the cache used for pending (suspended) children.
+        // When a tree is suspended we don't want to throw away the fresh
+        // cache instance being used for any new cache boundaries within it.
+        // This cache instance is stored on the OffscreenComponent that is
+        // a child of a Suspense component. However, passive effects don't
+        // run for children of a hidden suspense component. So for hidden
+        // suspense components we retain/release the next/previous cache
+        // on behalf of the hidden offscreen child here. For the non-hidden
+        // case, the component is handled normally (see below).
+        if (isHidden) {
+          const offscreen = finishedWork.child;
+          if (offscreen != null && offscreen.tag === OffscreenComponent) {
+            const previousCache: ?Cache =
+              offscreen.alternate?.memoizedState?.cachePool?.pool;
+            const nextCache: Cache = offscreen.memoizedState?.cachePool?.pool;
+            if (nextCache !== previousCache) {
+              if (nextCache != null) {
+                retainCache(nextCache);
+              }
+              if (previousCache != null) {
+                releaseCache(previousCache);
+              }
+            }
+          }
+        }
+      }
+      break;
+    }
+    case LegacyHiddenComponent:
+    case OffscreenComponent: {
+      const previousCache: ?Cache =
+        finishedWork.alternate?.memoizedState?.cachePool?.pool;
+      const nextCache: Cache = finishedWork.memoizedState?.cachePool?.pool;
+      // Retain/release the cache used for pending (suspended) nodes.
+      // Note that this is only reached in the non-suspended/visible case:
+      // when the content is suspended/hidden, the retain/release occurs
+      // via the parent Suspense component (see case above).
+      if (nextCache !== previousCache) {
+        if (nextCache != null) {
           retainCache(nextCache);
-          (nextCache: any).retained = true;
+        }
+        if (previousCache != null) {
+          releaseCache(previousCache);
         }
       }
       break;
     }
     case CacheComponent: {
       if (enableCache) {
-        // todo compare caches here
-        // if caches not same
-        // - retain next cache
-        // - if prev cache non-null: release (or move to unmount phase?)
-        // add comment that retain/release on child is technically not required
         const previousCache: ?Cache =
           finishedWork.alternate?.memoizedState.cache;
         const nextCache: Cache = finishedWork.memoizedState.cache;
+        // Retain/release the cache. In theory the cache component
+        // could be "borrowing" a cache instance owned by some parent,
+        // in which case we could avoid retaining/releasing. But it
+        // is non-trivial to determine when that is the case, so we
+        // always retain/release.
         if (nextCache !== previousCache) {
           retainCache(nextCache);
           if (previousCache != null) {
diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.old.js b/packages/react-reconciler/src/ReactFiberCompleteWork.old.js
index e7766864fea..6c40aa47cdc 100644
--- a/packages/react-reconciler/src/ReactFiberCompleteWork.old.js
+++ b/packages/react-reconciler/src/ReactFiberCompleteWork.old.js
@@ -1005,6 +1005,13 @@ function completeWork(
       popSuspenseContext(workInProgress);
       const nextState: null | SuspenseState = workInProgress.memoizedState;
 
+      if (enableCache) {
+        // Suspense components may need to retain/release the cache stored on their child
+        // Offscreen component during the passive effects stage (running passive effects
+        // on behalf of that offscreen component).
+        workInProgress.flags |= Passive;
+      }
+
       if (enableSuspenseServerRenderer) {
         if (nextState !== null && nextState.dehydrated !== null) {
           if (current === null) {
@@ -1463,6 +1470,8 @@ function completeWork(
       }
 
       if (enableCache) {
+        // Run passive effects to retain/release the cache.
+        workInProgress.flags |= Passive;
         const spawnedCachePool: SpawnedCachePool | null = (workInProgress.updateQueue: any);
         if (spawnedCachePool !== null) {
           popCachePool(workInProgress);
@@ -1474,6 +1483,7 @@ function completeWork(
     case CacheComponent: {
       if (enableCache) {
         const cache: Cache = workInProgress.memoizedState.cache;
+        // Run passive effects to retain/release the cache.
         workInProgress.flags |= Passive;
         popCacheProvider(workInProgress, cache);
         bubbleProperties(workInProgress);
diff --git a/packages/react-reconciler/src/ReactFiberRoot.old.js b/packages/react-reconciler/src/ReactFiberRoot.old.js
index 982e0f82119..0dec8ef7132 100644
--- a/packages/react-reconciler/src/ReactFiberRoot.old.js
+++ b/packages/react-reconciler/src/ReactFiberRoot.old.js
@@ -28,7 +28,7 @@ import {
 } from 'shared/ReactFeatureFlags';
 import {initializeUpdateQueue} from './ReactUpdateQueue.old';
 import {LegacyRoot, ConcurrentRoot} from './ReactRootTags';
-import {createCache} from './ReactFiberCacheComponent.old';
+import {createCache, retainCache} from './ReactFiberCacheComponent.old';
 
 function FiberRootNode(containerInfo, tag, hydrate) {
   this.tag = tag;
@@ -119,6 +119,7 @@ export function createFiberRoot(
 
   if (enableCache) {
     const initialCache = createCache();
+    retainCache(initialCache);
     root.pooledCache = initialCache;
     const initialState = {
       element: null,
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
index 3f61794a9e0..0c439f43e5a 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
@@ -2052,7 +2052,7 @@ function commitRootImpl(root, renderPriorityLevel) {
     hasUncaughtError = false;
     const error = firstUncaughtError;
     firstUncaughtError = null;
-    releaseRootPooledCache(root);
+    releaseRootPooledCache(root, remainingLanes);
     throw error;
   }
 
@@ -2095,7 +2095,7 @@ function commitRootImpl(root, renderPriorityLevel) {
 
   // Now that effects have run - giving Cache boundaries a chance to retain the
   // cache instance - release the root's cache in case since the render is complete
-  releaseRootPooledCache(root);
+  releaseRootPooledCache(root, remainingLanes);
 
   if (__DEV__) {
     if (enableDebugTracing) {
@@ -2110,10 +2110,12 @@ function commitRootImpl(root, renderPriorityLevel) {
   return null;
 }
 
-function releaseRootPooledCache(root) {
+function releaseRootPooledCache(root: FiberRoot, remainingLanes: Lanes) {
   if (enableCache) {
-    const pooledCache = root.pooledCache;
-    if (pooledCache != null) {
+    const pooledCacheLanes = (root.pooledCacheLanes &= remainingLanes);
+    if (pooledCacheLanes === NoLanes) {
+      // None of the remaining work relies on the cache pool. Clear it so
+      // subsequent requests get a new cache
       root.pooledCache = null;
     }
   }

From f4f60236ca9394dd0e8f47734ae7cf2b962517d6 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Thu, 7 Oct 2021 13:25:11 -0700
Subject: [PATCH 17/25] test cleanup and comments

---
 .../src/__tests__/ReactCache-test.js          | 144 ++++--------------
 1 file changed, 26 insertions(+), 118 deletions(-)

diff --git a/packages/react-reconciler/src/__tests__/ReactCache-test.js b/packages/react-reconciler/src/__tests__/ReactCache-test.js
index db95b01cb68..5cb10b79c99 100644
--- a/packages/react-reconciler/src/__tests__/ReactCache-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactCache-test.js
@@ -167,11 +167,8 @@ describe('ReactCache', () => {
   // @gate experimental || www
   test('render Cache component', async () => {
     const root = ReactNoop.createRoot();
-    function Example(props) {
-      return <Cache>{props.text}</Cache>;
-    }
     await act(async () => {
-      root.render(<Example text="Hi" />);
+      root.render(<Cache>Hi</Cache>);
     });
     expect(root).toMatchRenderedOutput('Hi');
   });
@@ -994,7 +991,7 @@ describe('ReactCache', () => {
       expect(Scheduler).toHaveYielded(['A [v2]']);
       expect(root).toMatchRenderedOutput('A [v2]A [v1]');
 
-      // Replace all the children: this should clear *both* cache instances:
+      // Unmount children: this should clear *both* cache instances:
       // the root doesn't have a cache instance (since it wasn't accessed
       // during the initial render, and all subsequent cache accesses were within
       // a fresh boundary). Therefore this causes cleanup for both the fresh cache
@@ -1173,6 +1170,10 @@ describe('ReactCache', () => {
     expect(Scheduler).toHaveYielded(['A [v1]', 'A [v1]', 'A [v2]']);
     expect(root).toMatchRenderedOutput('A [v1]A [v1]A [v2]');
 
+    // Unmount children: the first text cache instance is created only after the root
+    // commits, so both fresh cache instances are released by their cache boundaries,
+    // cleaning up v1 (used for the first two children which render togeether) and
+    // v2 (used for the third boundary added later).
     await act(async () => {
       root.render('Bye!');
     });
@@ -1260,11 +1261,13 @@ describe('ReactCache', () => {
     expect(Scheduler).toHaveYielded(['A [v1]']);
     expect(root).toMatchRenderedOutput('A [v1]1');
 
+    // Unmount children: the first text cache instance is created only after initial
+    // render after calling showMore(). This instance is cleaned up when that boundary
+    // is unmounted. Bc root cache instance is never accessed, the inner cache
+    // boundary ends up at v1.
     await act(async () => {
       root.render('Bye!');
     });
-    // The root's cache is never accessed, so the v1 cache is for the fresh Cache boundary
-    // added during the transition. This is freed when children are unmounted
     expect(Scheduler).toHaveYielded([
       'Cache cleanup: A [v1]',
       'Cache cleanup: A [v1] (cached)',
@@ -1299,10 +1302,12 @@ describe('ReactCache', () => {
         </Suspense>,
       );
     });
-    // the v1 cache (with A) does not cleanup bc its still retained by the root
     expect(Scheduler).toHaveYielded(['B [v2]']);
     expect(root).toMatchRenderedOutput('B [v2]');
 
+    // Unmount children: the fresh cache instance for B cleans up since the cache boundary
+    // is the only owner, while the original cache instance (for A) is still retained by
+    // the root.
     await act(async () => {
       root.render('Bye!');
     });
@@ -1310,64 +1315,6 @@ describe('ReactCache', () => {
     expect(root).toMatchRenderedOutput('Bye!');
   });
 
-  // @gate experimental || www
-  test('overlapping transitions share the same cache', async () => {
-    const root = ReactNoop.createRoot();
-    await act(async () => {
-      root.render(
-        <Suspense fallback="Loading...">
-          <Cache key="A">
-            <AsyncText showVersion={true} text="A" />
-          </Cache>
-        </Suspense>,
-      );
-    });
-    expect(Scheduler).toHaveYielded(['Cache miss! [A]']);
-    expect(root).toMatchRenderedOutput('Loading...');
-
-    await act(async () => {
-      startTransition(() => {
-        root.render(
-          <Suspense fallback="Loading...">
-            <Cache key="B">
-              <AsyncText showVersion={true} text="B" />
-            </Cache>
-          </Suspense>,
-        );
-      });
-    });
-    expect(Scheduler).toHaveYielded(['Cache miss! [B]']);
-    expect(root).toMatchRenderedOutput('Loading...');
-
-    await act(async () => {
-      startTransition(() => {
-        root.render(
-          <Suspense fallback="Loading...">
-            <Cache key="C">
-              <AsyncText showVersion={true} text="C" />
-            </Cache>
-          </Suspense>,
-        );
-      });
-    });
-    expect(Scheduler).toHaveYielded(['Cache miss! [C]']);
-    expect(root).toMatchRenderedOutput('Loading...');
-
-    await act(async () => {
-      resolveMostRecentTextCache('C');
-    });
-    expect(Scheduler).toHaveYielded(['C [v1]']);
-    expect(root).toMatchRenderedOutput('C [v1]');
-
-    await act(async () => {
-      root.render('Bye!');
-    });
-    // The root's cache is never accessed, so the v1 cache is for the fresh Cache boundary
-    // added during the transition. This is freed when children are unmounted
-    expect(Scheduler).toHaveYielded([]);
-    expect(root).toMatchRenderedOutput('Bye!');
-  });
-
   // @gate experimental || www
   test('overlapping transitions after an initial mount use the same fresh cache', async () => {
     const root = ReactNoop.createRoot();
@@ -1389,6 +1336,7 @@ describe('ReactCache', () => {
     expect(Scheduler).toHaveYielded(['A [v1]']);
     expect(root).toMatchRenderedOutput('A [v1]');
 
+    // After a mount, subsequent transitions use a fresh cache
     await act(async () => {
       startTransition(() => {
         root.render(
@@ -1403,6 +1351,9 @@ describe('ReactCache', () => {
     expect(Scheduler).toHaveYielded(['Cache miss! [B]']);
     expect(root).toMatchRenderedOutput('A [v1]');
 
+    // Update to a different text and with a different key for the cache
+    // boundary: this should still use the fresh cache instance created
+    // for the earlier transition
     await act(async () => {
       startTransition(() => {
         root.render(
@@ -1423,40 +1374,16 @@ describe('ReactCache', () => {
     expect(Scheduler).toHaveYielded(['C [v2]']);
     expect(root).toMatchRenderedOutput('C [v2]');
 
+    // Unmount children: the fresh cache used for the updates is freed, while the
+    // original cache (with A) is still retained at the root.
     await act(async () => {
-      startTransition(() => {
-        root.render(
-          <Suspense fallback="Loading...">
-            <Cache key="D">
-              <AsyncText showVersion={true} text="D" />
-            </Cache>
-          </Suspense>,
-        );
-      });
-    });
-    expect(Scheduler).toHaveYielded(['Cache miss! [D]']);
-    expect(root).toMatchRenderedOutput('C [v2]');
-
-    await act(async () => {
-      resolveMostRecentTextCache('D');
+      root.render('Bye!');
     });
     expect(Scheduler).toHaveYielded([
-      'D [v3]',
       'Cache cleanup: B [v2]',
       'Cache cleanup: C [v2]',
       'Cache cleanup: C [v2] (cached)',
     ]);
-    expect(root).toMatchRenderedOutput('D [v3]');
-
-    await act(async () => {
-      root.render('Bye!');
-    });
-    // The root's cache is never accessed, so the v1 cache is for the fresh Cache boundary
-    // added during the transition. This is freed when children are unmounted
-    expect(Scheduler).toHaveYielded([
-      'Cache cleanup: D [v3]',
-      'Cache cleanup: D [v3] (cached)',
-    ]);
     expect(root).toMatchRenderedOutput('Bye!');
   });
 
@@ -1481,6 +1408,7 @@ describe('ReactCache', () => {
     expect(Scheduler).toHaveYielded(['A [v1]']);
     expect(root).toMatchRenderedOutput('A [v1]');
 
+    // After a mount, subsequent updates use a fresh cache
     await act(async () => {
       root.render(
         <Suspense fallback="Loading...">
@@ -1493,6 +1421,8 @@ describe('ReactCache', () => {
     expect(Scheduler).toHaveYielded(['Cache miss! [B]']);
     expect(root).toMatchRenderedOutput('Loading...');
 
+    // A second update uses the same fresh cache: even though this is a new
+    // Cache boundary, the render uses the fresh cache from the pending update.
     await act(async () => {
       root.render(
         <Suspense fallback="Loading...">
@@ -1511,38 +1441,16 @@ describe('ReactCache', () => {
     expect(Scheduler).toHaveYielded(['C [v2]']);
     expect(root).toMatchRenderedOutput('C [v2]');
 
+    // Unmount children: the fresh cache used for the updates is freed, while the
+    // original cache (with A) is still retained at the root.
     await act(async () => {
-      root.render(
-        <Suspense fallback="Loading...">
-          <Cache key="D">
-            <AsyncText showVersion={true} text="D" />
-          </Cache>
-        </Suspense>,
-      );
-    });
-    expect(Scheduler).toHaveYielded(['Cache miss! [D]']);
-    expect(root).toMatchRenderedOutput('Loading...');
-
-    await act(async () => {
-      resolveMostRecentTextCache('D');
+      root.render('Bye!');
     });
     expect(Scheduler).toHaveYielded([
-      'D [v3]',
       'Cache cleanup: B [v2]',
       'Cache cleanup: C [v2]',
       'Cache cleanup: C [v2] (cached)',
     ]);
-    expect(root).toMatchRenderedOutput('D [v3]');
-
-    await act(async () => {
-      root.render('Bye!');
-    });
-    // The root's cache is never accessed, so the v1 cache is for the fresh Cache boundary
-    // added during the transition. This is freed when children are unmounted
-    expect(Scheduler).toHaveYielded([
-      'Cache cleanup: D [v3]',
-      'Cache cleanup: D [v3] (cached)',
-    ]);
     expect(root).toMatchRenderedOutput('Bye!');
   });
 });

From 6fd24866e96f5878ad099ff4f1a7286d3b01f82a Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Thu, 7 Oct 2021 13:36:25 -0700
Subject: [PATCH 18/25] more cleanup, schedule the abort

---
 .../src/ReactFiberCacheComponent.new.js       | 66 ++++++-------------
 1 file changed, 20 insertions(+), 46 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
index 5cf5d6b0dad..28d807b8265 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
@@ -18,12 +18,12 @@ import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols';
 import {isPrimaryRenderer} from './ReactFiberHostConfig';
 import {createCursor, push, pop} from './ReactFiberStack.new';
 import {pushProvider, popProvider} from './ReactFiberNewContext.new';
+import * as Scheduler from 'scheduler';
 
 export type Cache = {|
   controller: AbortController,
   data: Map<() => mixed, mixed>,
   refCount: number,
-  key?: string,
 |};
 
 export type CacheComponentState = {|
@@ -36,6 +36,13 @@ export type SpawnedCachePool = {|
   +pool: Cache,
 |};
 
+// Intentionally not named imports because Rollup would
+// use dynamic dispatch for CommonJS interop named imports.
+const {
+  unstable_scheduleCallback: scheduleCallback,
+  unstable_NormalPriority: NormalPriority,
+} = Scheduler;
+
 export const CacheContext: ReactContext<Cache> = enableCache
   ? {
       $$typeof: REACT_CONTEXT_TYPE,
@@ -65,7 +72,6 @@ const prevFreshCacheOnStack: StackCursor<Cache | null> = createCursor(null);
 // Creates a new empty Cache instance with a ref-count of 0. The caller is responsible
 // for retaining the cache once it is in use (retainCache), and releasing the cache
 // once it is no longer needed (releaseCache).
-let _cacheIndex = 0;
 export function createCache(): Cache {
   const cache: Cache = {
     controller: new AbortController(),
@@ -73,57 +79,36 @@ export function createCache(): Cache {
     refCount: 0,
   };
 
-  // TODO: remove debugging code
-  if (__DEV__) {
-    cache.key = '' + _cacheIndex++;
-    trace(`createCache #${cache.key ?? ''}`);
-  }
-
   return cache;
 }
 
 export function retainCache(cache: Cache) {
   if (__DEV__) {
-    trace(
-      `retainCache #${cache.key ?? ''} ${cache.refCount} -> ${cache.refCount +
-        1}`,
-    );
+    if (cache.controller.signal.aborted) {
+      console.warn(
+        'A cache instance was retained after it was already freed. ' +
+          'This likely indicates a bug in React.',
+      );
+    }
   }
   cache.refCount++;
 }
 
 // Cleanup a cache instance, potentially freeing it if there are no more references
 export function releaseCache(cache: Cache) {
-  if (__DEV__) {
-    trace(
-      `releaseCache #${cache.key ?? ''} ${cache.refCount} -> ${cache.refCount -
-        1}`,
-    );
-  }
   cache.refCount--;
   if (__DEV__) {
     if (cache.refCount < 0) {
-      throw new Error(
-        'Error in React: cache reference count should not be negative',
+      console.warn(
+        'A cache instance was released after it was already freed. ' +
+          'This likely indicates a bug in React.',
       );
     }
   }
   if (cache.refCount === 0) {
-    // TODO: considering scheduling this call, and adding error handling for
-    // any event listeners that get triggered.
-    cache.controller.abort();
-  }
-}
-
-export function trace(msg: string, levels: number = 2) {
-  if (__DEV__) {
-    // console.log(
-    //   `${msg}:\n` +
-    //     new Error().stack
-    //       .split('\n')
-    //       .slice(3, 3 + Math.max(levels, 0))
-    //       .join('\n'),
-    // );
+    scheduleCallback(NormalPriority, () => {
+      cache.controller.abort();
+    });
   }
 }
 
@@ -163,7 +148,6 @@ export function pushRootCachePool(root: FiberRoot) {
   // the root itself during the complete/unwind phase of the HostRoot.
   const rootCache = root.pooledCache;
   if (rootCache != null) {
-    trace('pushRootCachePool');
     pooledCache = rootCache;
     root.pooledCache = null;
   } else {
@@ -182,7 +166,6 @@ export function popRootCachePool(root: FiberRoot, renderLanes: Lanes) {
   // on it (which we track with `pooledCacheLanes`) have committed.
   root.pooledCache = pooledCache;
   if (pooledCache !== null) {
-    trace('popRootCachePool');
     root.pooledCacheLanes |= renderLanes;
   }
   // set to null, conceptually we are moving ownership to the root
@@ -202,7 +185,6 @@ export function restoreSpawnedCachePool(
   if (nextParentCache !== prevCachePool.parent) {
     // There was a refresh. Don't bother restoring anything since the refresh
     // will override it.
-    trace('restoreSpawnedCachePool refresh');
     return null;
   } else {
     // No refresh. Resume with the previous cache. This will override the cache
@@ -213,7 +195,6 @@ export function restoreSpawnedCachePool(
 
     // 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.
-    trace('restoreSpawnedCachePool hasCache=' + (pooledCache != null));
     return prevCachePool;
   }
 }
@@ -228,7 +209,6 @@ export function popCachePool(workInProgress: Fiber) {
   if (!enableCache) {
     return;
   }
-  trace('popCachePool');
   _suspendedPooledCache = pooledCache;
   pooledCache = prevFreshCacheOnStack.current;
   pop(prevFreshCacheOnStack, workInProgress);
@@ -238,11 +218,6 @@ export function getSuspendedCachePool(): SpawnedCachePool | null {
   if (!enableCache) {
     return null;
   }
-  trace(
-    'getSuspendedCachePool hasCache=' +
-      ((pooledCache ?? _suspendedPooledCache) != null),
-  );
-
   // We check the cache on the stack first, since that's the one any new Caches
   // would have accessed.
   let pool = pooledCache;
@@ -278,7 +253,6 @@ export function getOffscreenDeferredCachePool(): SpawnedCachePool | null {
     // There's no deferred cache pool.
     return null;
   }
-  trace('getOffscreenDeferredCachePool');
 
   return {
     // We must also store the parent, so that when we resume we can detect

From 92d6e7c731c9a71821eb8f9f476cc76a4c8d25e4 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Thu, 7 Oct 2021 13:37:40 -0700
Subject: [PATCH 19/25] sync fork

---
 .../src/ReactFiberCacheComponent.old.js       | 66 ++++++-------------
 1 file changed, 20 insertions(+), 46 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.old.js b/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
index 60fb6c0b5e4..5ab2b783c62 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
@@ -18,12 +18,12 @@ import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols';
 import {isPrimaryRenderer} from './ReactFiberHostConfig';
 import {createCursor, push, pop} from './ReactFiberStack.old';
 import {pushProvider, popProvider} from './ReactFiberNewContext.old';
+import * as Scheduler from 'scheduler';
 
 export type Cache = {|
   controller: AbortController,
   data: Map<() => mixed, mixed>,
   refCount: number,
-  key?: string,
 |};
 
 export type CacheComponentState = {|
@@ -36,6 +36,13 @@ export type SpawnedCachePool = {|
   +pool: Cache,
 |};
 
+// Intentionally not named imports because Rollup would
+// use dynamic dispatch for CommonJS interop named imports.
+const {
+  unstable_scheduleCallback: scheduleCallback,
+  unstable_NormalPriority: NormalPriority,
+} = Scheduler;
+
 export const CacheContext: ReactContext<Cache> = enableCache
   ? {
       $$typeof: REACT_CONTEXT_TYPE,
@@ -65,7 +72,6 @@ const prevFreshCacheOnStack: StackCursor<Cache | null> = createCursor(null);
 // Creates a new empty Cache instance with a ref-count of 0. The caller is responsible
 // for retaining the cache once it is in use (retainCache), and releasing the cache
 // once it is no longer needed (releaseCache).
-let _cacheIndex = 0;
 export function createCache(): Cache {
   const cache: Cache = {
     controller: new AbortController(),
@@ -73,57 +79,36 @@ export function createCache(): Cache {
     refCount: 0,
   };
 
-  // TODO: remove debugging code
-  if (__DEV__) {
-    cache.key = '' + _cacheIndex++;
-    trace(`createCache #${cache.key ?? ''}`);
-  }
-
   return cache;
 }
 
 export function retainCache(cache: Cache) {
   if (__DEV__) {
-    trace(
-      `retainCache #${cache.key ?? ''} ${cache.refCount} -> ${cache.refCount +
-        1}`,
-    );
+    if (cache.controller.signal.aborted) {
+      console.warn(
+        'A cache instance was retained after it was already freed. ' +
+          'This likely indicates a bug in React.',
+      );
+    }
   }
   cache.refCount++;
 }
 
 // Cleanup a cache instance, potentially freeing it if there are no more references
 export function releaseCache(cache: Cache) {
-  if (__DEV__) {
-    trace(
-      `releaseCache #${cache.key ?? ''} ${cache.refCount} -> ${cache.refCount -
-        1}`,
-    );
-  }
   cache.refCount--;
   if (__DEV__) {
     if (cache.refCount < 0) {
-      throw new Error(
-        'Error in React: cache reference count should not be negative',
+      console.warn(
+        'A cache instance was released after it was already freed. ' +
+          'This likely indicates a bug in React.',
       );
     }
   }
   if (cache.refCount === 0) {
-    // TODO: considering scheduling this call, and adding error handling for
-    // any event listeners that get triggered.
-    cache.controller.abort();
-  }
-}
-
-export function trace(msg: string, levels: number = 2) {
-  if (__DEV__) {
-    // console.log(
-    //   `${msg}:\n` +
-    //     new Error().stack
-    //       .split('\n')
-    //       .slice(3, 3 + Math.max(levels, 0))
-    //       .join('\n'),
-    // );
+    scheduleCallback(NormalPriority, () => {
+      cache.controller.abort();
+    });
   }
 }
 
@@ -163,7 +148,6 @@ export function pushRootCachePool(root: FiberRoot) {
   // the root itself during the complete/unwind phase of the HostRoot.
   const rootCache = root.pooledCache;
   if (rootCache != null) {
-    trace('pushRootCachePool');
     pooledCache = rootCache;
     root.pooledCache = null;
   } else {
@@ -182,7 +166,6 @@ export function popRootCachePool(root: FiberRoot, renderLanes: Lanes) {
   // on it (which we track with `pooledCacheLanes`) have committed.
   root.pooledCache = pooledCache;
   if (pooledCache !== null) {
-    trace('popRootCachePool');
     root.pooledCacheLanes |= renderLanes;
   }
   // set to null, conceptually we are moving ownership to the root
@@ -202,7 +185,6 @@ export function restoreSpawnedCachePool(
   if (nextParentCache !== prevCachePool.parent) {
     // There was a refresh. Don't bother restoring anything since the refresh
     // will override it.
-    trace('restoreSpawnedCachePool refresh');
     return null;
   } else {
     // No refresh. Resume with the previous cache. This will override the cache
@@ -213,7 +195,6 @@ export function restoreSpawnedCachePool(
 
     // 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.
-    trace('restoreSpawnedCachePool hasCache=' + (pooledCache != null));
     return prevCachePool;
   }
 }
@@ -228,7 +209,6 @@ export function popCachePool(workInProgress: Fiber) {
   if (!enableCache) {
     return;
   }
-  trace('popCachePool');
   _suspendedPooledCache = pooledCache;
   pooledCache = prevFreshCacheOnStack.current;
   pop(prevFreshCacheOnStack, workInProgress);
@@ -238,11 +218,6 @@ export function getSuspendedCachePool(): SpawnedCachePool | null {
   if (!enableCache) {
     return null;
   }
-  trace(
-    'getSuspendedCachePool hasCache=' +
-      ((pooledCache ?? _suspendedPooledCache) != null),
-  );
-
   // We check the cache on the stack first, since that's the one any new Caches
   // would have accessed.
   let pool = pooledCache;
@@ -278,7 +253,6 @@ export function getOffscreenDeferredCachePool(): SpawnedCachePool | null {
     // There's no deferred cache pool.
     return null;
   }
-  trace('getOffscreenDeferredCachePool');
 
   return {
     // We must also store the parent, so that when we resume we can detect

From 7339ef566497df66ee3cd7f61ce26d1c179beecf Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Thu, 7 Oct 2021 13:45:44 -0700
Subject: [PATCH 20/25] more cleanup

---
 .../src/ReactFiberCommitWork.new.js            | 18 ++++++++++--------
 1 file changed, 10 insertions(+), 8 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
index edad51f7ecf..2a860065026 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
@@ -2598,8 +2598,6 @@ function commitPassiveMountEffects_complete(
 function commitPassiveMountOnFiber(
   finishedRoot: FiberRoot,
   finishedWork: Fiber,
-  // maybe thread through previous fiber or cache from previous fiber
-  //
 ): void {
   switch (finishedWork.tag) {
     case FunctionComponent:
@@ -2627,8 +2625,8 @@ function commitPassiveMountOnFiber(
           finishedWork.alternate?.memoizedState.cache;
         const nextCache: Cache = finishedWork.memoizedState.cache;
         // Retain/release the root cache.
-        // Note that on initial mount, previousCache and nextCache will be the same,
-        // this retain won't occur. To counter this, we instead retain the HostRoot's
+        // Note that on initial mount, previousCache and nextCache will be the same
+        // and this retain won't occur. To counter this, we instead retain the HostRoot's
         // initial cache when creating the root itself (see createFiberRoot() in
         // ReactFiberRoot.js). Subsequent updates that change the cache are reflected
         // here, such that previous/next caches are retained correctly.
@@ -2917,10 +2915,14 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber(
       break;
     }
     case HostRoot: {
-      if (enableCache) {
-        const cache = current.memoizedState.cache;
-        releaseCache(cache);
-      }
+      // TODO: run passive unmount effects when unmounting a root.
+      // Because passive unmount effects are not currently run,
+      // the cache instance owned by the root will never be freed.
+      // When effects are run, the cache should be freed here:
+      // if (enableCache) {
+      //   const cache = current.memoizedState.cache;
+      //   releaseCache(cache);
+      // }
       break;
     }
     case CacheComponent: {

From 52067eaccf62e022613f21b530c79075757c8b88 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Thu, 7 Oct 2021 15:20:52 -0700
Subject: [PATCH 21/25] new test for cleanup of abandoned transitions

---
 .../src/ReactFiberCacheComponent.new.js       |  6 ++
 .../src/ReactFiberCommitWork.new.js           | 60 ++++++++++++-------
 .../src/ReactFiberCommitWork.old.js           | 18 +++---
 .../src/__tests__/ReactCache-test.js          | 58 ++++++++++++++++++
 4 files changed, 112 insertions(+), 30 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
index 28d807b8265..30bdd173b5c 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
@@ -72,6 +72,7 @@ const prevFreshCacheOnStack: StackCursor<Cache | null> = createCursor(null);
 // Creates a new empty Cache instance with a ref-count of 0. The caller is responsible
 // for retaining the cache once it is in use (retainCache), and releasing the cache
 // once it is no longer needed (releaseCache).
+let _cacheIndex = 0;
 export function createCache(): Cache {
   const cache: Cache = {
     controller: new AbortController(),
@@ -79,10 +80,14 @@ export function createCache(): Cache {
     refCount: 0,
   };
 
+  (cache: any).key = String(_cacheIndex++);
+  console.log(`create ${cache.key}`);
+
   return cache;
 }
 
 export function retainCache(cache: Cache) {
+  console.log(`retain ${cache.key} ${cache.refCount} -> ${cache.refCount + 1}`);
   if (__DEV__) {
     if (cache.controller.signal.aborted) {
       console.warn(
@@ -96,6 +101,7 @@ export function retainCache(cache: Cache) {
 
 // Cleanup a cache instance, potentially freeing it if there are no more references
 export function releaseCache(cache: Cache) {
+  console.log(`retain ${cache.key} ${cache.refCount} -> ${cache.refCount - 1}`);
   cache.refCount--;
   if (__DEV__) {
     if (cache.refCount < 0) {
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
index 2a860065026..b947798a5c3 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js
@@ -2672,19 +2672,21 @@ function commitPassiveMountOnFiber(
     }
     case LegacyHiddenComponent:
     case OffscreenComponent: {
-      const previousCache: ?Cache =
-        finishedWork.alternate?.memoizedState?.cachePool?.pool;
-      const nextCache: Cache = finishedWork.memoizedState?.cachePool?.pool;
-      // Retain/release the cache used for pending (suspended) nodes.
-      // Note that this is only reached in the non-suspended/visible case:
-      // when the content is suspended/hidden, the retain/release occurs
-      // via the parent Suspense component (see case above).
-      if (nextCache !== previousCache) {
-        if (nextCache != null) {
-          retainCache(nextCache);
-        }
-        if (previousCache != null) {
-          releaseCache(previousCache);
+      if (enableCache) {
+        const previousCache: ?Cache =
+          finishedWork.alternate?.memoizedState?.cachePool?.pool;
+        const nextCache: Cache = finishedWork.memoizedState?.cachePool?.pool;
+        // Retain/release the cache used for pending (suspended) nodes.
+        // Note that this is only reached in the non-suspended/visible case:
+        // when the content is suspended/hidden, the retain/release occurs
+        // via the parent Suspense component (see case above).
+        if (nextCache !== previousCache) {
+          if (nextCache != null) {
+            retainCache(nextCache);
+          }
+          if (previousCache != null) {
+            releaseCache(previousCache);
+          }
         }
       }
       break;
@@ -2914,15 +2916,29 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber(
       }
       break;
     }
-    case HostRoot: {
-      // TODO: run passive unmount effects when unmounting a root.
-      // Because passive unmount effects are not currently run,
-      // the cache instance owned by the root will never be freed.
-      // When effects are run, the cache should be freed here:
-      // if (enableCache) {
-      //   const cache = current.memoizedState.cache;
-      //   releaseCache(cache);
-      // }
+    // TODO: run passive unmount effects when unmounting a root.
+    // Because passive unmount effects are not currently run,
+    // the cache instance owned by the root will never be freed.
+    // When effects are run, the cache should be freed here:
+    // case HostRoot: {
+    //   if (enableCache) {
+    //     const cache = current.memoizedState.cache;
+    //     releaseCache(cache);
+    //   }
+    //   break;
+    // }
+    case LegacyHiddenComponent:
+    case OffscreenComponent: {
+      if (enableCache) {
+        const cache: Cache = current.memoizedState?.cachePool?.pool;
+        // Retain/release the cache used for pending (suspended) nodes.
+        // Note that this is only reached in the non-suspended/visible case:
+        // when the content is suspended/hidden, the retain/release occurs
+        // via the parent Suspense component (see case above).
+        if (cache != null) {
+          retainCache(cache);
+        }
+      }
       break;
     }
     case CacheComponent: {
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js
index 95486896b5b..b30accf99d0 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js
@@ -2598,8 +2598,6 @@ function commitPassiveMountEffects_complete(
 function commitPassiveMountOnFiber(
   finishedRoot: FiberRoot,
   finishedWork: Fiber,
-  // maybe thread through previous fiber or cache from previous fiber
-  //
 ): void {
   switch (finishedWork.tag) {
     case FunctionComponent:
@@ -2627,8 +2625,8 @@ function commitPassiveMountOnFiber(
           finishedWork.alternate?.memoizedState.cache;
         const nextCache: Cache = finishedWork.memoizedState.cache;
         // Retain/release the root cache.
-        // Note that on initial mount, previousCache and nextCache will be the same,
-        // this retain won't occur. To counter this, we instead retain the HostRoot's
+        // Note that on initial mount, previousCache and nextCache will be the same
+        // and this retain won't occur. To counter this, we instead retain the HostRoot's
         // initial cache when creating the root itself (see createFiberRoot() in
         // ReactFiberRoot.js). Subsequent updates that change the cache are reflected
         // here, such that previous/next caches are retained correctly.
@@ -2917,10 +2915,14 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber(
       break;
     }
     case HostRoot: {
-      if (enableCache) {
-        const cache = current.memoizedState.cache;
-        releaseCache(cache);
-      }
+      // TODO: run passive unmount effects when unmounting a root.
+      // Because passive unmount effects are not currently run,
+      // the cache instance owned by the root will never be freed.
+      // When effects are run, the cache should be freed here:
+      // if (enableCache) {
+      //   const cache = current.memoizedState.cache;
+      //   releaseCache(cache);
+      // }
       break;
     }
     case CacheComponent: {
diff --git a/packages/react-reconciler/src/__tests__/ReactCache-test.js b/packages/react-reconciler/src/__tests__/ReactCache-test.js
index 5cb10b79c99..57b8cd5475c 100644
--- a/packages/react-reconciler/src/__tests__/ReactCache-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactCache-test.js
@@ -1453,4 +1453,62 @@ describe('ReactCache', () => {
     ]);
     expect(root).toMatchRenderedOutput('Bye!');
   });
+
+  // @gate experimental || www
+  test.only('cleans up cache only used in an aborted transition', async () => {
+    const root = ReactNoop.createRoot();
+    seedNextTextCache('A');
+    console.log('render A');
+    await act(async () => {
+      root.render(
+        <Suspense fallback="Loading...">
+          <Cache key="A">
+            <AsyncText showVersion={true} text="A" />
+          </Cache>
+        </Suspense>,
+      );
+    });
+    expect(Scheduler).toHaveYielded(['A [v1]']);
+    expect(root).toMatchRenderedOutput('A [v1]');
+
+    // Start a transition from A -> B..., which should create a fresh cache
+    // for the new cache boundary (bc of the different key)
+    console.log('A -> B');
+    await act(async () => {
+      startTransition(() => {
+        root.render(
+          <Suspense fallback="Loading...">
+            <Cache key="B">
+              <AsyncText showVersion={true} text="B" />
+            </Cache>
+          </Suspense>,
+        );
+      });
+    });
+    expect(Scheduler).toHaveYielded(['Cache miss! [B]']);
+    expect(root).toMatchRenderedOutput('A [v1]');
+
+    // ...but cancel by transitioning "back" to A (which we never really left)
+    console.log('B -> A');
+    await act(async () => {
+      startTransition(() => {
+        root.render(
+          <Suspense fallback="Loading...">
+            <Cache key="A">
+              <AsyncText showVersion={true} text="A" />
+            </Cache>
+          </Suspense>,
+        );
+      });
+    });
+    expect(Scheduler).toHaveYielded(['A [v1]', 'Cache cleanup: B [v2]']);
+    expect(root).toMatchRenderedOutput('A [v1]');
+
+    // Unmount children: ...
+    await act(async () => {
+      root.render('Bye!');
+    });
+    expect(Scheduler).toHaveYielded([]);
+    expect(root).toMatchRenderedOutput('Bye!');
+  });
 });

From 20ce291082730f3dea3068c51058d6ea3f048dd1 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Fri, 8 Oct 2021 09:27:49 -0700
Subject: [PATCH 22/25] handle aborted transactions

---
 .../src/ReactFiberCacheComponent.new.js       | 12 ++--
 .../src/ReactFiberCacheComponent.old.js       |  6 +-
 .../src/ReactFiberCommitWork.old.js           | 60 ++++++++++++-------
 .../src/ReactFiberRoot.new.js                 |  1 +
 .../src/ReactFiberRoot.old.js                 |  1 +
 .../src/ReactFiberWorkLoop.new.js             | 24 ++++++--
 .../src/ReactFiberWorkLoop.old.js             | 24 ++++++--
 .../src/__tests__/ReactCache-test.js          |  5 +-
 8 files changed, 89 insertions(+), 44 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
index 30bdd173b5c..182d0e9d83c 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
@@ -72,7 +72,6 @@ const prevFreshCacheOnStack: StackCursor<Cache | null> = createCursor(null);
 // Creates a new empty Cache instance with a ref-count of 0. The caller is responsible
 // for retaining the cache once it is in use (retainCache), and releasing the cache
 // once it is no longer needed (releaseCache).
-let _cacheIndex = 0;
 export function createCache(): Cache {
   const cache: Cache = {
     controller: new AbortController(),
@@ -80,14 +79,10 @@ export function createCache(): Cache {
     refCount: 0,
   };
 
-  (cache: any).key = String(_cacheIndex++);
-  console.log(`create ${cache.key}`);
-
   return cache;
 }
 
 export function retainCache(cache: Cache) {
-  console.log(`retain ${cache.key} ${cache.refCount} -> ${cache.refCount + 1}`);
   if (__DEV__) {
     if (cache.controller.signal.aborted) {
       console.warn(
@@ -101,7 +96,6 @@ export function retainCache(cache: Cache) {
 
 // Cleanup a cache instance, potentially freeing it if there are no more references
 export function releaseCache(cache: Cache) {
-  console.log(`retain ${cache.key} ${cache.refCount} -> ${cache.refCount - 1}`);
   cache.refCount--;
   if (__DEV__) {
     if (cache.refCount < 0) {
@@ -139,8 +133,12 @@ export function requestCacheFromPool(renderLanes: Lanes): Cache {
   if (pooledCache !== null) {
     return pooledCache;
   }
-  // Create a fresh cache.
+  // Create a fresh cache. The pooled cache must be owned - it is freed
+  // in releaseRootPooledCache() - but the cache instance handed out
+  // is retained/released in the commit phase of the component that
+  // references is (ie the host root, cache boundary, suspense component)
   pooledCache = createCache();
+  retainCache(pooledCache);
   return pooledCache;
 }
 
diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.old.js b/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
index 5ab2b783c62..c33f6b9a2a6 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
@@ -133,8 +133,12 @@ export function requestCacheFromPool(renderLanes: Lanes): Cache {
   if (pooledCache !== null) {
     return pooledCache;
   }
-  // Create a fresh cache.
+  // Create a fresh cache. The pooled cache must be owned - it is freed
+  // in releaseRootPooledCache() - but the cache instance handed out
+  // is retained/released in the commit phase of the component that
+  // references is (ie the host root, cache boundary, suspense component)
   pooledCache = createCache();
+  retainCache(pooledCache);
   return pooledCache;
 }
 
diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js
index b30accf99d0..496d9f81ab3 100644
--- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js
+++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js
@@ -2672,19 +2672,21 @@ function commitPassiveMountOnFiber(
     }
     case LegacyHiddenComponent:
     case OffscreenComponent: {
-      const previousCache: ?Cache =
-        finishedWork.alternate?.memoizedState?.cachePool?.pool;
-      const nextCache: Cache = finishedWork.memoizedState?.cachePool?.pool;
-      // Retain/release the cache used for pending (suspended) nodes.
-      // Note that this is only reached in the non-suspended/visible case:
-      // when the content is suspended/hidden, the retain/release occurs
-      // via the parent Suspense component (see case above).
-      if (nextCache !== previousCache) {
-        if (nextCache != null) {
-          retainCache(nextCache);
-        }
-        if (previousCache != null) {
-          releaseCache(previousCache);
+      if (enableCache) {
+        const previousCache: ?Cache =
+          finishedWork.alternate?.memoizedState?.cachePool?.pool;
+        const nextCache: Cache = finishedWork.memoizedState?.cachePool?.pool;
+        // Retain/release the cache used for pending (suspended) nodes.
+        // Note that this is only reached in the non-suspended/visible case:
+        // when the content is suspended/hidden, the retain/release occurs
+        // via the parent Suspense component (see case above).
+        if (nextCache !== previousCache) {
+          if (nextCache != null) {
+            retainCache(nextCache);
+          }
+          if (previousCache != null) {
+            releaseCache(previousCache);
+          }
         }
       }
       break;
@@ -2914,15 +2916,29 @@ function commitPassiveUnmountInsideDeletedTreeOnFiber(
       }
       break;
     }
-    case HostRoot: {
-      // TODO: run passive unmount effects when unmounting a root.
-      // Because passive unmount effects are not currently run,
-      // the cache instance owned by the root will never be freed.
-      // When effects are run, the cache should be freed here:
-      // if (enableCache) {
-      //   const cache = current.memoizedState.cache;
-      //   releaseCache(cache);
-      // }
+    // TODO: run passive unmount effects when unmounting a root.
+    // Because passive unmount effects are not currently run,
+    // the cache instance owned by the root will never be freed.
+    // When effects are run, the cache should be freed here:
+    // case HostRoot: {
+    //   if (enableCache) {
+    //     const cache = current.memoizedState.cache;
+    //     releaseCache(cache);
+    //   }
+    //   break;
+    // }
+    case LegacyHiddenComponent:
+    case OffscreenComponent: {
+      if (enableCache) {
+        const cache: Cache = current.memoizedState?.cachePool?.pool;
+        // Retain/release the cache used for pending (suspended) nodes.
+        // Note that this is only reached in the non-suspended/visible case:
+        // when the content is suspended/hidden, the retain/release occurs
+        // via the parent Suspense component (see case above).
+        if (cache != null) {
+          retainCache(cache);
+        }
+      }
       break;
     }
     case CacheComponent: {
diff --git a/packages/react-reconciler/src/ReactFiberRoot.new.js b/packages/react-reconciler/src/ReactFiberRoot.new.js
index 73a83b47415..9560bb0f577 100644
--- a/packages/react-reconciler/src/ReactFiberRoot.new.js
+++ b/packages/react-reconciler/src/ReactFiberRoot.new.js
@@ -121,6 +121,7 @@ export function createFiberRoot(
     const initialCache = createCache();
     retainCache(initialCache);
     root.pooledCache = initialCache;
+    retainCache(initialCache); // retain twice
     const initialState = {
       element: null,
       cache: initialCache,
diff --git a/packages/react-reconciler/src/ReactFiberRoot.old.js b/packages/react-reconciler/src/ReactFiberRoot.old.js
index 0dec8ef7132..e90a3199796 100644
--- a/packages/react-reconciler/src/ReactFiberRoot.old.js
+++ b/packages/react-reconciler/src/ReactFiberRoot.old.js
@@ -121,6 +121,7 @@ export function createFiberRoot(
     const initialCache = createCache();
     retainCache(initialCache);
     root.pooledCache = initialCache;
+    retainCache(initialCache); // retain twice
     const initialState = {
       element: null,
       cache: initialCache,
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
index 3c051fbfeca..d1a07849437 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -237,6 +237,7 @@ import {
   isDevToolsPresent,
 } from './ReactFiberDevToolsHook.new';
 import {onCommitRoot as onCommitRootTestSelector} from './ReactTestSelectors';
+import {releaseCache} from './ReactFiberCacheComponent.new';
 
 const ceil = Math.ceil;
 
@@ -1880,6 +1881,10 @@ function commitRootImpl(root, renderPriorityLevel) {
       rootDoesHavePassiveEffects = true;
       scheduleCallback(NormalSchedulerPriority, () => {
         flushPassiveEffects();
+        // This render triggered passive effects: release the root cache pool
+        // *after* passive effects fire to avoid freeing a cache pool that may
+        // be referenced by a node in the tree (HostRoot, Cache boundary etc)
+        releaseRootPooledCache(root, remainingLanes);
         return null;
       });
     }
@@ -2052,7 +2057,11 @@ function commitRootImpl(root, renderPriorityLevel) {
     hasUncaughtError = false;
     const error = firstUncaughtError;
     firstUncaughtError = null;
-    releaseRootPooledCache(root, remainingLanes);
+    if (!rootDidHavePassiveEffects) {
+      // There were no passive effects, so we can immediately release the cache
+      // pool for this render.
+      releaseRootPooledCache(root, remainingLanes);
+    }
     throw error;
   }
 
@@ -2090,13 +2099,15 @@ function commitRootImpl(root, renderPriorityLevel) {
     nestedUpdateCount = 0;
   }
 
+  if (!rootDidHavePassiveEffects) {
+    // There were no passive effects, so we can immediately release the cache
+    // pool for this render.
+    releaseRootPooledCache(root, remainingLanes);
+  }
+
   // If layout work was scheduled, flush it now.
   flushSyncCallbacks();
 
-  // Now that effects have run - giving Cache boundaries a chance to retain the
-  // cache instance - release the root's cache in case since the render is complete
-  releaseRootPooledCache(root, remainingLanes);
-
   if (__DEV__) {
     if (enableDebugTracing) {
       logCommitStopped();
@@ -2116,6 +2127,9 @@ function releaseRootPooledCache(root: FiberRoot, remainingLanes: Lanes) {
     if (pooledCacheLanes === NoLanes) {
       // None of the remaining work relies on the cache pool. Clear it so
       // subsequent requests get a new cache
+      if (root.pooledCache != null) {
+        releaseCache(root.pooledCache);
+      }
       root.pooledCache = null;
     }
   }
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
index 0c439f43e5a..56e6e65bc02 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
@@ -237,6 +237,7 @@ import {
   isDevToolsPresent,
 } from './ReactFiberDevToolsHook.old';
 import {onCommitRoot as onCommitRootTestSelector} from './ReactTestSelectors';
+import {releaseCache} from './ReactFiberCacheComponent.old';
 
 const ceil = Math.ceil;
 
@@ -1880,6 +1881,10 @@ function commitRootImpl(root, renderPriorityLevel) {
       rootDoesHavePassiveEffects = true;
       scheduleCallback(NormalSchedulerPriority, () => {
         flushPassiveEffects();
+        // This render triggered passive effects: release the root cache pool
+        // *after* passive effects fire to avoid freeing a cache pool that may
+        // be referenced by a node in the tree (HostRoot, Cache boundary etc)
+        releaseRootPooledCache(root, remainingLanes);
         return null;
       });
     }
@@ -2052,7 +2057,11 @@ function commitRootImpl(root, renderPriorityLevel) {
     hasUncaughtError = false;
     const error = firstUncaughtError;
     firstUncaughtError = null;
-    releaseRootPooledCache(root, remainingLanes);
+    if (!rootDidHavePassiveEffects) {
+      // There were no passive effects, so we can immediately release the cache
+      // pool for this render.
+      releaseRootPooledCache(root, remainingLanes);
+    }
     throw error;
   }
 
@@ -2090,13 +2099,15 @@ function commitRootImpl(root, renderPriorityLevel) {
     nestedUpdateCount = 0;
   }
 
+  if (!rootDidHavePassiveEffects) {
+    // There were no passive effects, so we can immediately release the cache
+    // pool for this render.
+    releaseRootPooledCache(root, remainingLanes);
+  }
+
   // If layout work was scheduled, flush it now.
   flushSyncCallbacks();
 
-  // Now that effects have run - giving Cache boundaries a chance to retain the
-  // cache instance - release the root's cache in case since the render is complete
-  releaseRootPooledCache(root, remainingLanes);
-
   if (__DEV__) {
     if (enableDebugTracing) {
       logCommitStopped();
@@ -2116,6 +2127,9 @@ function releaseRootPooledCache(root: FiberRoot, remainingLanes: Lanes) {
     if (pooledCacheLanes === NoLanes) {
       // None of the remaining work relies on the cache pool. Clear it so
       // subsequent requests get a new cache
+      if (root.pooledCache != null) {
+        releaseCache(root.pooledCache);
+      }
       root.pooledCache = null;
     }
   }
diff --git a/packages/react-reconciler/src/__tests__/ReactCache-test.js b/packages/react-reconciler/src/__tests__/ReactCache-test.js
index 57b8cd5475c..6056eae3393 100644
--- a/packages/react-reconciler/src/__tests__/ReactCache-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactCache-test.js
@@ -1455,10 +1455,9 @@ describe('ReactCache', () => {
   });
 
   // @gate experimental || www
-  test.only('cleans up cache only used in an aborted transition', async () => {
+  test('cleans up cache only used in an aborted transition', async () => {
     const root = ReactNoop.createRoot();
     seedNextTextCache('A');
-    console.log('render A');
     await act(async () => {
       root.render(
         <Suspense fallback="Loading...">
@@ -1473,7 +1472,6 @@ describe('ReactCache', () => {
 
     // Start a transition from A -> B..., which should create a fresh cache
     // for the new cache boundary (bc of the different key)
-    console.log('A -> B');
     await act(async () => {
       startTransition(() => {
         root.render(
@@ -1489,7 +1487,6 @@ describe('ReactCache', () => {
     expect(root).toMatchRenderedOutput('A [v1]');
 
     // ...but cancel by transitioning "back" to A (which we never really left)
-    console.log('B -> A');
     await act(async () => {
       startTransition(() => {
         root.render(

From 3d61c1d484cb7b53652125b73713aa5143624b66 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Fri, 8 Oct 2021 09:34:07 -0700
Subject: [PATCH 23/25] comments

---
 packages/react-reconciler/src/ReactFiberWorkLoop.new.js | 7 ++++---
 packages/react-reconciler/src/ReactFiberWorkLoop.old.js | 7 ++++---
 2 files changed, 8 insertions(+), 6 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
index d1a07849437..26d03ebe5ae 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -2127,10 +2127,11 @@ function releaseRootPooledCache(root: FiberRoot, remainingLanes: Lanes) {
     if (pooledCacheLanes === NoLanes) {
       // None of the remaining work relies on the cache pool. Clear it so
       // subsequent requests get a new cache
-      if (root.pooledCache != null) {
-        releaseCache(root.pooledCache);
+      const pooledCache = root.pooledCache;
+      if (pooledCache != null) {
+        root.pooledCache = null;
+        releaseCache(pooledCache);
       }
-      root.pooledCache = null;
     }
   }
 }
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
index 56e6e65bc02..b181d26eeac 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
@@ -2127,10 +2127,11 @@ function releaseRootPooledCache(root: FiberRoot, remainingLanes: Lanes) {
     if (pooledCacheLanes === NoLanes) {
       // None of the remaining work relies on the cache pool. Clear it so
       // subsequent requests get a new cache
-      if (root.pooledCache != null) {
-        releaseCache(root.pooledCache);
+      const pooledCache = root.pooledCache;
+      if (pooledCache != null) {
+        root.pooledCache = null;
+        releaseCache(pooledCache);
       }
-      root.pooledCache = null;
     }
   }
 }

From 07ca3cafaa1d474e2e9f8c733d3cf9668eb716b7 Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Fri, 8 Oct 2021 16:43:51 -0700
Subject: [PATCH 24/25] Add comments/todos, move pooled cache release, add
 failing test cases for refresh

---
 .../src/ReactFiberCacheComponent.new.js       |  2 +
 .../src/ReactFiberCacheComponent.old.js       |  2 +
 .../src/ReactFiberHooks.new.js                |  3 +
 .../src/ReactFiberHooks.old.js                |  3 +
 .../src/ReactFiberRoot.new.js                 | 10 ++-
 .../src/ReactFiberRoot.old.js                 | 10 ++-
 .../src/ReactFiberWorkLoop.new.js             | 32 +++++---
 .../src/ReactFiberWorkLoop.old.js             | 32 +++++---
 .../src/__tests__/ReactCache-test.js          | 82 +++++++++++++++++++
 9 files changed, 150 insertions(+), 26 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
index 182d0e9d83c..8d01b77d9bf 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.new.js
@@ -137,6 +137,8 @@ export function requestCacheFromPool(renderLanes: Lanes): Cache {
   // in releaseRootPooledCache() - but the cache instance handed out
   // is retained/released in the commit phase of the component that
   // references is (ie the host root, cache boundary, suspense component)
+  // Ie, pooledCache is conceptually an Option<Arc<Cache>> (owned),
+  // whereas the return value of this function is a &Arc<Cache> (borrowed).
   pooledCache = createCache();
   retainCache(pooledCache);
   return pooledCache;
diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.old.js b/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
index c33f6b9a2a6..1041012b155 100644
--- a/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
+++ b/packages/react-reconciler/src/ReactFiberCacheComponent.old.js
@@ -137,6 +137,8 @@ export function requestCacheFromPool(renderLanes: Lanes): Cache {
   // in releaseRootPooledCache() - but the cache instance handed out
   // is retained/released in the commit phase of the component that
   // references is (ie the host root, cache boundary, suspense component)
+  // Ie, pooledCache is conceptually an Option<Arc<Cache>> (owned),
+  // whereas the return value of this function is a &Arc<Cache> (borrowed).
   pooledCache = createCache();
   retainCache(pooledCache);
   return pooledCache;
diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js
index 09cbc4d24fd..a0b762ba506 100644
--- a/packages/react-reconciler/src/ReactFiberHooks.new.js
+++ b/packages/react-reconciler/src/ReactFiberHooks.new.js
@@ -2128,6 +2128,9 @@ function refreshCache<T>(fiber: Fiber, seedKey: ?() => T, seedValue: T) {
           entangleLegacyQueueTransitions(root, provider, lane);
         }
 
+        // TODO: If a refresh never commits, the new cache created here must be
+        // released. A simple case is start refreshing a cache boundary, but then
+        // unmount that boundary before the refresh completes.
         const seededCache = createCache();
         if (seedKey !== null && seedKey !== undefined && root !== null) {
           // Seed the cache with the value passed by the caller. This could be
diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js
index cbc861a2921..0afd40d4a96 100644
--- a/packages/react-reconciler/src/ReactFiberHooks.old.js
+++ b/packages/react-reconciler/src/ReactFiberHooks.old.js
@@ -2128,6 +2128,9 @@ function refreshCache<T>(fiber: Fiber, seedKey: ?() => T, seedValue: T) {
           entangleLegacyQueueTransitions(root, provider, lane);
         }
 
+        // TODO: If a refresh never commits, the new cache created here must be
+        // released. A simple case is start refreshing a cache boundary, but then
+        // unmount that boundary before the refresh completes.
         const seededCache = createCache();
         if (seedKey !== null && seedKey !== undefined && root !== null) {
           // Seed the cache with the value passed by the caller. This could be
diff --git a/packages/react-reconciler/src/ReactFiberRoot.new.js b/packages/react-reconciler/src/ReactFiberRoot.new.js
index 9560bb0f577..7164f77e0c6 100644
--- a/packages/react-reconciler/src/ReactFiberRoot.new.js
+++ b/packages/react-reconciler/src/ReactFiberRoot.new.js
@@ -120,8 +120,16 @@ export function createFiberRoot(
   if (enableCache) {
     const initialCache = createCache();
     retainCache(initialCache);
+
+    // The pooledCache is a fresh cache instance is being used for newly
+    // mounted boundaries during a render. In general, the pooledCache is
+    // always cleared from the root at the end of a render: it is either
+    // released when render commits, or moved to an Offscreen component
+    // if rendering suspends. Because the lifetime of the pooled cache
+    // is distinct from the main memoizedState.cache, it must be retained
+    // separately.
     root.pooledCache = initialCache;
-    retainCache(initialCache); // retain twice
+    retainCache(initialCache);
     const initialState = {
       element: null,
       cache: initialCache,
diff --git a/packages/react-reconciler/src/ReactFiberRoot.old.js b/packages/react-reconciler/src/ReactFiberRoot.old.js
index e90a3199796..180d500c01f 100644
--- a/packages/react-reconciler/src/ReactFiberRoot.old.js
+++ b/packages/react-reconciler/src/ReactFiberRoot.old.js
@@ -120,8 +120,16 @@ export function createFiberRoot(
   if (enableCache) {
     const initialCache = createCache();
     retainCache(initialCache);
+
+    // The pooledCache is a fresh cache instance is being used for newly
+    // mounted boundaries during a render. In general, the pooledCache is
+    // always cleared from the root at the end of a render: it is either
+    // released when render commits, or moved to an Offscreen component
+    // if rendering suspends. Because the lifetime of the pooled cache
+    // is distinct from the main memoizedState.cache, it must be retained
+    // separately.
     root.pooledCache = initialCache;
-    retainCache(initialCache); // retain twice
+    retainCache(initialCache);
     const initialState = {
       element: null,
       cache: initialCache,
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
index 26d03ebe5ae..71c83ad42e1 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js
@@ -333,6 +333,7 @@ let rootDoesHavePassiveEffects: boolean = false;
 let rootWithPendingPassiveEffects: FiberRoot | null = null;
 let pendingPassiveEffectsLanes: Lanes = NoLanes;
 let pendingPassiveProfilerEffects: Array<Fiber> = [];
+let pendingPassiveEffectsRemainingLanes: Lanes = NoLanes;
 
 // Use these to prevent an infinite loop of nested updates
 const NESTED_UPDATE_LIMIT = 50;
@@ -1879,12 +1880,12 @@ function commitRootImpl(root, renderPriorityLevel) {
   ) {
     if (!rootDoesHavePassiveEffects) {
       rootDoesHavePassiveEffects = true;
+      pendingPassiveEffectsRemainingLanes = remainingLanes;
       scheduleCallback(NormalSchedulerPriority, () => {
         flushPassiveEffects();
         // This render triggered passive effects: release the root cache pool
         // *after* passive effects fire to avoid freeing a cache pool that may
         // be referenced by a node in the tree (HostRoot, Cache boundary etc)
-        releaseRootPooledCache(root, remainingLanes);
         return null;
       });
     }
@@ -2010,6 +2011,10 @@ function commitRootImpl(root, renderPriorityLevel) {
     rootDoesHavePassiveEffects = false;
     rootWithPendingPassiveEffects = root;
     pendingPassiveEffectsLanes = lanes;
+  } else {
+    // There were no passive effects, so we can immediately release the cache
+    // pool for this render.
+    releaseRootPooledCache(root, remainingLanes);
   }
 
   // Read this again, since an effect might have updated it
@@ -2057,11 +2062,6 @@ function commitRootImpl(root, renderPriorityLevel) {
     hasUncaughtError = false;
     const error = firstUncaughtError;
     firstUncaughtError = null;
-    if (!rootDidHavePassiveEffects) {
-      // There were no passive effects, so we can immediately release the cache
-      // pool for this render.
-      releaseRootPooledCache(root, remainingLanes);
-    }
     throw error;
   }
 
@@ -2099,12 +2099,6 @@ function commitRootImpl(root, renderPriorityLevel) {
     nestedUpdateCount = 0;
   }
 
-  if (!rootDidHavePassiveEffects) {
-    // There were no passive effects, so we can immediately release the cache
-    // pool for this render.
-    releaseRootPooledCache(root, remainingLanes);
-  }
-
   // If layout work was scheduled, flush it now.
   flushSyncCallbacks();
 
@@ -2144,6 +2138,15 @@ export function flushPassiveEffects(): boolean {
   // `Scheduler.runWithPriority`, which accepts a function. But now we track the
   // priority within React itself, so we can mutate the variable directly.
   if (rootWithPendingPassiveEffects !== null) {
+    // Cache the root since rootWithPendingPassiveEffects is cleared in
+    // flushPassiveEffectsImpl
+    const root = rootWithPendingPassiveEffects;
+    // Cache and clear the remaining lanes flag; it must be reset since this
+    // method can be called from various places, not always from commitRoot
+    // where the remaining lanes are known
+    const remainingLanes = pendingPassiveEffectsRemainingLanes;
+    pendingPassiveEffectsRemainingLanes = NoLanes;
+
     const renderPriority = lanesToEventPriority(pendingPassiveEffectsLanes);
     const priority = lowerEventPriority(DefaultEventPriority, renderPriority);
     const prevTransition = ReactCurrentBatchConfig.transition;
@@ -2155,6 +2158,11 @@ export function flushPassiveEffects(): boolean {
     } finally {
       setCurrentUpdatePriority(previousPriority);
       ReactCurrentBatchConfig.transition = prevTransition;
+
+      // Once passive effects have run for the tree - giving components a
+      // chance to retain cache instances they use - release the pooled
+      // cache at the root (if there is one)
+      releaseRootPooledCache(root, remainingLanes);
     }
   }
   return false;
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
index b181d26eeac..84e9a2d15a2 100644
--- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
+++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js
@@ -333,6 +333,7 @@ let rootDoesHavePassiveEffects: boolean = false;
 let rootWithPendingPassiveEffects: FiberRoot | null = null;
 let pendingPassiveEffectsLanes: Lanes = NoLanes;
 let pendingPassiveProfilerEffects: Array<Fiber> = [];
+let pendingPassiveEffectsRemainingLanes: Lanes = NoLanes;
 
 // Use these to prevent an infinite loop of nested updates
 const NESTED_UPDATE_LIMIT = 50;
@@ -1879,12 +1880,12 @@ function commitRootImpl(root, renderPriorityLevel) {
   ) {
     if (!rootDoesHavePassiveEffects) {
       rootDoesHavePassiveEffects = true;
+      pendingPassiveEffectsRemainingLanes = remainingLanes;
       scheduleCallback(NormalSchedulerPriority, () => {
         flushPassiveEffects();
         // This render triggered passive effects: release the root cache pool
         // *after* passive effects fire to avoid freeing a cache pool that may
         // be referenced by a node in the tree (HostRoot, Cache boundary etc)
-        releaseRootPooledCache(root, remainingLanes);
         return null;
       });
     }
@@ -2010,6 +2011,10 @@ function commitRootImpl(root, renderPriorityLevel) {
     rootDoesHavePassiveEffects = false;
     rootWithPendingPassiveEffects = root;
     pendingPassiveEffectsLanes = lanes;
+  } else {
+    // There were no passive effects, so we can immediately release the cache
+    // pool for this render.
+    releaseRootPooledCache(root, remainingLanes);
   }
 
   // Read this again, since an effect might have updated it
@@ -2057,11 +2062,6 @@ function commitRootImpl(root, renderPriorityLevel) {
     hasUncaughtError = false;
     const error = firstUncaughtError;
     firstUncaughtError = null;
-    if (!rootDidHavePassiveEffects) {
-      // There were no passive effects, so we can immediately release the cache
-      // pool for this render.
-      releaseRootPooledCache(root, remainingLanes);
-    }
     throw error;
   }
 
@@ -2099,12 +2099,6 @@ function commitRootImpl(root, renderPriorityLevel) {
     nestedUpdateCount = 0;
   }
 
-  if (!rootDidHavePassiveEffects) {
-    // There were no passive effects, so we can immediately release the cache
-    // pool for this render.
-    releaseRootPooledCache(root, remainingLanes);
-  }
-
   // If layout work was scheduled, flush it now.
   flushSyncCallbacks();
 
@@ -2144,6 +2138,15 @@ export function flushPassiveEffects(): boolean {
   // `Scheduler.runWithPriority`, which accepts a function. But now we track the
   // priority within React itself, so we can mutate the variable directly.
   if (rootWithPendingPassiveEffects !== null) {
+    // Cache the root since rootWithPendingPassiveEffects is cleared in
+    // flushPassiveEffectsImpl
+    const root = rootWithPendingPassiveEffects;
+    // Cache and clear the remaining lanes flag; it must be reset since this
+    // method can be called from various places, not always from commitRoot
+    // where the remaining lanes are known
+    const remainingLanes = pendingPassiveEffectsRemainingLanes;
+    pendingPassiveEffectsRemainingLanes = NoLanes;
+
     const renderPriority = lanesToEventPriority(pendingPassiveEffectsLanes);
     const priority = lowerEventPriority(DefaultEventPriority, renderPriority);
     const prevTransition = ReactCurrentBatchConfig.transition;
@@ -2155,6 +2158,11 @@ export function flushPassiveEffects(): boolean {
     } finally {
       setCurrentUpdatePriority(previousPriority);
       ReactCurrentBatchConfig.transition = prevTransition;
+
+      // Once passive effects have run for the tree - giving components a
+      // chance to retain cache instances they use - release the pooled
+      // cache at the root (if there is one)
+      releaseRootPooledCache(root, remainingLanes);
     }
   }
   return false;
diff --git a/packages/react-reconciler/src/__tests__/ReactCache-test.js b/packages/react-reconciler/src/__tests__/ReactCache-test.js
index 6056eae3393..fbcfb3df353 100644
--- a/packages/react-reconciler/src/__tests__/ReactCache-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactCache-test.js
@@ -1508,4 +1508,86 @@ describe('ReactCache', () => {
     expect(Scheduler).toHaveYielded([]);
     expect(root).toMatchRenderedOutput('Bye!');
   });
+
+  // @gate experimental || www
+  test.skip('fresh cache for root refresh is released if the refresh never commits', async () => {
+    const root = ReactNoop.createRoot();
+    let refresh;
+    function Example({text}) {
+      refresh = useCacheRefresh();
+      return <AsyncText showVersion={true} text={text} />;
+    }
+    seedNextTextCache('A');
+    await act(async () => {
+      root.render(
+        <Suspense fallback="Loading...">
+          <Example text="A" />
+        </Suspense>,
+      );
+    });
+    expect(Scheduler).toHaveYielded(['A [v1]']);
+    expect(root).toMatchRenderedOutput('A [v1]');
+
+    await act(async () => {
+      startTransition(() => {
+        refresh();
+      });
+    });
+    expect(Scheduler).toHaveYielded(['Cache miss! [A]']);
+    expect(root).toMatchRenderedOutput('A [v1]');
+
+    await act(async () => {
+      root.render('Bye!');
+    });
+    expect(Scheduler).toHaveYielded([
+      // TODO: the v1 cache should *not* be cleaned up, it is still retained by the root
+      // The following line is presently yielded but should not be:
+      // 'Cache cleanup: A [v1] (cached)',
+
+      // TODO: the v2 cache *should* be cleaned up, it was created for the abandoned refresh
+      // The following line is presently not yielded but should be:
+      'Cache cleanup: A [v2]',
+    ]);
+    expect(root).toMatchRenderedOutput('Bye!');
+  });
+
+  // @gate experimental || www
+  test.skip('fresh cache for cache boundary refresh is released if the refresh never commits', async () => {
+    const root = ReactNoop.createRoot();
+    let refresh;
+    function Example({text}) {
+      refresh = useCacheRefresh();
+      return <AsyncText showVersion={true} text={text} />;
+    }
+    seedNextTextCache('A');
+    await act(async () => {
+      root.render(
+        <Suspense fallback="Loading...">
+          <Cache>
+            <Example text="A" />
+          </Cache>
+        </Suspense>,
+      );
+    });
+    expect(Scheduler).toHaveYielded(['A [v1]']);
+    expect(root).toMatchRenderedOutput('A [v1]');
+
+    await act(async () => {
+      startTransition(() => {
+        refresh();
+      });
+    });
+    expect(Scheduler).toHaveYielded(['Cache miss! [A]']);
+    expect(root).toMatchRenderedOutput('A [v1]');
+
+    await act(async () => {
+      root.render('Bye!');
+    });
+    expect(Scheduler).toHaveYielded([
+      // TODO: the v2 cache *should* be cleaned up, it was created for the abandoned refresh
+      // The following line is presently not yielded but should be:
+      'Cache cleanup: A [v2]',
+    ]);
+    expect(root).toMatchRenderedOutput('Bye!');
+  });
 });

From daba131d447bbb51b9d3bd71aa97fcb35f23797a Mon Sep 17 00:00:00 2001
From: Joe Savona <joesavona@fb.com>
Date: Tue, 12 Oct 2021 09:57:55 -0700
Subject: [PATCH 25/25] finalish comments

---
 .../react-reconciler/src/ReactFiberRoot.new.js     | 14 +++++++-------
 .../react-reconciler/src/ReactFiberRoot.old.js     | 14 +++++++-------
 .../src/__tests__/ReactCache-test.js               |  5 +++--
 .../StrictEffectsModeDefaults-test.internal.js     | 11 +++++------
 4 files changed, 22 insertions(+), 22 deletions(-)

diff --git a/packages/react-reconciler/src/ReactFiberRoot.new.js b/packages/react-reconciler/src/ReactFiberRoot.new.js
index 7164f77e0c6..803adee1e22 100644
--- a/packages/react-reconciler/src/ReactFiberRoot.new.js
+++ b/packages/react-reconciler/src/ReactFiberRoot.new.js
@@ -121,13 +121,13 @@ export function createFiberRoot(
     const initialCache = createCache();
     retainCache(initialCache);
 
-    // The pooledCache is a fresh cache instance is being used for newly
-    // mounted boundaries during a render. In general, the pooledCache is
-    // always cleared from the root at the end of a render: it is either
-    // released when render commits, or moved to an Offscreen component
-    // if rendering suspends. Because the lifetime of the pooled cache
-    // is distinct from the main memoizedState.cache, it must be retained
-    // separately.
+    // The pooledCache is a fresh cache instance that is used temporarily
+    // for newly mounted boundaries during a render. In general, the
+    // pooledCache is always cleared from the root at the end of a render:
+    // it is either released when render commits, or moved to an Offscreen
+    // component if rendering suspends. Because the lifetime of the pooled
+    // cache is distinct from the main memoizedState.cache, it must be
+    // retained separately.
     root.pooledCache = initialCache;
     retainCache(initialCache);
     const initialState = {
diff --git a/packages/react-reconciler/src/ReactFiberRoot.old.js b/packages/react-reconciler/src/ReactFiberRoot.old.js
index 180d500c01f..504dac966ef 100644
--- a/packages/react-reconciler/src/ReactFiberRoot.old.js
+++ b/packages/react-reconciler/src/ReactFiberRoot.old.js
@@ -121,13 +121,13 @@ export function createFiberRoot(
     const initialCache = createCache();
     retainCache(initialCache);
 
-    // The pooledCache is a fresh cache instance is being used for newly
-    // mounted boundaries during a render. In general, the pooledCache is
-    // always cleared from the root at the end of a render: it is either
-    // released when render commits, or moved to an Offscreen component
-    // if rendering suspends. Because the lifetime of the pooled cache
-    // is distinct from the main memoizedState.cache, it must be retained
-    // separately.
+    // The pooledCache is a fresh cache instance that is used temporarily
+    // for newly mounted boundaries during a render. In general, the
+    // pooledCache is always cleared from the root at the end of a render:
+    // it is either released when render commits, or moved to an Offscreen
+    // component if rendering suspends. Because the lifetime of the pooled
+    // cache is distinct from the main memoizedState.cache, it must be
+    // retained separately.
     root.pooledCache = initialCache;
     retainCache(initialCache);
     const initialState = {
diff --git a/packages/react-reconciler/src/__tests__/ReactCache-test.js b/packages/react-reconciler/src/__tests__/ReactCache-test.js
index fbcfb3df353..8901ea62122 100644
--- a/packages/react-reconciler/src/__tests__/ReactCache-test.js
+++ b/packages/react-reconciler/src/__tests__/ReactCache-test.js
@@ -1510,7 +1510,7 @@ describe('ReactCache', () => {
   });
 
   // @gate experimental || www
-  test.skip('fresh cache for root refresh is released if the refresh never commits', async () => {
+  test.skip('if a root cache refresh never commits its fresh cache is released', async () => {
     const root = ReactNoop.createRoot();
     let refresh;
     function Example({text}) {
@@ -1552,7 +1552,7 @@ describe('ReactCache', () => {
   });
 
   // @gate experimental || www
-  test.skip('fresh cache for cache boundary refresh is released if the refresh never commits', async () => {
+  test.skip('if a cache boundary refresh never commits its fresh cache is released', async () => {
     const root = ReactNoop.createRoot();
     let refresh;
     function Example({text}) {
@@ -1580,6 +1580,7 @@ describe('ReactCache', () => {
     expect(Scheduler).toHaveYielded(['Cache miss! [A]']);
     expect(root).toMatchRenderedOutput('A [v1]');
 
+    // Unmount the boundary before the refresh can complete
     await act(async () => {
       root.render('Bye!');
     });
diff --git a/packages/react-reconciler/src/__tests__/StrictEffectsModeDefaults-test.internal.js b/packages/react-reconciler/src/__tests__/StrictEffectsModeDefaults-test.internal.js
index e443aa0a231..a725f12c663 100644
--- a/packages/react-reconciler/src/__tests__/StrictEffectsModeDefaults-test.internal.js
+++ b/packages/react-reconciler/src/__tests__/StrictEffectsModeDefaults-test.internal.js
@@ -100,8 +100,6 @@ describe('StrictEffectsMode defaults', () => {
 
         expect(Scheduler).toFlushUntilNextPaint([
           'useLayoutEffect mount "one"',
-          'useLayoutEffect unmount "one"',
-          'useLayoutEffect mount "one"',
         ]);
       });
 
@@ -113,15 +111,16 @@ describe('StrictEffectsMode defaults', () => {
           </>,
         );
 
+        expect(Scheduler).toHaveYielded([
+          // Since "two" is new, it should be double-invoked.
+          'useLayoutEffect unmount "one"',
+          'useLayoutEffect mount "one"',
+        ]);
         expect(Scheduler).toFlushUntilNextPaint([
           // Cleanup and re-run "one" (and "two") since there is no dependencies array.
           'useLayoutEffect unmount "one"',
           'useLayoutEffect mount "one"',
           'useLayoutEffect mount "two"',
-
-          // Since "two" is new, it should be double-invoked.
-          'useLayoutEffect unmount "two"',
-          'useLayoutEffect mount "two"',
         ]);
       });
     });