From b13ea9e7daef3f272cdf51ae4b277d78eb2757a1 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 14 Dec 2020 17:11:02 -0600 Subject: [PATCH] Refreshes should not affect "sibling" boundaries I had thought we decided that refreshing a boundary would also refresh all the content that is currently consistent (i.e. shared the same underlying cache) with it, but I was wrong. Refreshing should only affect the nearest tree and its descendents. "Sibling" content will intentionally be inconsistent after the refresh. This allows me to drop the subscription stuff, which is nice. --- .../src/ReactFiberBeginWork.new.js | 45 +++++---- .../src/ReactFiberBeginWork.old.js | 45 +++++---- .../src/ReactFiberCacheComponent.js | 10 +- .../src/ReactFiberCommitWork.new.js | 26 +----- .../src/ReactFiberCommitWork.old.js | 26 +----- .../src/ReactFiberCompleteWork.new.js | 27 ++---- .../src/ReactFiberCompleteWork.old.js | 27 ++---- .../src/ReactFiberHooks.new.js | 93 +++++++++---------- .../src/ReactFiberHooks.old.js | 93 +++++++++---------- .../src/ReactFiberLane.new.js | 5 +- .../src/ReactFiberLane.old.js | 5 +- .../src/ReactFiberUnwindWork.new.js | 11 ++- .../src/ReactFiberUnwindWork.old.js | 11 ++- .../src/__tests__/ReactCache-test.js | 16 ++-- 14 files changed, 178 insertions(+), 262 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index b0eb4dc830cc9..8a0397938a221 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -23,7 +23,7 @@ import type { OffscreenProps, OffscreenState, } from './ReactFiberOffscreenComponent'; -import type {Cache} from './ReactFiberCacheComponent'; +import type {CacheInstance} from './ReactFiberCacheComponent'; import checkPropTypes from 'shared/checkPropTypes'; @@ -667,17 +667,17 @@ function updateCacheComponent( // Read directly from the context. We don't set up a context dependency // because the propagation function automatically includes CacheComponents in // its search. - const parentCache: Cache | null = isPrimaryRenderer + const parentCache: CacheInstance | null = isPrimaryRenderer ? CacheContext._currentValue : CacheContext._currentValue2; - let ownCache: Cache | null = null; + let ownCacheInstance: CacheInstance | null = null; // TODO: Fast path if parent has a new cache. Merely an optimization. Might // not be worth it. if (false) { // The parent boundary also has a new cache. We're either inside a new tree, // or there was a refresh. In both cases, we should use the parent cache. - ownCache = null; + ownCacheInstance = null; } else { if (current === null) { // This is a newly mounted component. Request a fresh cache. @@ -692,12 +692,15 @@ function updateCacheComponent( // spawned from a previous render that already committed. Otherwise, this // is the root of a cache consistency boundary. if (freshCache !== parentCache) { - ownCache = freshCache; - pushProvider(workInProgress, CacheContext, freshCache); + ownCacheInstance = { + cache: freshCache, + provider: workInProgress, + }; + pushProvider(workInProgress, CacheContext, ownCacheInstance); // No need to propagate the refresh, because this is a new tree. } else { // Use the parent cache - ownCache = null; + ownCacheInstance = null; } } else { // This component already mounted. @@ -711,8 +714,11 @@ function updateCacheComponent( ); const freshCache = requestFreshCache(root, renderLanes); if (freshCache !== parentCache) { - ownCache = freshCache; - pushProvider(workInProgress, CacheContext, freshCache); + ownCacheInstance = { + cache: freshCache, + provider: workInProgress, + }; + pushProvider(workInProgress, CacheContext, ownCacheInstance); // Refreshes propagate through the entire subtree. The refreshed cache // will override nested caches. propagateCacheRefresh(workInProgress, renderLanes); @@ -721,17 +727,17 @@ function updateCacheComponent( // unreachable in practice, because this means the parent cache was // refreshed in the same render. So we would have already handled this // in the earlier branch, where we check if the parent is new. - ownCache = null; + ownCacheInstance = null; } } else { // Reuse the memoized cache. - const prevCache: Cache | null = current.memoizedState; - if (prevCache !== null) { - ownCache = prevCache; + const prevCacheInstance: CacheInstance | null = current.memoizedState; + if (prevCacheInstance !== null) { + ownCacheInstance = prevCacheInstance; // There was no refresh, so no need to propagate to nested boundaries. - pushProvider(workInProgress, CacheContext, prevCache); + pushProvider(workInProgress, CacheContext, ownCacheInstance); } else { - ownCache = null; + ownCacheInstance = null; } } } @@ -741,7 +747,7 @@ function updateCacheComponent( // point to a cache object. Otherwise, a null state indicates that this // CacheComponent inherits from a parent boundary. We can use this to infer // whether to push/pop the cache context. - workInProgress.memoizedState = ownCache; + workInProgress.memoizedState = ownCacheInstance; const nextChildren = workInProgress.pendingProps.children; reconcileChildren(current, workInProgress, nextChildren, renderLanes); @@ -3337,9 +3343,10 @@ function beginWork( } case CacheComponent: { if (enableCache) { - const ownCache: Cache | null = workInProgress.memoizedState; - if (ownCache !== null) { - pushProvider(workInProgress, CacheContext, ownCache); + const ownCacheInstance: CacheInstance | null = + workInProgress.memoizedState; + if (ownCacheInstance !== null) { + pushProvider(workInProgress, CacheContext, ownCacheInstance); } } break; diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index 55ca3dbcb9c31..d27df29088c4b 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -23,7 +23,7 @@ import type { OffscreenProps, OffscreenState, } from './ReactFiberOffscreenComponent'; -import type {Cache} from './ReactFiberCacheComponent'; +import type {CacheInstance} from './ReactFiberCacheComponent'; import checkPropTypes from 'shared/checkPropTypes'; @@ -667,17 +667,17 @@ function updateCacheComponent( // Read directly from the context. We don't set up a context dependency // because the propagation function automatically includes CacheComponents in // its search. - const parentCache: Cache | null = isPrimaryRenderer + const parentCache: CacheInstance | null = isPrimaryRenderer ? CacheContext._currentValue : CacheContext._currentValue2; - let ownCache: Cache | null = null; + let ownCacheInstance: CacheInstance | null = null; // TODO: Fast path if parent has a new cache. Merely an optimization. Might // not be worth it. if (false) { // The parent boundary also has a new cache. We're either inside a new tree, // or there was a refresh. In both cases, we should use the parent cache. - ownCache = null; + ownCacheInstance = null; } else { if (current === null) { // This is a newly mounted component. Request a fresh cache. @@ -692,12 +692,15 @@ function updateCacheComponent( // spawned from a previous render that already committed. Otherwise, this // is the root of a cache consistency boundary. if (freshCache !== parentCache) { - ownCache = freshCache; - pushProvider(workInProgress, CacheContext, freshCache); + ownCacheInstance = { + cache: freshCache, + provider: workInProgress, + }; + pushProvider(workInProgress, CacheContext, ownCacheInstance); // No need to propagate the refresh, because this is a new tree. } else { // Use the parent cache - ownCache = null; + ownCacheInstance = null; } } else { // This component already mounted. @@ -711,8 +714,11 @@ function updateCacheComponent( ); const freshCache = requestFreshCache(root, renderLanes); if (freshCache !== parentCache) { - ownCache = freshCache; - pushProvider(workInProgress, CacheContext, freshCache); + ownCacheInstance = { + cache: freshCache, + provider: workInProgress, + }; + pushProvider(workInProgress, CacheContext, ownCacheInstance); // Refreshes propagate through the entire subtree. The refreshed cache // will override nested caches. propagateCacheRefresh(workInProgress, renderLanes); @@ -721,17 +727,17 @@ function updateCacheComponent( // unreachable in practice, because this means the parent cache was // refreshed in the same render. So we would have already handled this // in the earlier branch, where we check if the parent is new. - ownCache = null; + ownCacheInstance = null; } } else { // Reuse the memoized cache. - const prevCache: Cache | null = current.memoizedState; - if (prevCache !== null) { - ownCache = prevCache; + const prevCacheInstance: CacheInstance | null = current.memoizedState; + if (prevCacheInstance !== null) { + ownCacheInstance = prevCacheInstance; // There was no refresh, so no need to propagate to nested boundaries. - pushProvider(workInProgress, CacheContext, prevCache); + pushProvider(workInProgress, CacheContext, ownCacheInstance); } else { - ownCache = null; + ownCacheInstance = null; } } } @@ -741,7 +747,7 @@ function updateCacheComponent( // point to a cache object. Otherwise, a null state indicates that this // CacheComponent inherits from a parent boundary. We can use this to infer // whether to push/pop the cache context. - workInProgress.memoizedState = ownCache; + workInProgress.memoizedState = ownCacheInstance; const nextChildren = workInProgress.pendingProps.children; reconcileChildren(current, workInProgress, nextChildren, renderLanes); @@ -3337,9 +3343,10 @@ function beginWork( } case CacheComponent: { if (enableCache) { - const ownCache: Cache | null = workInProgress.memoizedState; - if (ownCache !== null) { - pushProvider(workInProgress, CacheContext, ownCache); + const ownCacheInstance: CacheInstance | null = + workInProgress.memoizedState; + if (ownCacheInstance !== null) { + pushProvider(workInProgress, CacheContext, ownCacheInstance); } } break; diff --git a/packages/react-reconciler/src/ReactFiberCacheComponent.js b/packages/react-reconciler/src/ReactFiberCacheComponent.js index 36ae44e1a3f97..5b64e5c42fa27 100644 --- a/packages/react-reconciler/src/ReactFiberCacheComponent.js +++ b/packages/react-reconciler/src/ReactFiberCacheComponent.js @@ -11,12 +11,14 @@ import type {ReactContext} from 'shared/ReactTypes'; import {REACT_CONTEXT_TYPE} from 'shared/ReactSymbols'; -export type Cache = {| - providers: Set | null, - data: Map<() => mixed, mixed> | null, +export type Cache = Map<() => mixed, mixed>; + +export type CacheInstance = {| + cache: Cache | null, + provider: Fiber, |}; -export const CacheContext: ReactContext = { +export const CacheContext: ReactContext = { $$typeof: REACT_CONTEXT_TYPE, // We don't use Consumer/Provider for Cache components. So we'll cheat. Consumer: (null: any), diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index 28f2f2a2be3be..d0f98cc87c54f 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -36,7 +36,6 @@ import { enableFundamentalAPI, enableSuspenseCallback, enableScopeAPI, - enableCache, } from 'shared/ReactFeatureFlags'; import { FunctionComponent, @@ -793,31 +792,8 @@ function commitLifeCycles( case ScopeComponent: case OffscreenComponent: case LegacyHiddenComponent: + case CacheComponent: return; - case CacheComponent: { - if (enableCache) { - if (current !== null) { - const oldCache: Cache | null = current.memoizedState; - if (oldCache !== null) { - const oldCacheProviders = oldCache.providers; - if (oldCacheProviders) { - oldCacheProviders.delete(current); - oldCacheProviders.delete(finishedWork); - } - } - } - const newCache: Cache | null = finishedWork.memoizedState; - if (newCache !== null) { - const newCacheProviders = newCache.providers; - if (newCacheProviders === null) { - newCache.providers = new Set([finishedWork]); - } else { - newCacheProviders.add(finishedWork); - } - } - } - return; - } } invariant( false, diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index d46759f7c9334..2b37c649350b5 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -37,7 +37,6 @@ import { enableSuspenseCallback, enableScopeAPI, enableDoubleInvokingEffects, - enableCache, } from 'shared/ReactFeatureFlags'; import { FunctionComponent, @@ -794,31 +793,8 @@ function commitLifeCycles( case ScopeComponent: case OffscreenComponent: case LegacyHiddenComponent: + case CacheComponent: return; - case CacheComponent: { - if (enableCache) { - if (current !== null) { - const oldCache: Cache | null = current.memoizedState; - if (oldCache !== null) { - const oldCacheProviders = oldCache.providers; - if (oldCacheProviders) { - oldCacheProviders.delete(current); - oldCacheProviders.delete(finishedWork); - } - } - } - const newCache: Cache | null = finishedWork.memoizedState; - if (newCache !== null) { - const newCacheProviders = newCache.providers; - if (newCacheProviders === null) { - newCache.providers = new Set([finishedWork]); - } else { - newCacheProviders.add(finishedWork); - } - } - } - return; - } } invariant( false, diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js index 3aa000e4ca810..14dc69d01e658 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.new.js @@ -28,6 +28,7 @@ import type { } from './ReactFiberSuspenseComponent.new'; import type {SuspenseContext} from './ReactFiberSuspenseContext.new'; import type {OffscreenState} from './ReactFiberOffscreenComponent'; +import type {CacheInstance} from './ReactFiberCacheComponent'; import {resetWorkInProgressVersions as resetMutableSourceWorkInProgressVersions} from './ReactMutableSource.new'; @@ -1488,27 +1489,11 @@ function completeWork( } case CacheComponent: { if (enableCache) { - // If the cache provided by this boundary has changed, schedule an - // effect to add this component to the cache's providers, and to remove - // it from the old cache. - // TODO: Schedule for Passive phase - const ownCache: Cache | null = workInProgress.memoizedState; - if (current === null) { - if (ownCache !== null) { - // This is a cache provider. - popProvider(CacheContext, workInProgress); - // Set up a refresh subscription. - workInProgress.flags |= Update; - } - } else { - if (ownCache !== null) { - // This is a cache provider. - popProvider(CacheContext, workInProgress); - } - if (ownCache !== current.memoizedState) { - // Cache changed. Create or update a refresh subscription. - workInProgress.flags |= Update; - } + const ownCacheInstance: CacheInstance | null = + workInProgress.memoizedState; + if (ownCacheInstance !== null) { + // This is a cache provider. + popProvider(CacheContext, workInProgress); } bubbleProperties(workInProgress); return null; diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.old.js b/packages/react-reconciler/src/ReactFiberCompleteWork.old.js index b20910ddc5ece..ae6bc84df5372 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.old.js @@ -28,6 +28,7 @@ import type { } from './ReactFiberSuspenseComponent.old'; import type {SuspenseContext} from './ReactFiberSuspenseContext.old'; import type {OffscreenState} from './ReactFiberOffscreenComponent'; +import type {CacheInstance} from './ReactFiberCacheComponent'; import {resetWorkInProgressVersions as resetMutableSourceWorkInProgressVersions} from './ReactMutableSource.old'; @@ -1488,27 +1489,11 @@ function completeWork( } case CacheComponent: { if (enableCache) { - // If the cache provided by this boundary has changed, schedule an - // effect to add this component to the cache's providers, and to remove - // it from the old cache. - // TODO: Schedule for Passive phase - const ownCache: Cache | null = workInProgress.memoizedState; - if (current === null) { - if (ownCache !== null) { - // This is a cache provider. - popProvider(CacheContext, workInProgress); - // Set up a refresh subscription. - workInProgress.flags |= Update; - } - } else { - if (ownCache !== null) { - // This is a cache provider. - popProvider(CacheContext, workInProgress); - } - if (ownCache !== current.memoizedState) { - // Cache changed. Create or update a refresh subscription. - workInProgress.flags |= Update; - } + const ownCacheInstance: CacheInstance | null = + workInProgress.memoizedState; + if (ownCacheInstance !== null) { + // This is a cache provider. + popProvider(CacheContext, workInProgress); } bubbleProperties(workInProgress); return null; diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index c2c1098363301..aecbc15b60eaa 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -19,7 +19,7 @@ import type {HookFlags} from './ReactHookEffectTags'; import type {ReactPriorityLevel} from './ReactInternalTypes'; import type {FiberRoot} from './ReactInternalTypes'; import type {OpaqueIDType} from './ReactFiberHostConfig'; -import type {Cache} from './ReactFiberCacheComponent'; +import type {CacheInstance} from './ReactFiberCacheComponent'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import { @@ -1710,57 +1710,48 @@ function rerenderOpaqueIdentifier(): OpaqueIDType | void { } function mountRefresh() { - const cache: Cache | null = readContext(CacheContext); - return mountCallback(refreshCache.bind(null, cache), [cache]); + // TODO: CacheInstance should never be null. Update type. + const cacheInstance: CacheInstance | null = readContext(CacheContext); + return mountCallback(refreshCache.bind(null, cacheInstance), [cacheInstance]); } function updateRefresh() { - const cache: Cache | null = readContext(CacheContext); - return updateCallback(refreshCache.bind(null, cache), [cache]); + const cacheInstance: CacheInstance | null = readContext(CacheContext); + return updateCallback(refreshCache.bind(null, cacheInstance), [ + cacheInstance, + ]); } -function refreshCache(cache: Cache | null, seedKey: ?() => T, seedValue: T) { - if (cache !== null) { - const providers = cache.providers; - if (providers !== null) { - let seededCache = null; - if (seedKey !== null && seedKey !== undefined) { +function refreshCache( + cacheInstance: CacheInstance | null, + seedKey: ?() => T, + seedValue: T, +) { + if (cacheInstance !== null) { + const provider = cacheInstance.provider; + + // Inlined startTransition + // TODO: Maybe we shouldn't automatically give this transition priority. Are + // there valid use cases for a high-pri refresh? Like if the content is + // super stale and you want to immediately hide it. + const prevTransition = ReactCurrentBatchConfig.transition; + ReactCurrentBatchConfig.transition = 1; + // TODO: Do we really need the try/finally? I don't think any of these + // functions would ever throw unless there's an internal error. + try { + const eventTime = requestEventTime(); + const lane = requestUpdateLane(provider); + const root = scheduleUpdateOnFiber(provider, lane, eventTime); + if (seedKey !== null && seedKey !== undefined && root !== null) { // TODO: Warn if wrong type - seededCache = { - providers: null, - data: new Map([[seedKey, seedValue]]), - }; + const seededCache = new Map([[seedKey, seedValue]]); + transferCacheToSpawnedLane(root, seededCache, lane); } - providers.forEach(provider => - scheduleCacheRefresh(provider, seededCache), - ); + } finally { + ReactCurrentBatchConfig.transition = prevTransition; } } else { - // TODO: Warn if cache is null? - } -} - -function scheduleCacheRefresh( - cacheComponentFiber: Fiber, - seededCache: Cache | null, -) { - // Inlined startTransition - // TODO: Maybe we shouldn't automatically give this transition priority. Are - // there valid use cases for a high-pri refresh? Like if the content is - // super stale and you want to immediately hide it. - const prevTransition = ReactCurrentBatchConfig.transition; - ReactCurrentBatchConfig.transition = 1; - // TODO: Do we really need the try/finally? I don't think any of these - // functions would ever throw unless there's an internal error. - try { - const eventTime = requestEventTime(); - const lane = requestUpdateLane(cacheComponentFiber); - const root = scheduleUpdateOnFiber(cacheComponentFiber, lane, eventTime); - if (seededCache !== null && root !== null) { - transferCacheToSpawnedLane(root, seededCache, lane); - } - } finally { - ReactCurrentBatchConfig.transition = prevTransition; + // TODO: CacheInstance should never be null. Update type. } } @@ -1875,27 +1866,27 @@ function dispatchAction( } function getCacheForType(resourceType: () => T): T { - const cache: Cache | null = readContext(CacheContext); + const cacheInstance: CacheInstance | null = readContext(CacheContext); invariant( - cache !== null, + cacheInstance !== null, 'Tried to fetch data, but no cache was found. To fix, wrap your ' + "component in a boundary. It doesn't need to be a direct " + 'parent; it can be anywhere in the ancestor path', ); - let cachesByType = cache.data; - if (cachesByType === null) { - cachesByType = cache.data = new Map(); + let cache = cacheInstance.cache; + if (cache === null) { + cache = cacheInstance.cache = new Map(); // TODO: Warn if constructor returns undefined? Creates ambiguity with // existence check above. (I don't want to use `has`. Two map lookups // instead of one? Silly.) const cacheForType = resourceType(); - cachesByType.set(resourceType, cacheForType); + cache.set(resourceType, cacheForType); return cacheForType; } else { - let cacheForType: T | void = (cachesByType.get(resourceType): any); + let cacheForType: T | void = (cache.get(resourceType): any); if (cacheForType === undefined) { cacheForType = resourceType(); - cachesByType.set(resourceType, cacheForType); + cache.set(resourceType, cacheForType); } return cacheForType; } diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index b962af97401ed..6122a77b951ec 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -19,7 +19,7 @@ import type {HookFlags} from './ReactHookEffectTags'; import type {ReactPriorityLevel} from './ReactInternalTypes'; import type {FiberRoot} from './ReactInternalTypes'; import type {OpaqueIDType} from './ReactFiberHostConfig'; -import type {Cache} from './ReactFiberCacheComponent'; +import type {CacheInstance} from './ReactFiberCacheComponent'; import ReactSharedInternals from 'shared/ReactSharedInternals'; import { @@ -1781,57 +1781,48 @@ function rerenderOpaqueIdentifier(): OpaqueIDType | void { } function mountRefresh() { - const cache: Cache | null = readContext(CacheContext); - return mountCallback(refreshCache.bind(null, cache), [cache]); + // TODO: CacheInstance should never be null. Update type. + const cacheInstance: CacheInstance | null = readContext(CacheContext); + return mountCallback(refreshCache.bind(null, cacheInstance), [cacheInstance]); } function updateRefresh() { - const cache: Cache | null = readContext(CacheContext); - return updateCallback(refreshCache.bind(null, cache), [cache]); + const cacheInstance: CacheInstance | null = readContext(CacheContext); + return updateCallback(refreshCache.bind(null, cacheInstance), [ + cacheInstance, + ]); } -function refreshCache(cache: Cache | null, seedKey: ?() => T, seedValue: T) { - if (cache !== null) { - const providers = cache.providers; - if (providers !== null) { - let seededCache = null; - if (seedKey !== null && seedKey !== undefined) { +function refreshCache( + cacheInstance: CacheInstance | null, + seedKey: ?() => T, + seedValue: T, +) { + if (cacheInstance !== null) { + const provider = cacheInstance.provider; + + // Inlined startTransition + // TODO: Maybe we shouldn't automatically give this transition priority. Are + // there valid use cases for a high-pri refresh? Like if the content is + // super stale and you want to immediately hide it. + const prevTransition = ReactCurrentBatchConfig.transition; + ReactCurrentBatchConfig.transition = 1; + // TODO: Do we really need the try/finally? I don't think any of these + // functions would ever throw unless there's an internal error. + try { + const eventTime = requestEventTime(); + const lane = requestUpdateLane(provider); + const root = scheduleUpdateOnFiber(provider, lane, eventTime); + if (seedKey !== null && seedKey !== undefined && root !== null) { // TODO: Warn if wrong type - seededCache = { - providers: null, - data: new Map([[seedKey, seedValue]]), - }; + const seededCache = new Map([[seedKey, seedValue]]); + transferCacheToSpawnedLane(root, seededCache, lane); } - providers.forEach(provider => - scheduleCacheRefresh(provider, seededCache), - ); + } finally { + ReactCurrentBatchConfig.transition = prevTransition; } } else { - // TODO: Warn if cache is null? - } -} - -function scheduleCacheRefresh( - cacheComponentFiber: Fiber, - seededCache: Cache | null, -) { - // Inlined startTransition - // TODO: Maybe we shouldn't automatically give this transition priority. Are - // there valid use cases for a high-pri refresh? Like if the content is - // super stale and you want to immediately hide it. - const prevTransition = ReactCurrentBatchConfig.transition; - ReactCurrentBatchConfig.transition = 1; - // TODO: Do we really need the try/finally? I don't think any of these - // functions would ever throw unless there's an internal error. - try { - const eventTime = requestEventTime(); - const lane = requestUpdateLane(cacheComponentFiber); - const root = scheduleUpdateOnFiber(cacheComponentFiber, lane, eventTime); - if (seededCache !== null && root !== null) { - transferCacheToSpawnedLane(root, seededCache, lane); - } - } finally { - ReactCurrentBatchConfig.transition = prevTransition; + // TODO: CacheInstance should never be null. Update type. } } @@ -1946,27 +1937,27 @@ function dispatchAction( } function getCacheForType(resourceType: () => T): T { - const cache: Cache | null = readContext(CacheContext); + const cacheInstance: CacheInstance | null = readContext(CacheContext); invariant( - cache !== null, + cacheInstance !== null, 'Tried to fetch data, but no cache was found. To fix, wrap your ' + "component in a boundary. It doesn't need to be a direct " + 'parent; it can be anywhere in the ancestor path', ); - let cachesByType = cache.data; - if (cachesByType === null) { - cachesByType = cache.data = new Map(); + let cache = cacheInstance.cache; + if (cache === null) { + cache = cacheInstance.cache = new Map(); // TODO: Warn if constructor returns undefined? Creates ambiguity with // existence check above. (I don't want to use `has`. Two map lookups // instead of one? Silly.) const cacheForType = resourceType(); - cachesByType.set(resourceType, cacheForType); + cache.set(resourceType, cacheForType); return cacheForType; } else { - let cacheForType: T | void = (cachesByType.get(resourceType): any); + let cacheForType: T | void = (cache.get(resourceType): any); if (cacheForType === undefined) { cacheForType = resourceType(); - cachesByType.set(resourceType, cacheForType); + cache.set(resourceType, cacheForType); } return cacheForType; } diff --git a/packages/react-reconciler/src/ReactFiberLane.new.js b/packages/react-reconciler/src/ReactFiberLane.new.js index 8acfdade7df0a..b82f49290f014 100644 --- a/packages/react-reconciler/src/ReactFiberLane.new.js +++ b/packages/react-reconciler/src/ReactFiberLane.new.js @@ -861,10 +861,7 @@ export function requestFreshCache(root: FiberRoot, renderLanes: Lanes): Cache { } // Create a fresh cache. - const freshCache = { - providers: null, - data: null, - }; + const freshCache = new Map(); // This is now the pooled cache. root.pooledCache = freshCache; diff --git a/packages/react-reconciler/src/ReactFiberLane.old.js b/packages/react-reconciler/src/ReactFiberLane.old.js index 4b07ae1f0e7f2..444b5ef425ea1 100644 --- a/packages/react-reconciler/src/ReactFiberLane.old.js +++ b/packages/react-reconciler/src/ReactFiberLane.old.js @@ -861,10 +861,7 @@ export function requestFreshCache(root: FiberRoot, renderLanes: Lanes): Cache { } // Create a fresh cache. - const freshCache = { - providers: null, - data: null, - }; + const freshCache = new Map(); // This is now the pooled cache. root.pooledCache = freshCache; diff --git a/packages/react-reconciler/src/ReactFiberUnwindWork.new.js b/packages/react-reconciler/src/ReactFiberUnwindWork.new.js index eb480177eb570..da9741be3b05c 100644 --- a/packages/react-reconciler/src/ReactFiberUnwindWork.new.js +++ b/packages/react-reconciler/src/ReactFiberUnwindWork.new.js @@ -11,6 +11,7 @@ import type {ReactContext} from 'shared/ReactTypes'; import type {Fiber} from './ReactInternalTypes'; import type {Lanes} from './ReactFiberLane.new'; import type {SuspenseState} from './ReactFiberSuspenseComponent.new'; +import type {CacheInstance} from './ReactFiberCacheComponent'; import {resetWorkInProgressVersions as resetMutableSourceWorkInProgressVersions} from './ReactMutableSource.new'; import { @@ -133,8 +134,9 @@ function unwindWork(workInProgress: Fiber, renderLanes: Lanes) { return null; case CacheComponent: if (enableCache) { - const ownCache: Cache | null = workInProgress.memoizedState; - if (ownCache !== null) { + const ownCacheInstance: CacheInstance | null = + workInProgress.memoizedState; + if (ownCacheInstance !== null) { popProvider(CacheContext, workInProgress); } } @@ -182,8 +184,9 @@ function unwindInterruptedWork(interruptedWork: Fiber) { break; case CacheComponent: if (enableCache) { - const ownCache: Cache | null = interruptedWork.memoizedState; - if (ownCache !== null) { + const ownCacheInstance: CacheInstance | null = + interruptedWork.memoizedState; + if (ownCacheInstance !== null) { popProvider(CacheContext, interruptedWork); } } diff --git a/packages/react-reconciler/src/ReactFiberUnwindWork.old.js b/packages/react-reconciler/src/ReactFiberUnwindWork.old.js index 6827ac7ccf0ac..5237b0c45b5ff 100644 --- a/packages/react-reconciler/src/ReactFiberUnwindWork.old.js +++ b/packages/react-reconciler/src/ReactFiberUnwindWork.old.js @@ -11,6 +11,7 @@ import type {ReactContext} from 'shared/ReactTypes'; import type {Fiber} from './ReactInternalTypes'; import type {Lanes} from './ReactFiberLane.old'; import type {SuspenseState} from './ReactFiberSuspenseComponent.old'; +import type {CacheInstance} from './ReactFiberCacheComponent'; import {resetWorkInProgressVersions as resetMutableSourceWorkInProgressVersions} from './ReactMutableSource.old'; import { @@ -133,8 +134,9 @@ function unwindWork(workInProgress: Fiber, renderLanes: Lanes) { return null; case CacheComponent: if (enableCache) { - const ownCache: Cache | null = workInProgress.memoizedState; - if (ownCache !== null) { + const ownCacheInstance: CacheInstance | null = + workInProgress.memoizedState; + if (ownCacheInstance !== null) { popProvider(CacheContext, workInProgress); } } @@ -182,8 +184,9 @@ function unwindInterruptedWork(interruptedWork: Fiber) { break; case CacheComponent: if (enableCache) { - const ownCache: Cache | null = interruptedWork.memoizedState; - if (ownCache !== null) { + const ownCacheInstance: CacheInstance | null = + interruptedWork.memoizedState; + if (ownCacheInstance !== null) { popProvider(CacheContext, interruptedWork); } } diff --git a/packages/react-reconciler/src/__tests__/ReactCache-test.js b/packages/react-reconciler/src/__tests__/ReactCache-test.js index a933fda928d8c..18d4f59710d2d 100644 --- a/packages/react-reconciler/src/__tests__/ReactCache-test.js +++ b/packages/react-reconciler/src/__tests__/ReactCache-test.js @@ -529,7 +529,7 @@ describe('ReactCache', () => { // @gate experimental test( - 'refreshing a cache boundary also refreshes the other boundaries ' + + 'refreshing a cache boundary does not refresh the other boundaries ' + 'that mounted at the same time (i.e. the ones that share the same cache)', async () => { let refreshFirstBoundary; @@ -575,23 +575,19 @@ describe('ReactCache', () => { expect(Scheduler).toHaveYielded(['A [v1]', 'A [v1]']); expect(root).toMatchRenderedOutput('A [v1]A [v1]'); - // Refresh the first boundary. It should also refresh the second boundary, - // since they appeared at the same time. + // Refresh the first boundary. It should not refresh the second boundary, + // even though they previously shared the same underlying cache. mutateRemoteTextService(); await ReactNoop.act(async () => { await refreshFirstBoundary(); }); - expect(Scheduler).toHaveYielded([ - 'Cache miss! [A]', - 'Loading...', - 'Loading...', - ]); + expect(Scheduler).toHaveYielded(['Cache miss! [A]', 'Loading...']); await ReactNoop.act(async () => { await resolveText('A'); }); - expect(Scheduler).toHaveYielded(['A [v2]', 'A [v2]']); - expect(root).toMatchRenderedOutput('A [v2]A [v2]'); + expect(Scheduler).toHaveYielded(['A [v2]']); + expect(root).toMatchRenderedOutput('A [v2]A [v1]'); }, ); });