From 258b375a419098a5b6c3568e8d35046d142ea912 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 25 Feb 2021 16:42:11 -0600 Subject: [PATCH 1/5] Move context comparison to consumer In the lazy context implementation, not all context changes are propagated from the provider, so we can't rely on the propagation alone to mark the consumer as dirty. The consumer needs to compare to the previous value, like we do for state and context. I added a `memoizedValue` field to the context dependency type. Then in the consumer, we iterate over the current dependencies to see if something changed. We only do this iteration after props and state has already bailed out, so it's a relatively uncommon path, except at the root of a changed subtree. Alternatively, we could move these comparisons into `readContext`, but that's a much hotter path, so I think this is an appropriate trade off. --- ...eactLegacyContextDisabled-test.internal.js | 10 +- .../src/ReactFiberBeginWork.new.js | 4 + .../src/ReactFiberBeginWork.old.js | 4 + .../src/ReactFiberClassComponent.new.js | 21 +- .../src/ReactFiberClassComponent.old.js | 21 +- .../src/ReactFiberHooks.new.js | 29 ++- .../src/ReactFiberHooks.old.js | 29 ++- .../src/ReactFiberNewContext.new.js | 123 ++++++++--- .../src/ReactFiberNewContext.old.js | 123 ++++++++--- .../src/ReactInternalTypes.js | 1 + .../__tests__/ReactContextPropagation-test.js | 206 ++++++++++++++++++ .../__tests__/ReactContextValidator-test.js | 8 +- packages/shared/ReactFeatureFlags.js | 2 + .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 1 + .../ReactFeatureFlags.test-renderer.native.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 1 + .../shared/forks/ReactFeatureFlags.testing.js | 1 + .../forks/ReactFeatureFlags.testing.www.js | 1 + .../forks/ReactFeatureFlags.www-dynamic.js | 1 + .../shared/forks/ReactFeatureFlags.www.js | 1 + 22 files changed, 507 insertions(+), 83 deletions(-) create mode 100644 packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js diff --git a/packages/react-dom/src/__tests__/ReactLegacyContextDisabled-test.internal.js b/packages/react-dom/src/__tests__/ReactLegacyContextDisabled-test.internal.js index c34268924ffbb..c6a2d1fbb5aa1 100644 --- a/packages/react-dom/src/__tests__/ReactLegacyContextDisabled-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactLegacyContextDisabled-test.internal.js @@ -226,7 +226,15 @@ describe('ReactLegacyContextDisabled', () => { container, ); expect(container.textContent).toBe('bbb'); - expect(lifecycleContextLog).toEqual(['b', 'b']); // sCU skipped due to changed context value. + if (gate(flags => flags.enableLazyContextPropagation)) { + // In the lazy propagation implementation, we don't check if context + // changed until after shouldComponentUpdate is run. + expect(lifecycleContextLog).toEqual(['b', 'b', 'b']); + } else { + // In the eager implementation, a dirty flag was set when the parent + // changed, so we skipped sCU. + expect(lifecycleContextLog).toEqual(['b', 'b']); + } ReactDOM.unmountComponentAtNode(container); }); }); diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 69149016f5ec0..66584784ee379 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -3074,6 +3074,10 @@ export function markWorkInProgressReceivedUpdate() { didReceiveUpdate = true; } +export function checkIfWorkInProgressReceivedUpdate() { + return didReceiveUpdate; +} + function bailoutOnAlreadyFinishedWork( current: Fiber | null, workInProgress: Fiber, diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index b8bacadcfe036..8faa4d859b7e0 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -3074,6 +3074,10 @@ export function markWorkInProgressReceivedUpdate() { didReceiveUpdate = true; } +export function checkIfWorkInProgressReceivedUpdate() { + return didReceiveUpdate; +} + function bailoutOnAlreadyFinishedWork( current: Fiber | null, workInProgress: Fiber, diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.new.js b/packages/react-reconciler/src/ReactFiberClassComponent.new.js index 0042bf2f93742..ea1b4cb5e3c8d 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.new.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.new.js @@ -20,6 +20,7 @@ import { enableSchedulingProfiler, warnAboutDeprecatedLifecycles, enableStrictEffects, + enableLazyContextPropagation, } from 'shared/ReactFeatureFlags'; import ReactStrictModeWarnings from './ReactStrictModeWarnings.new'; import {isMounted} from './ReactFiberTreeReflection'; @@ -58,7 +59,7 @@ import { hasContextChanged, emptyContextObject, } from './ReactFiberContext.new'; -import {readContext} from './ReactFiberNewContext.new'; +import {readContext, checkIfContextChanged} from './ReactFiberNewContext.new'; import { requestEventTime, requestUpdateLane, @@ -1150,7 +1151,13 @@ function updateClassInstance( unresolvedOldProps === unresolvedNewProps && oldState === newState && !hasContextChanged() && - !checkHasForceUpdateAfterProcessing() + !checkHasForceUpdateAfterProcessing() && + !( + enableLazyContextPropagation && + current !== null && + current.dependencies !== null && + checkIfContextChanged(current.dependencies) + ) ) { // If an update was already in progress, we should schedule an Update // effect even though we're bailing out, so that cWU/cDU are called. @@ -1193,7 +1200,15 @@ function updateClassInstance( oldState, newState, nextContext, - ); + ) || + // TODO: In some cases, we'll end up checking if context has changed twice, + // both before and after `shouldComponentUpdate` has been called. Not ideal, + // but I'm loath to refactor this function. This only happens for memoized + // components so it's not that common. + (enableLazyContextPropagation && + current !== null && + current.dependencies !== null && + checkIfContextChanged(current.dependencies)); if (shouldUpdate) { // In order to support react-lifecycles-compat polyfilled components, diff --git a/packages/react-reconciler/src/ReactFiberClassComponent.old.js b/packages/react-reconciler/src/ReactFiberClassComponent.old.js index 984960438672b..b952c24cd041d 100644 --- a/packages/react-reconciler/src/ReactFiberClassComponent.old.js +++ b/packages/react-reconciler/src/ReactFiberClassComponent.old.js @@ -20,6 +20,7 @@ import { enableSchedulingProfiler, warnAboutDeprecatedLifecycles, enableStrictEffects, + enableLazyContextPropagation, } from 'shared/ReactFeatureFlags'; import ReactStrictModeWarnings from './ReactStrictModeWarnings.old'; import {isMounted} from './ReactFiberTreeReflection'; @@ -58,7 +59,7 @@ import { hasContextChanged, emptyContextObject, } from './ReactFiberContext.old'; -import {readContext} from './ReactFiberNewContext.old'; +import {readContext, checkIfContextChanged} from './ReactFiberNewContext.old'; import { requestEventTime, requestUpdateLane, @@ -1150,7 +1151,13 @@ function updateClassInstance( unresolvedOldProps === unresolvedNewProps && oldState === newState && !hasContextChanged() && - !checkHasForceUpdateAfterProcessing() + !checkHasForceUpdateAfterProcessing() && + !( + enableLazyContextPropagation && + current !== null && + current.dependencies !== null && + checkIfContextChanged(current.dependencies) + ) ) { // If an update was already in progress, we should schedule an Update // effect even though we're bailing out, so that cWU/cDU are called. @@ -1193,7 +1200,15 @@ function updateClassInstance( oldState, newState, nextContext, - ); + ) || + // TODO: In some cases, we'll end up checking if context has changed twice, + // both before and after `shouldComponentUpdate` has been called. Not ideal, + // but I'm loath to refactor this function. This only happens for memoized + // components so it's not that common. + (enableLazyContextPropagation && + current !== null && + current.dependencies !== null && + checkIfContextChanged(current.dependencies)); if (shouldUpdate) { // In order to support react-lifecycles-compat polyfilled components, diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index bfea1ddda134b..e7e5c3128beac 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -30,6 +30,7 @@ import { decoupleUpdatePriorityFromScheduler, enableUseRefAccessWarning, enableStrictEffects, + enableLazyContextPropagation, } from 'shared/ReactFeatureFlags'; import { @@ -54,7 +55,7 @@ import { higherLanePriority, DefaultLanePriority, } from './ReactFiberLane.new'; -import {readContext} from './ReactFiberNewContext.new'; +import {readContext, checkIfContextChanged} from './ReactFiberNewContext.new'; import {HostRoot, CacheComponent} from './ReactWorkTags'; import { Update as UpdateEffect, @@ -83,7 +84,10 @@ import { import invariant from 'shared/invariant'; import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; import is from 'shared/objectIs'; -import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork.new'; +import { + markWorkInProgressReceivedUpdate, + checkIfWorkInProgressReceivedUpdate, +} from './ReactFiberBeginWork.new'; import { UserBlockingPriority, NormalPriority, @@ -496,6 +500,27 @@ export function renderWithHooks( 'early return statement.', ); + if (enableLazyContextPropagation) { + if (current !== null) { + if (!checkIfWorkInProgressReceivedUpdate()) { + // If there were no changes to props or state, we need to check if there + // was a context change. We didn't already do this because there's no + // 1:1 correspondence between dependencies and hooks. Although, because + // there almost always is in the common case (`readContext` is an + // internal API), we could compare in there. OTOH, we only hit this case + // if everything else bails out, so on the whole it might be better to + // keep the comparison out of the common path. + const currentDependencies = current.dependencies; + if ( + currentDependencies !== null && + checkIfContextChanged(currentDependencies) + ) { + markWorkInProgressReceivedUpdate(); + } + } + } + } + return children; } diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index fea9afd815fc8..55dca75656c35 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -30,6 +30,7 @@ import { decoupleUpdatePriorityFromScheduler, enableUseRefAccessWarning, enableStrictEffects, + enableLazyContextPropagation, } from 'shared/ReactFeatureFlags'; import { @@ -54,7 +55,7 @@ import { higherLanePriority, DefaultLanePriority, } from './ReactFiberLane.old'; -import {readContext} from './ReactFiberNewContext.old'; +import {readContext, checkIfContextChanged} from './ReactFiberNewContext.old'; import {HostRoot, CacheComponent} from './ReactWorkTags'; import { Update as UpdateEffect, @@ -83,7 +84,10 @@ import { import invariant from 'shared/invariant'; import getComponentNameFromFiber from 'react-reconciler/src/getComponentNameFromFiber'; import is from 'shared/objectIs'; -import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork.old'; +import { + markWorkInProgressReceivedUpdate, + checkIfWorkInProgressReceivedUpdate, +} from './ReactFiberBeginWork.old'; import { UserBlockingPriority, NormalPriority, @@ -496,6 +500,27 @@ export function renderWithHooks( 'early return statement.', ); + if (enableLazyContextPropagation) { + if (current !== null) { + if (!checkIfWorkInProgressReceivedUpdate()) { + // If there were no changes to props or state, we need to check if there + // was a context change. We didn't already do this because there's no + // 1:1 correspondence between dependencies and hooks. Although, because + // there almost always is in the common case (`readContext` is an + // internal API), we could compare in there. OTOH, we only hit this case + // if everything else bails out, so on the whole it might be better to + // keep the comparison out of the common path. + const currentDependencies = current.dependencies; + if ( + currentDependencies !== null && + checkIfContextChanged(currentDependencies) + ) { + markWorkInProgressReceivedUpdate(); + } + } + } + } + return children; } diff --git a/packages/react-reconciler/src/ReactFiberNewContext.new.js b/packages/react-reconciler/src/ReactFiberNewContext.new.js index ad524da736d0c..91ba7c9ff6516 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.new.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.new.js @@ -8,7 +8,11 @@ */ import type {ReactContext} from 'shared/ReactTypes'; -import type {Fiber, ContextDependency} from './ReactInternalTypes'; +import type { + Fiber, + ContextDependency, + Dependencies, +} from './ReactInternalTypes'; import type {StackCursor} from './ReactFiberStack.new'; import type {Lanes} from './ReactFiberLane.new'; import type {SharedQueue} from './ReactUpdateQueue.new'; @@ -34,7 +38,10 @@ import invariant from 'shared/invariant'; import is from 'shared/objectIs'; import {createUpdate, ForceUpdate} from './ReactUpdateQueue.new'; import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork.new'; -import {enableSuspenseServerRenderer} from 'shared/ReactFeatureFlags'; +import { + enableSuspenseServerRenderer, + enableLazyContextPropagation, +} from 'shared/ReactFeatureFlags'; const valueCursor: StackCursor = createCursor(null); @@ -210,33 +217,46 @@ export function propagateContextChange( ) { // Match! Schedule an update on this fiber. - if (fiber.tag === ClassComponent) { - // Schedule a force update on the work-in-progress. - const lane = pickArbitraryLane(renderLanes); - const update = createUpdate(NoTimestamp, lane); - update.tag = ForceUpdate; - // TODO: Because we don't have a work-in-progress, this will add the - // update to the current fiber, too, which means it will persist even if - // this render is thrown away. Since it's a race condition, not sure it's - // worth fixing. - - // Inlined `enqueueUpdate` to remove interleaved update check - const updateQueue = fiber.updateQueue; - if (updateQueue === null) { - // Only occurs if the fiber has been unmounted. - } else { - const sharedQueue: SharedQueue = (updateQueue: any).shared; - const pending = sharedQueue.pending; - if (pending === null) { - // This is the first update. Create a circular list. - update.next = update; + if (enableLazyContextPropagation) { + // In the lazy implemenation, don't mark a dirty flag on the + // dependency itself. Not all changes are propagated, so we can't + // rely on the propagation function alone to determine whether + // something has changed; the consumer will check. In the future, + // we could add back a dirty flag as an optimization to avoid + // double checking, but until we have selectors it's not really + // worth the trouble. + } else { + if (fiber.tag === ClassComponent) { + // Schedule a force update on the work-in-progress. + const lane = pickArbitraryLane(renderLanes); + const update = createUpdate(NoTimestamp, lane); + update.tag = ForceUpdate; + // TODO: Because we don't have a work-in-progress, this will add the + // update to the current fiber, too, which means it will persist even if + // this render is thrown away. Since it's a race condition, not sure it's + // worth fixing. + + // Inlined `enqueueUpdate` to remove interleaved update check + const updateQueue = fiber.updateQueue; + if (updateQueue === null) { + // Only occurs if the fiber has been unmounted. } else { - update.next = pending.next; - pending.next = update; + const sharedQueue: SharedQueue = (updateQueue: any).shared; + const pending = sharedQueue.pending; + if (pending === null) { + // This is the first update. Create a circular list. + update.next = update; + } else { + update.next = pending.next; + pending.next = update; + } + sharedQueue.pending = update; } - sharedQueue.pending = update; } + // Mark the updated lanes on the list, too. + list.lanes = mergeLanes(list.lanes, renderLanes); } + fiber.lanes = mergeLanes(fiber.lanes, renderLanes); const alternate = fiber.alternate; if (alternate !== null) { @@ -244,9 +264,6 @@ export function propagateContextChange( } scheduleWorkOnParentPath(fiber.return, renderLanes); - // Mark the updated lanes on the list, too. - list.lanes = mergeLanes(list.lanes, renderLanes); - // Since we already found a match, we can stop traversing the // dependency list. break; @@ -311,6 +328,30 @@ export function propagateContextChange( } } +export function checkIfContextChanged(currentDependencies: Dependencies) { + if (!enableLazyContextPropagation) { + return false; + } + // Iterate over the current dependencies to see if something changed. This + // only gets called if props and state has already bailed out, so it's a + // relatively uncommon path, except at the root of a changed subtree. + // Alternatively, we could move these comparisons into `readContext`, but + // that's a much hotter path, so I think this is an appropriate trade off. + let dependency = currentDependencies.firstContext; + while (dependency !== null) { + const context = dependency.context; + const newValue = isPrimaryRenderer + ? context._currentValue + : context._currentValue2; + const oldValue = dependency.memoizedValue; + if (!is(newValue, oldValue)) { + return true; + } + dependency = dependency.next; + } + return false; +} + export function prepareToReadContext( workInProgress: Fiber, renderLanes: Lanes, @@ -321,14 +362,19 @@ export function prepareToReadContext( const dependencies = workInProgress.dependencies; if (dependencies !== null) { - const firstContext = dependencies.firstContext; - if (firstContext !== null) { - if (includesSomeLane(dependencies.lanes, renderLanes)) { - // Context list has a pending update. Mark that this fiber performed work. - markWorkInProgressReceivedUpdate(); - } + if (enableLazyContextPropagation) { // Reset the work-in-progress list dependencies.firstContext = null; + } else { + const firstContext = dependencies.firstContext; + if (firstContext !== null) { + if (includesSomeLane(dependencies.lanes, renderLanes)) { + // Context list has a pending update. Mark that this fiber performed work. + markWorkInProgressReceivedUpdate(); + } + // Reset the work-in-progress list + dependencies.firstContext = null; + } } } } @@ -350,6 +396,10 @@ export function readContext( } } + const value = isPrimaryRenderer + ? context._currentValue + : context._currentValue2; + if (lastContextWithAllBitsObserved === context) { // Nothing to do. We already observe everything in this context. } else if (observedBits === false || observedBits === 0) { @@ -370,6 +420,7 @@ export function readContext( const contextItem = { context: ((context: any): ReactContext), observedBits: resolvedObservedBits, + memoizedValue: value, next: null, }; @@ -387,6 +438,8 @@ export function readContext( currentlyRenderingFiber.dependencies = { lanes: NoLanes, firstContext: contextItem, + + // TODO: This is an old field. Delete it. responders: null, }; } else { @@ -394,5 +447,5 @@ export function readContext( lastContextDependency = lastContextDependency.next = contextItem; } } - return isPrimaryRenderer ? context._currentValue : context._currentValue2; + return value; } diff --git a/packages/react-reconciler/src/ReactFiberNewContext.old.js b/packages/react-reconciler/src/ReactFiberNewContext.old.js index c1102b89f93f2..602eadd6b010a 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.old.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.old.js @@ -8,7 +8,11 @@ */ import type {ReactContext} from 'shared/ReactTypes'; -import type {Fiber, ContextDependency} from './ReactInternalTypes'; +import type { + Fiber, + ContextDependency, + Dependencies, +} from './ReactInternalTypes'; import type {StackCursor} from './ReactFiberStack.old'; import type {Lanes} from './ReactFiberLane.old'; import type {SharedQueue} from './ReactUpdateQueue.old'; @@ -34,7 +38,10 @@ import invariant from 'shared/invariant'; import is from 'shared/objectIs'; import {createUpdate, ForceUpdate} from './ReactUpdateQueue.old'; import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork.old'; -import {enableSuspenseServerRenderer} from 'shared/ReactFeatureFlags'; +import { + enableSuspenseServerRenderer, + enableLazyContextPropagation, +} from 'shared/ReactFeatureFlags'; const valueCursor: StackCursor = createCursor(null); @@ -210,33 +217,46 @@ export function propagateContextChange( ) { // Match! Schedule an update on this fiber. - if (fiber.tag === ClassComponent) { - // Schedule a force update on the work-in-progress. - const lane = pickArbitraryLane(renderLanes); - const update = createUpdate(NoTimestamp, lane); - update.tag = ForceUpdate; - // TODO: Because we don't have a work-in-progress, this will add the - // update to the current fiber, too, which means it will persist even if - // this render is thrown away. Since it's a race condition, not sure it's - // worth fixing. - - // Inlined `enqueueUpdate` to remove interleaved update check - const updateQueue = fiber.updateQueue; - if (updateQueue === null) { - // Only occurs if the fiber has been unmounted. - } else { - const sharedQueue: SharedQueue = (updateQueue: any).shared; - const pending = sharedQueue.pending; - if (pending === null) { - // This is the first update. Create a circular list. - update.next = update; + if (enableLazyContextPropagation) { + // In the lazy implemenation, don't mark a dirty flag on the + // dependency itself. Not all changes are propagated, so we can't + // rely on the propagation function alone to determine whether + // something has changed; the consumer will check. In the future, + // we could add back a dirty flag as an optimization to avoid + // double checking, but until we have selectors it's not really + // worth the trouble. + } else { + if (fiber.tag === ClassComponent) { + // Schedule a force update on the work-in-progress. + const lane = pickArbitraryLane(renderLanes); + const update = createUpdate(NoTimestamp, lane); + update.tag = ForceUpdate; + // TODO: Because we don't have a work-in-progress, this will add the + // update to the current fiber, too, which means it will persist even if + // this render is thrown away. Since it's a race condition, not sure it's + // worth fixing. + + // Inlined `enqueueUpdate` to remove interleaved update check + const updateQueue = fiber.updateQueue; + if (updateQueue === null) { + // Only occurs if the fiber has been unmounted. } else { - update.next = pending.next; - pending.next = update; + const sharedQueue: SharedQueue = (updateQueue: any).shared; + const pending = sharedQueue.pending; + if (pending === null) { + // This is the first update. Create a circular list. + update.next = update; + } else { + update.next = pending.next; + pending.next = update; + } + sharedQueue.pending = update; } - sharedQueue.pending = update; } + // Mark the updated lanes on the list, too. + list.lanes = mergeLanes(list.lanes, renderLanes); } + fiber.lanes = mergeLanes(fiber.lanes, renderLanes); const alternate = fiber.alternate; if (alternate !== null) { @@ -244,9 +264,6 @@ export function propagateContextChange( } scheduleWorkOnParentPath(fiber.return, renderLanes); - // Mark the updated lanes on the list, too. - list.lanes = mergeLanes(list.lanes, renderLanes); - // Since we already found a match, we can stop traversing the // dependency list. break; @@ -311,6 +328,30 @@ export function propagateContextChange( } } +export function checkIfContextChanged(currentDependencies: Dependencies) { + if (!enableLazyContextPropagation) { + return false; + } + // Iterate over the current dependencies to see if something changed. This + // only gets called if props and state has already bailed out, so it's a + // relatively uncommon path, except at the root of a changed subtree. + // Alternatively, we could move these comparisons into `readContext`, but + // that's a much hotter path, so I think this is an appropriate trade off. + let dependency = currentDependencies.firstContext; + while (dependency !== null) { + const context = dependency.context; + const newValue = isPrimaryRenderer + ? context._currentValue + : context._currentValue2; + const oldValue = dependency.memoizedValue; + if (!is(newValue, oldValue)) { + return true; + } + dependency = dependency.next; + } + return false; +} + export function prepareToReadContext( workInProgress: Fiber, renderLanes: Lanes, @@ -321,14 +362,19 @@ export function prepareToReadContext( const dependencies = workInProgress.dependencies; if (dependencies !== null) { - const firstContext = dependencies.firstContext; - if (firstContext !== null) { - if (includesSomeLane(dependencies.lanes, renderLanes)) { - // Context list has a pending update. Mark that this fiber performed work. - markWorkInProgressReceivedUpdate(); - } + if (enableLazyContextPropagation) { // Reset the work-in-progress list dependencies.firstContext = null; + } else { + const firstContext = dependencies.firstContext; + if (firstContext !== null) { + if (includesSomeLane(dependencies.lanes, renderLanes)) { + // Context list has a pending update. Mark that this fiber performed work. + markWorkInProgressReceivedUpdate(); + } + // Reset the work-in-progress list + dependencies.firstContext = null; + } } } } @@ -350,6 +396,10 @@ export function readContext( } } + const value = isPrimaryRenderer + ? context._currentValue + : context._currentValue2; + if (lastContextWithAllBitsObserved === context) { // Nothing to do. We already observe everything in this context. } else if (observedBits === false || observedBits === 0) { @@ -370,6 +420,7 @@ export function readContext( const contextItem = { context: ((context: any): ReactContext), observedBits: resolvedObservedBits, + memoizedValue: value, next: null, }; @@ -387,6 +438,8 @@ export function readContext( currentlyRenderingFiber.dependencies = { lanes: NoLanes, firstContext: contextItem, + + // TODO: This is an old field. Delete it. responders: null, }; } else { @@ -394,5 +447,5 @@ export function readContext( lastContextDependency = lastContextDependency.next = contextItem; } } - return isPrimaryRenderer ? context._currentValue : context._currentValue2; + return value; } diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index cb34ac74948e0..b1764bed5ac34 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -51,6 +51,7 @@ export type ContextDependency = { context: ReactContext, observedBits: number, next: ContextDependency | null, + memoizedValue: T, ... }; diff --git a/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js b/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js new file mode 100644 index 0000000000000..bdc89014ad640 --- /dev/null +++ b/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js @@ -0,0 +1,206 @@ +let React; +let ReactNoop; +let Scheduler; +let useState; +let useContext; + +describe('ReactLazyContextPropagation', () => { + beforeEach(() => { + jest.resetModules(); + + React = require('react'); + ReactNoop = require('react-noop-renderer'); + Scheduler = require('scheduler'); + useState = React.useState; + useContext = React.useContext; + }); + + function Text({text}) { + Scheduler.unstable_yieldValue(text); + return text; + } + + // NOTE: These tests are not specific to the lazy propagation (as opposed to + // eager propagation). The behavior should be the same in both + // implementations. These are tests that are more relevant to the lazy + // propagation implementation, though. + + test( + 'context change should prevent bailout of memoized component (useMemo -> ' + + 'no intermediate fiber)', + async () => { + const root = ReactNoop.createRoot(); + + const Context = React.createContext(0); + + let setValue; + function App() { + const [value, _setValue] = useState(0); + setValue = _setValue; + + // NOTE: It's an important part of this test that we're memoizing the + // props of the Consumer component, as opposed to wrapping in an + // additional memoized fiber, because the implementation propagates + // context changes whenever a fiber bails out. + const consumer = React.useMemo(() => , []); + + return {consumer}; + } + + function Consumer() { + const value = useContext(Context); + // Even though Consumer is memoized, Consumer should re-render + // DeepChild whenever the context value changes. Otherwise DeepChild + // won't receive the new value. + return ; + } + + function DeepChild({value}) { + return ; + } + + await ReactNoop.act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded([0]); + expect(root).toMatchRenderedOutput('0'); + + await ReactNoop.act(async () => { + setValue(1); + }); + expect(Scheduler).toHaveYielded([1]); + expect(root).toMatchRenderedOutput('1'); + }, + ); + + test('context change should prevent bailout of memoized component (memo HOC)', async () => { + const root = ReactNoop.createRoot(); + + const Context = React.createContext(0); + + let setValue; + function App() { + const [value, _setValue] = useState(0); + setValue = _setValue; + return ( + + + + ); + } + + const Consumer = React.memo(() => { + const value = useContext(Context); + // Even though Consumer is memoized, Consumer should re-render + // DeepChild whenever the context value changes. Otherwise DeepChild + // won't receive the new value. + return ; + }); + + function DeepChild({value}) { + return ; + } + + await ReactNoop.act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded([0]); + expect(root).toMatchRenderedOutput('0'); + + await ReactNoop.act(async () => { + setValue(1); + }); + expect(Scheduler).toHaveYielded([1]); + expect(root).toMatchRenderedOutput('1'); + }); + + test('context change should prevent bailout of memoized component (PureComponent)', async () => { + const root = ReactNoop.createRoot(); + + const Context = React.createContext(0); + + let setValue; + function App() { + const [value, _setValue] = useState(0); + setValue = _setValue; + return ( + + + + ); + } + + class Consumer extends React.PureComponent { + static contextType = Context; + render() { + // Even though Consumer is memoized, Consumer should re-render + // DeepChild whenever the context value changes. Otherwise DeepChild + // won't receive the new value. + return ; + } + } + + function DeepChild({value}) { + return ; + } + + await ReactNoop.act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded([0]); + expect(root).toMatchRenderedOutput('0'); + + await ReactNoop.act(async () => { + setValue(1); + }); + expect(Scheduler).toHaveYielded([1]); + expect(root).toMatchRenderedOutput('1'); + }); + + test("context consumer bails out if context hasn't changed", async () => { + const root = ReactNoop.createRoot(); + + const Context = React.createContext(0); + + function App() { + return ( + + + + ); + } + + let setOtherValue; + const Consumer = React.memo(() => { + const value = useContext(Context); + + const [, _setOtherValue] = useState(0); + setOtherValue = _setOtherValue; + + Scheduler.unstable_yieldValue('Consumer'); + + return ; + }); + + await ReactNoop.act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['Consumer', 0]); + expect(root).toMatchRenderedOutput('0'); + + await ReactNoop.act(async () => { + // Intentionally calling setState to some other arbitrary value before + // setting it back to the current one. That way an update is scheduled, + // but we'll bail out during render when nothing has changed. + setOtherValue(1); + setOtherValue(0); + }); + // NOTE: If this didn't yield anything, that indicates that we never visited + // the consumer during the render phase, which probably means the eager + // bailout mechanism kicked in. Because we're testing the _lazy_ bailout + // mechanism, update this test to foil the _eager_ bailout, somehow. Perhaps + // by switching to useReducer. + expect(Scheduler).toHaveYielded(['Consumer']); + expect(root).toMatchRenderedOutput('0'); + }); +}); diff --git a/packages/react/src/__tests__/ReactContextValidator-test.js b/packages/react/src/__tests__/ReactContextValidator-test.js index f3fc8875c79b8..ae1a9a0da7fe4 100644 --- a/packages/react/src/__tests__/ReactContextValidator-test.js +++ b/packages/react/src/__tests__/ReactContextValidator-test.js @@ -431,8 +431,12 @@ describe('ReactContextValidator', () => { expect(renderContext).toBe(secondContext); expect(componentDidUpdateContext).toBe(secondContext); - // sCU is not called in this case because React force updates when a provider re-renders - expect(shouldComponentUpdateWasCalled).toBe(false); + if (gate(flags => flags.enableLazyContextPropagation)) { + expect(shouldComponentUpdateWasCalled).toBe(true); + } else { + // sCU is not called in this case because React force updates when a provider re-renders + expect(shouldComponentUpdateWasCalled).toBe(false); + } }); it('should re-render PureComponents when context Provider updates', () => { diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 201546e822b20..484717599557a 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -158,3 +158,5 @@ export const enableDiscreteEventMicroTasks = false; export const enableSyncMicroTasks = false; export const enableNativeEventPriorityInference = false; + +export const enableLazyContextPropagation = false; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index e0bff4b32f634..b664dc00632a5 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -61,6 +61,7 @@ export const disableSchedulerTimeoutInWorkLoop = false; export const enableDiscreteEventMicroTasks = false; export const enableSyncMicroTasks = false; export const enableNativeEventPriorityInference = false; +export const enableLazyContextPropagation = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index a40b6e293c241..f5d44fff13083 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -60,6 +60,7 @@ export const disableSchedulerTimeoutInWorkLoop = false; export const enableDiscreteEventMicroTasks = false; export const enableSyncMicroTasks = false; export const enableNativeEventPriorityInference = false; +export const enableLazyContextPropagation = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 97a26ddd86819..e05226fd5d019 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -60,6 +60,7 @@ export const disableSchedulerTimeoutInWorkLoop = false; export const enableDiscreteEventMicroTasks = false; export const enableSyncMicroTasks = false; export const enableNativeEventPriorityInference = false; +export const enableLazyContextPropagation = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js index 1417670d7915c..41a368f659ec8 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native.js @@ -60,6 +60,7 @@ export const disableSchedulerTimeoutInWorkLoop = false; export const enableDiscreteEventMicroTasks = false; export const enableSyncMicroTasks = false; export const enableNativeEventPriorityInference = false; +export const enableLazyContextPropagation = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 17848a59376c6..577e77f18df90 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -60,6 +60,7 @@ export const disableSchedulerTimeoutInWorkLoop = false; export const enableDiscreteEventMicroTasks = false; export const enableSyncMicroTasks = false; export const enableNativeEventPriorityInference = false; +export const enableLazyContextPropagation = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.testing.js b/packages/shared/forks/ReactFeatureFlags.testing.js index ce73c8a870cf0..0d7c78e39e294 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.js @@ -60,6 +60,7 @@ export const disableSchedulerTimeoutInWorkLoop = false; export const enableDiscreteEventMicroTasks = false; export const enableSyncMicroTasks = false; export const enableNativeEventPriorityInference = false; +export const enableLazyContextPropagation = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.testing.www.js b/packages/shared/forks/ReactFeatureFlags.testing.www.js index 02dba8551fb1a..1dfa34662cd73 100644 --- a/packages/shared/forks/ReactFeatureFlags.testing.www.js +++ b/packages/shared/forks/ReactFeatureFlags.testing.www.js @@ -60,6 +60,7 @@ export const disableSchedulerTimeoutInWorkLoop = false; export const enableDiscreteEventMicroTasks = false; export const enableSyncMicroTasks = false; export const enableNativeEventPriorityInference = false; +export const enableLazyContextPropagation = false; // Flow magic to verify the exports of this file match the original version. // eslint-disable-next-line no-unused-vars diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index b75b80dbde650..416dfb6191442 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -59,3 +59,4 @@ export const disableSchedulerTimeoutInWorkLoop = __VARIANT__; export const enableDiscreteEventMicroTasks = __VARIANT__; export const enableSyncMicroTasks = __VARIANT__; export const enableNativeEventPriorityInference = __VARIANT__; +export const enableLazyContextPropagation = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 0612eacf80d07..f1fb6d499b5f3 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -35,6 +35,7 @@ export const { enableDiscreteEventMicroTasks, enableSyncMicroTasks, enableNativeEventPriorityInference, + enableLazyContextPropagation, } = dynamicFeatureFlags; // On WWW, __EXPERIMENTAL__ is used for a new modern build. From 62a37a8ebc0fcc7f343bffa434c24f6eb390a04e Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Thu, 25 Feb 2021 22:53:02 -0600 Subject: [PATCH 2/5] [Experiment] Lazily propagate context changes MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit When a context provider changes, we scan the tree for matching consumers and mark them as dirty so that we know they have pending work. This prevents us from bailing out if, say, an intermediate wrapper is memoized. Currently, we propagate these changes eagerly, at the provider. However, in many cases, we would have ended up visiting the consumer nodes anyway, as part of the normal render traversal, because there's no memoized node in between that bails out. We can save CPU cycles by propagating changes only when we hit a memoized component — so, instead of propagating eagerly at the provider, we propagate lazily if or when something bails out. Most of our bailout logic is centralized in `bailoutOnAlreadyFinishedWork`, so this ended up being not that difficult to implement correctly. There are some exceptions: Suspense and Offscreen. Those are special because they sometimes defer the rendering of their children to a completely separate render cycle. In those cases, we must take extra care to propagate *all* the context changes, not just the first one. I'm pleasantly surprised at how little I needed to change in this initial implementation. I was worried I'd have to use the reconciler fork, but I ended up being able to wrap all my changes in a regular feature flag. So, we could run an experiment in parallel to our other ones. I do consider this a risky rollout overall because of the potential for subtle semantic deviations. However, the model is simple enough that I don't expect us to have trouble fixing regressions if or when they arise during internal dogfooding. --- This is largely based on [RFC#118](https://github.com/reactjs/rfcs/pull/118), by @gnoff. I did deviate in some of the implementation details, though. The main one is how I chose to track context changes. Instead of storing a dirty flag on the stack, I added a `memoizedValue` field to the context dependency object. Then, to check if something has changed, the consumer compares the new context value to the old (memoized) one. This is necessary because of Suspense and Offscreen — those components defer work from one render into a later one. When the subtree continues rendering, the stack from the previous render is no longer available. But the memoized values on the dependencies list are. This requires a bit more work when a consumer bails out, but nothing considerable, and there are ways we could optimize it even further. Conceptually, this model is really appealing, since it matches how our other features "reactively" detect changes — `useMemo`, `useEffect`, `getDerivedStateFromProps`, the built-in cache, and so on. I also intentionally dropped support for `unstable_calculateChangedBits`. We're planning to remove this API anyway before the next major release, in favor of context selectors. It's an unstable feature that we never advertised; I don't think it's seen much adoption. Co-Authored-By: Josh Story --- .../src/ReactFiberBeginWork.new.js | 130 ++++- .../src/ReactFiberBeginWork.old.js | 130 ++++- .../react-reconciler/src/ReactFiberFlags.js | 45 +- .../src/ReactFiberNewContext.new.js | 74 ++- .../src/ReactFiberNewContext.old.js | 74 ++- .../src/ReactFiberThrow.new.js | 19 + .../src/ReactFiberThrow.old.js | 19 + .../__tests__/ReactContextPropagation-test.js | 496 +++++++++++++++++- .../src/__tests__/ReactNewContext-test.js | 12 +- 9 files changed, 921 insertions(+), 78 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 66584784ee379..345f668fc62e8 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -81,6 +81,7 @@ import { warnAboutDefaultPropsOnFunctionComponents, enableScopeAPI, enableCache, + enableLazyContextPropagation, } from 'shared/ReactFeatureFlags'; import invariant from 'shared/invariant'; import shallowEqual from 'shared/shallowEqual'; @@ -154,6 +155,9 @@ import {findFirstSuspended} from './ReactFiberSuspenseComponent.new'; import { pushProvider, propagateContextChange, + lazilyPropagateParentContextChanges, + propagateParentContextChangesToDeferredTree, + checkIfContextChanged, readContext, prepareToReadContext, calculateChangedBits, @@ -646,6 +650,18 @@ function updateOffscreenComponent( // We're about to bail out, but we need to push this to the stack anyway // to avoid a push/pop misalignment. pushRenderLanes(workInProgress, nextBaseLanes); + + if (enableLazyContextPropagation && current !== null) { + // Since this tree will resume rendering in a separate render, we need + // to propagate parent contexts now so we don't lose track of which + // ones changed. + propagateParentContextChangesToDeferredTree( + current, + workInProgress, + renderLanes, + ); + } + return null; } else { // This is the second render. The surrounding visible content has already @@ -2445,6 +2461,19 @@ function updateDehydratedSuspenseComponent( renderLanes, ); } + + if ( + enableLazyContextPropagation && + // TODO: Factoring is a little weird, since we check this right below, too. + // But don't want to re-arrange the if-else chain until/unless this + // feature lands. + !didReceiveUpdate + ) { + // We need to check if any children have context before we decide to bail + // out, so propagate the changes now. + lazilyPropagateParentContextChanges(current, workInProgress, renderLanes); + } + // We use lanes to indicate that a child might depend on context, so if // any context has changed, we need to treat is as if the input might have changed. const hasContextChanged = includesSomeLane(renderLanes, current.childLanes); @@ -2970,25 +2999,37 @@ function updateContextProvider( pushProvider(workInProgress, context, newValue); - if (oldProps !== null) { - const oldValue = oldProps.value; - const changedBits = calculateChangedBits(context, newValue, oldValue); - if (changedBits === 0) { - // No change. Bailout early if children are the same. - if ( - oldProps.children === newProps.children && - !hasLegacyContextChanged() - ) { - return bailoutOnAlreadyFinishedWork( - current, + if (enableLazyContextPropagation) { + // In the lazy propagation implementation, we don't scan for matching + // consumers until something bails out, because until something bails out + // we're going to visit those nodes, anyway. The trade-off is that it shifts + // responsibility to the consumer to track whether something has changed. + } else { + if (oldProps !== null) { + const oldValue = oldProps.value; + const changedBits = calculateChangedBits(context, newValue, oldValue); + if (changedBits === 0) { + // No change. Bailout early if children are the same. + if ( + oldProps.children === newProps.children && + !hasLegacyContextChanged() + ) { + return bailoutOnAlreadyFinishedWork( + current, + workInProgress, + renderLanes, + ); + } + } else { + // The context value changed. Search for matching consumers and schedule + // them to update. + propagateContextChange( workInProgress, + context, + changedBits, renderLanes, ); } - } else { - // The context value changed. Search for matching consumers and schedule - // them to update. - propagateContextChange(workInProgress, context, changedBits, renderLanes); } } @@ -3100,13 +3141,23 @@ function bailoutOnAlreadyFinishedWork( // The children don't have any work either. We can skip them. // TODO: Once we add back resuming, we should check if the children are // a work-in-progress set. If so, we need to transfer their effects. - return null; - } else { - // This fiber doesn't have work, but its subtree does. Clone the child - // fibers and continue. - cloneChildFibers(current, workInProgress); - return workInProgress.child; + + if (enableLazyContextPropagation && current !== null) { + // Before bailing out, check if there are any context changes in + // the children. + lazilyPropagateParentContextChanges(current, workInProgress, renderLanes); + if (!includesSomeLane(renderLanes, workInProgress.childLanes)) { + return null; + } + } else { + return null; + } } + + // This fiber doesn't have work, but its subtree does. Clone the child + // fibers and continue. + cloneChildFibers(current, workInProgress); + return workInProgress.child; } function remountFiber( @@ -3175,7 +3226,7 @@ function beginWork( workInProgress: Fiber, renderLanes: Lanes, ): Fiber | null { - const updateLanes = workInProgress.lanes; + let updateLanes = workInProgress.lanes; if (__DEV__) { if (workInProgress._debugNeedsRemount && current !== null) { @@ -3196,6 +3247,17 @@ function beginWork( } if (current !== null) { + // TODO: The factoring of this block is weird. + if ( + enableLazyContextPropagation && + !includesSomeLane(renderLanes, updateLanes) + ) { + const dependencies = current.dependencies; + if (dependencies !== null && checkIfContextChanged(dependencies)) { + updateLanes = mergeLanes(updateLanes, renderLanes); + } + } + const oldProps = current.memoizedProps; const newProps = workInProgress.pendingProps; @@ -3316,6 +3378,9 @@ function beginWork( // primary children and work on the fallback. return child.sibling; } else { + // Note: We can return `null` here because we already checked + // whether there were nested context consumers, via the call to + // `bailoutOnAlreadyFinishedWork` above. return null; } } @@ -3330,11 +3395,30 @@ function beginWork( case SuspenseListComponent: { const didSuspendBefore = (current.flags & DidCapture) !== NoFlags; - const hasChildWork = includesSomeLane( + let hasChildWork = includesSomeLane( renderLanes, workInProgress.childLanes, ); + if (enableLazyContextPropagation && !hasChildWork) { + // Context changes may not have been propagated yet. We need to do + // that now, before we can decide whether to bail out. + // TODO: We use `childLanes` as a heuristic for whether there is + // remaining work in a few places, including + // `bailoutOnAlreadyFinishedWork` and + // `updateDehydratedSuspenseComponent`. We should maybe extract this + // into a dedicated function. + lazilyPropagateParentContextChanges( + current, + workInProgress, + renderLanes, + ); + hasChildWork = includesSomeLane( + renderLanes, + workInProgress.childLanes, + ); + } + if (didSuspendBefore) { if (hasChildWork) { // If something was in fallback state last time, and we have all the diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index 8faa4d859b7e0..72b4e33431b00 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -81,6 +81,7 @@ import { warnAboutDefaultPropsOnFunctionComponents, enableScopeAPI, enableCache, + enableLazyContextPropagation, } from 'shared/ReactFeatureFlags'; import invariant from 'shared/invariant'; import shallowEqual from 'shared/shallowEqual'; @@ -154,6 +155,9 @@ import {findFirstSuspended} from './ReactFiberSuspenseComponent.old'; import { pushProvider, propagateContextChange, + lazilyPropagateParentContextChanges, + propagateParentContextChangesToDeferredTree, + checkIfContextChanged, readContext, prepareToReadContext, calculateChangedBits, @@ -646,6 +650,18 @@ function updateOffscreenComponent( // We're about to bail out, but we need to push this to the stack anyway // to avoid a push/pop misalignment. pushRenderLanes(workInProgress, nextBaseLanes); + + if (enableLazyContextPropagation && current !== null) { + // Since this tree will resume rendering in a separate render, we need + // to propagate parent contexts now so we don't lose track of which + // ones changed. + propagateParentContextChangesToDeferredTree( + current, + workInProgress, + renderLanes, + ); + } + return null; } else { // This is the second render. The surrounding visible content has already @@ -2445,6 +2461,19 @@ function updateDehydratedSuspenseComponent( renderLanes, ); } + + if ( + enableLazyContextPropagation && + // TODO: Factoring is a little weird, since we check this right below, too. + // But don't want to re-arrange the if-else chain until/unless this + // feature lands. + !didReceiveUpdate + ) { + // We need to check if any children have context before we decide to bail + // out, so propagate the changes now. + lazilyPropagateParentContextChanges(current, workInProgress, renderLanes); + } + // We use lanes to indicate that a child might depend on context, so if // any context has changed, we need to treat is as if the input might have changed. const hasContextChanged = includesSomeLane(renderLanes, current.childLanes); @@ -2970,25 +2999,37 @@ function updateContextProvider( pushProvider(workInProgress, context, newValue); - if (oldProps !== null) { - const oldValue = oldProps.value; - const changedBits = calculateChangedBits(context, newValue, oldValue); - if (changedBits === 0) { - // No change. Bailout early if children are the same. - if ( - oldProps.children === newProps.children && - !hasLegacyContextChanged() - ) { - return bailoutOnAlreadyFinishedWork( - current, + if (enableLazyContextPropagation) { + // In the lazy propagation implementation, we don't scan for matching + // consumers until something bails out, because until something bails out + // we're going to visit those nodes, anyway. The trade-off is that it shifts + // responsibility to the consumer to track whether something has changed. + } else { + if (oldProps !== null) { + const oldValue = oldProps.value; + const changedBits = calculateChangedBits(context, newValue, oldValue); + if (changedBits === 0) { + // No change. Bailout early if children are the same. + if ( + oldProps.children === newProps.children && + !hasLegacyContextChanged() + ) { + return bailoutOnAlreadyFinishedWork( + current, + workInProgress, + renderLanes, + ); + } + } else { + // The context value changed. Search for matching consumers and schedule + // them to update. + propagateContextChange( workInProgress, + context, + changedBits, renderLanes, ); } - } else { - // The context value changed. Search for matching consumers and schedule - // them to update. - propagateContextChange(workInProgress, context, changedBits, renderLanes); } } @@ -3100,13 +3141,23 @@ function bailoutOnAlreadyFinishedWork( // The children don't have any work either. We can skip them. // TODO: Once we add back resuming, we should check if the children are // a work-in-progress set. If so, we need to transfer their effects. - return null; - } else { - // This fiber doesn't have work, but its subtree does. Clone the child - // fibers and continue. - cloneChildFibers(current, workInProgress); - return workInProgress.child; + + if (enableLazyContextPropagation && current !== null) { + // Before bailing out, check if there are any context changes in + // the children. + lazilyPropagateParentContextChanges(current, workInProgress, renderLanes); + if (!includesSomeLane(renderLanes, workInProgress.childLanes)) { + return null; + } + } else { + return null; + } } + + // This fiber doesn't have work, but its subtree does. Clone the child + // fibers and continue. + cloneChildFibers(current, workInProgress); + return workInProgress.child; } function remountFiber( @@ -3175,7 +3226,7 @@ function beginWork( workInProgress: Fiber, renderLanes: Lanes, ): Fiber | null { - const updateLanes = workInProgress.lanes; + let updateLanes = workInProgress.lanes; if (__DEV__) { if (workInProgress._debugNeedsRemount && current !== null) { @@ -3196,6 +3247,17 @@ function beginWork( } if (current !== null) { + // TODO: The factoring of this block is weird. + if ( + enableLazyContextPropagation && + !includesSomeLane(renderLanes, updateLanes) + ) { + const dependencies = current.dependencies; + if (dependencies !== null && checkIfContextChanged(dependencies)) { + updateLanes = mergeLanes(updateLanes, renderLanes); + } + } + const oldProps = current.memoizedProps; const newProps = workInProgress.pendingProps; @@ -3316,6 +3378,9 @@ function beginWork( // primary children and work on the fallback. return child.sibling; } else { + // Note: We can return `null` here because we already checked + // whether there were nested context consumers, via the call to + // `bailoutOnAlreadyFinishedWork` above. return null; } } @@ -3330,11 +3395,30 @@ function beginWork( case SuspenseListComponent: { const didSuspendBefore = (current.flags & DidCapture) !== NoFlags; - const hasChildWork = includesSomeLane( + let hasChildWork = includesSomeLane( renderLanes, workInProgress.childLanes, ); + if (enableLazyContextPropagation && !hasChildWork) { + // Context changes may not have been propagated yet. We need to do + // that now, before we can decide whether to bail out. + // TODO: We use `childLanes` as a heuristic for whether there is + // remaining work in a few places, including + // `bailoutOnAlreadyFinishedWork` and + // `updateDehydratedSuspenseComponent`. We should maybe extract this + // into a dedicated function. + lazilyPropagateParentContextChanges( + current, + workInProgress, + renderLanes, + ); + hasChildWork = includesSomeLane( + renderLanes, + workInProgress.childLanes, + ); + } + if (didSuspendBefore) { if (hasChildWork) { // If something was in fallback state last time, and we have all the diff --git a/packages/react-reconciler/src/ReactFiberFlags.js b/packages/react-reconciler/src/ReactFiberFlags.js index f29a1b645f15a..783e637555111 100644 --- a/packages/react-reconciler/src/ReactFiberFlags.js +++ b/packages/react-reconciler/src/ReactFiberFlags.js @@ -12,49 +12,50 @@ import {enableCreateEventHandleAPI} from 'shared/ReactFeatureFlags'; export type Flags = number; // Don't change these two values. They're used by React Dev Tools. -export const NoFlags = /* */ 0b00000000000000000000; -export const PerformedWork = /* */ 0b00000000000000000001; +export const NoFlags = /* */ 0b000000000000000000000; +export const PerformedWork = /* */ 0b000000000000000000001; // You can change the rest (and add more). -export const Placement = /* */ 0b00000000000000000010; -export const Update = /* */ 0b00000000000000000100; +export const Placement = /* */ 0b000000000000000000010; +export const Update = /* */ 0b000000000000000000100; export const PlacementAndUpdate = /* */ Placement | Update; -export const Deletion = /* */ 0b00000000000000001000; -export const ChildDeletion = /* */ 0b00000000000000010000; -export const ContentReset = /* */ 0b00000000000000100000; -export const Callback = /* */ 0b00000000000001000000; -export const DidCapture = /* */ 0b00000000000010000000; -export const Ref = /* */ 0b00000000000100000000; -export const Snapshot = /* */ 0b00000000001000000000; -export const Passive = /* */ 0b00000000010000000000; -export const Hydrating = /* */ 0b00000000100000000000; +export const Deletion = /* */ 0b000000000000000001000; +export const ChildDeletion = /* */ 0b000000000000000010000; +export const ContentReset = /* */ 0b000000000000000100000; +export const Callback = /* */ 0b000000000000001000000; +export const DidCapture = /* */ 0b000000000000010000000; +export const Ref = /* */ 0b000000000000100000000; +export const Snapshot = /* */ 0b000000000001000000000; +export const Passive = /* */ 0b000000000010000000000; +export const Hydrating = /* */ 0b000000000100000000000; export const HydratingAndUpdate = /* */ Hydrating | Update; -export const Visibility = /* */ 0b00000001000000000000; +export const Visibility = /* */ 0b000000001000000000000; export const LifecycleEffectMask = Passive | Update | Callback | Ref | Snapshot; // Union of all commit flags (flags with the lifetime of a particular commit) -export const HostEffectMask = /* */ 0b00000001111111111111; +export const HostEffectMask = /* */ 0b000000001111111111111; // These are not really side effects, but we still reuse this field. -export const Incomplete = /* */ 0b00000010000000000000; -export const ShouldCapture = /* */ 0b00000100000000000000; +export const Incomplete = /* */ 0b000000010000000000000; +export const ShouldCapture = /* */ 0b000000100000000000000; // TODO (effects) Remove this bit once the new reconciler is synced to the old. -export const PassiveUnmountPendingDev = /* */ 0b00001000000000000000; -export const ForceUpdateForLegacySuspense = /* */ 0b00010000000000000000; +export const PassiveUnmountPendingDev = /* */ 0b000001000000000000000; +export const ForceUpdateForLegacySuspense = /* */ 0b000010000000000000000; +export const DidPropagateContext = /* */ 0b000100000000000000000; // Static tags describe aspects of a fiber that are not specific to a render, // e.g. a fiber uses a passive effect (even if there are no updates on this particular render). // This enables us to defer more work in the unmount case, // since we can defer traversing the tree during layout to look for Passive effects, // and instead rely on the static flag as a signal that there may be cleanup work. -export const PassiveStatic = /* */ 0b00100000000000000000; +export const PassiveStatic = /* */ 0b001000000000000000000; // These flags allow us to traverse to fibers that have effects on mount // without traversing the entire tree after every commit for // double invoking -export const MountLayoutDev = /* */ 0b01000000000000000000; -export const MountPassiveDev = /* */ 0b10000000000000000000; +export const MountLayoutDev = /* */ 0b010000000000000000000; +export const MountPassiveDev = /* */ 0b100000000000000000000; // Groups of flags that are used in the commit phase to skip over trees that // don't contain effects, by checking subtreeFlags. diff --git a/packages/react-reconciler/src/ReactFiberNewContext.new.js b/packages/react-reconciler/src/ReactFiberNewContext.new.js index 91ba7c9ff6516..55d35510e2299 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.new.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.new.js @@ -7,7 +7,7 @@ * @flow */ -import type {ReactContext} from 'shared/ReactTypes'; +import type {ReactContext, ReactProviderType} from 'shared/ReactTypes'; import type { Fiber, ContextDependency, @@ -33,6 +33,7 @@ import { mergeLanes, pickArbitraryLane, } from './ReactFiberLane.new'; +import {NoFlags, DidPropagateContext} from './ReactFiberFlags'; import invariant from 'shared/invariant'; import is from 'shared/objectIs'; @@ -328,6 +329,77 @@ export function propagateContextChange( } } +export function lazilyPropagateParentContextChanges( + current: Fiber, + workInProgress: Fiber, + renderLanes: Lanes, +) { + propagateParentContextChanges(current, workInProgress, renderLanes, false); +} + +export function propagateParentContextChangesToDeferredTree( + current: Fiber, + workInProgress: Fiber, + renderLanes: Lanes, +) { + propagateParentContextChanges(current, workInProgress, renderLanes, true); +} + +function propagateParentContextChanges( + current: Fiber, + workInProgress: Fiber, + renderLanes: Lanes, + // TODO: This argument is currently unused. I think there's a way to optimize + // for the many providers case, where if the first propagation finds a match, + // the second one can avoid scanning down that same path. The trouble is that + // there could be a nested bailout below that. + forcePropagateEntireTree: boolean, +) { + if (!enableLazyContextPropagation) { + return false; + } + + let parent = workInProgress; + while (parent !== null && (parent.flags & DidPropagateContext) === NoFlags) { + if (parent.tag === ContextProvider) { + const currentParent = parent.alternate; + if (currentParent !== null) { + const oldProps = currentParent.memoizedProps; + if (oldProps !== null) { + const providerType: ReactProviderType = parent.type; + const context: ReactContext = providerType._context; + + const newProps = parent.pendingProps; + const newValue = newProps.value; + + const oldValue = oldProps.value; + + const changedBits = calculateChangedBits(context, newValue, oldValue); + if (changedBits !== 0) { + // The context value changed. Search for matching consumers and + // schedule them to update. + propagateContextChange( + workInProgress, + context, + changedBits, + renderLanes, + ); + } + } + } + } + parent = parent.return; + } + + // This is an optimization so that we only propagate each provider once per + // subtree. (We will propagate the same provider to different subtrees, though + // — that's why the flag is on the fiber that bailed out, not the provider.) + // If a deeply nested child bails out, and it calls this propagation function, + // it uses this flag to know that the remaining ancestor providers have + // already been propagated. + workInProgress.flags |= DidPropagateContext; +} + export function checkIfContextChanged(currentDependencies: Dependencies) { if (!enableLazyContextPropagation) { return false; diff --git a/packages/react-reconciler/src/ReactFiberNewContext.old.js b/packages/react-reconciler/src/ReactFiberNewContext.old.js index 602eadd6b010a..f6025075bdb87 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.old.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.old.js @@ -7,7 +7,7 @@ * @flow */ -import type {ReactContext} from 'shared/ReactTypes'; +import type {ReactContext, ReactProviderType} from 'shared/ReactTypes'; import type { Fiber, ContextDependency, @@ -33,6 +33,7 @@ import { mergeLanes, pickArbitraryLane, } from './ReactFiberLane.old'; +import {NoFlags, DidPropagateContext} from './ReactFiberFlags'; import invariant from 'shared/invariant'; import is from 'shared/objectIs'; @@ -328,6 +329,77 @@ export function propagateContextChange( } } +export function lazilyPropagateParentContextChanges( + current: Fiber, + workInProgress: Fiber, + renderLanes: Lanes, +) { + propagateParentContextChanges(current, workInProgress, renderLanes, false); +} + +export function propagateParentContextChangesToDeferredTree( + current: Fiber, + workInProgress: Fiber, + renderLanes: Lanes, +) { + propagateParentContextChanges(current, workInProgress, renderLanes, true); +} + +function propagateParentContextChanges( + current: Fiber, + workInProgress: Fiber, + renderLanes: Lanes, + // TODO: This argument is currently unused. I think there's a way to optimize + // for the many providers case, where if the first propagation finds a match, + // the second one can avoid scanning down that same path. The trouble is that + // there could be a nested bailout below that. + forcePropagateEntireTree: boolean, +) { + if (!enableLazyContextPropagation) { + return false; + } + + let parent = workInProgress; + while (parent !== null && (parent.flags & DidPropagateContext) === NoFlags) { + if (parent.tag === ContextProvider) { + const currentParent = parent.alternate; + if (currentParent !== null) { + const oldProps = currentParent.memoizedProps; + if (oldProps !== null) { + const providerType: ReactProviderType = parent.type; + const context: ReactContext = providerType._context; + + const newProps = parent.pendingProps; + const newValue = newProps.value; + + const oldValue = oldProps.value; + + const changedBits = calculateChangedBits(context, newValue, oldValue); + if (changedBits !== 0) { + // The context value changed. Search for matching consumers and + // schedule them to update. + propagateContextChange( + workInProgress, + context, + changedBits, + renderLanes, + ); + } + } + } + } + parent = parent.return; + } + + // This is an optimization so that we only propagate each provider once per + // subtree. (We will propagate the same provider to different subtrees, though + // — that's why the flag is on the fiber that bailed out, not the provider.) + // If a deeply nested child bails out, and it calls this propagation function, + // it uses this flag to know that the remaining ancestor providers have + // already been propagated. + workInProgress.flags |= DidPropagateContext; +} + export function checkIfContextChanged(currentDependencies: Dependencies) { if (!enableLazyContextPropagation) { return false; diff --git a/packages/react-reconciler/src/ReactFiberThrow.new.js b/packages/react-reconciler/src/ReactFiberThrow.new.js index 9ff0715f1621b..ba5a81ecccbf2 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.new.js +++ b/packages/react-reconciler/src/ReactFiberThrow.new.js @@ -38,6 +38,7 @@ import {NoMode, BlockingMode, DebugTracingMode} from './ReactTypeOfMode'; import { enableDebugTracing, enableSchedulingProfiler, + enableLazyContextPropagation, } from 'shared/ReactFeatureFlags'; import {createCapturedValue} from './ReactCapturedValue'; import { @@ -60,6 +61,7 @@ import { isAlreadyFailedLegacyErrorBoundary, pingSuspendedRoot, } from './ReactFiberWorkLoop.new'; +import {propagateParentContextChangesToDeferredTree} from './ReactFiberNewContext.new'; import {logCapturedError} from './ReactFiberErrorLogger'; import {logComponentSuspended} from './DebugTracing'; import {markComponentSuspended} from './SchedulingProfiler'; @@ -194,6 +196,23 @@ function throwException( typeof value === 'object' && typeof value.then === 'function' ) { + if (enableLazyContextPropagation) { + const currentSourceFiber = sourceFiber.alternate; + if (currentSourceFiber !== null) { + // Since we never visited the children of the suspended component, we + // need to propagate the context change now, to ensure that we visit + // them during the retry. + // + // We don't have to do this for errors because we retry errors without + // committing in between. So this is specific to Suspense. + propagateParentContextChangesToDeferredTree( + currentSourceFiber, + sourceFiber, + rootRenderLanes, + ); + } + } + // This is a wakeable. const wakeable: Wakeable = (value: any); diff --git a/packages/react-reconciler/src/ReactFiberThrow.old.js b/packages/react-reconciler/src/ReactFiberThrow.old.js index da0d90f057f6e..baa807e180dd0 100644 --- a/packages/react-reconciler/src/ReactFiberThrow.old.js +++ b/packages/react-reconciler/src/ReactFiberThrow.old.js @@ -38,6 +38,7 @@ import {NoMode, BlockingMode, DebugTracingMode} from './ReactTypeOfMode'; import { enableDebugTracing, enableSchedulingProfiler, + enableLazyContextPropagation, } from 'shared/ReactFeatureFlags'; import {createCapturedValue} from './ReactCapturedValue'; import { @@ -60,6 +61,7 @@ import { isAlreadyFailedLegacyErrorBoundary, pingSuspendedRoot, } from './ReactFiberWorkLoop.old'; +import {propagateParentContextChangesToDeferredTree} from './ReactFiberNewContext.old'; import {logCapturedError} from './ReactFiberErrorLogger'; import {logComponentSuspended} from './DebugTracing'; import {markComponentSuspended} from './SchedulingProfiler'; @@ -194,6 +196,23 @@ function throwException( typeof value === 'object' && typeof value.then === 'function' ) { + if (enableLazyContextPropagation) { + const currentSourceFiber = sourceFiber.alternate; + if (currentSourceFiber !== null) { + // Since we never visited the children of the suspended component, we + // need to propagate the context change now, to ensure that we visit + // them during the retry. + // + // We don't have to do this for errors because we retry errors without + // committing in between. So this is specific to Suspense. + propagateParentContextChangesToDeferredTree( + currentSourceFiber, + sourceFiber, + rootRenderLanes, + ); + } + } + // This is a wakeable. const wakeable: Wakeable = (value: any); diff --git a/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js b/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js index bdc89014ad640..557cea6931f42 100644 --- a/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js +++ b/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js @@ -3,6 +3,11 @@ let ReactNoop; let Scheduler; let useState; let useContext; +let Suspense; +let SuspenseList; +let getCacheForType; +let caches; +let seededCache; describe('ReactLazyContextPropagation', () => { beforeEach(() => { @@ -13,17 +18,148 @@ describe('ReactLazyContextPropagation', () => { Scheduler = require('scheduler'); useState = React.useState; useContext = React.useContext; + Suspense = React.Suspense; + SuspenseList = React.unstable_SuspenseList; + + getCacheForType = React.unstable_getCacheForType; + + caches = []; + seededCache = null; }); + // NOTE: These tests are not specific to the lazy propagation (as opposed to + // eager propagation). The behavior should be the same in both + // implementations. These are tests that are more relevant to the lazy + // propagation implementation, though. + + function createTextCache() { + if (seededCache !== null) { + // Trick to seed a cache before it exists. + // TODO: Need a built-in API to seed data before the initial render (i.e. + // not a refresh because nothing has mounted yet). + const cache = seededCache; + seededCache = null; + return cache; + } + + const data = new Map(); + const version = caches.length + 1; + const cache = { + version, + data, + resolve(text) { + const record = data.get(text); + if (record === undefined) { + const newRecord = { + status: 'resolved', + value: text, + }; + data.set(text, newRecord); + } else if (record.status === 'pending') { + const thenable = record.value; + record.status = 'resolved'; + record.value = text; + thenable.pings.forEach(t => t()); + } + }, + reject(text, error) { + const record = data.get(text); + if (record === undefined) { + const newRecord = { + status: 'rejected', + value: error, + }; + data.set(text, newRecord); + } else if (record.status === 'pending') { + const thenable = record.value; + record.status = 'rejected'; + record.value = error; + thenable.pings.forEach(t => t()); + } + }, + }; + caches.push(cache); + return cache; + } + + function readText(text) { + const textCache = getCacheForType(createTextCache); + const record = textCache.data.get(text); + if (record !== undefined) { + switch (record.status) { + case 'pending': + Scheduler.unstable_yieldValue(`Suspend! [${text}]`); + throw record.value; + case 'rejected': + Scheduler.unstable_yieldValue(`Error! [${text}]`); + throw record.value; + case 'resolved': + return textCache.version; + } + } else { + Scheduler.unstable_yieldValue(`Suspend! [${text}]`); + + const thenable = { + pings: [], + then(resolve) { + if (newRecord.status === 'pending') { + thenable.pings.push(resolve); + } else { + Promise.resolve().then(() => resolve(newRecord.value)); + } + }, + }; + + const newRecord = { + status: 'pending', + value: thenable, + }; + textCache.data.set(text, newRecord); + + throw thenable; + } + } + function Text({text}) { Scheduler.unstable_yieldValue(text); return text; } - // NOTE: These tests are not specific to the lazy propagation (as opposed to - // eager propagation). The behavior should be the same in both - // implementations. These are tests that are more relevant to the lazy - // propagation implementation, though. + // function AsyncText({text, showVersion}) { + // const version = readText(text); + // const fullText = showVersion ? `${text} [v${version}]` : text; + // Scheduler.unstable_yieldValue(fullText); + // return text; + // } + + function seedNextTextCache(text) { + if (seededCache === null) { + seededCache = createTextCache(); + } + seededCache.resolve(text); + } + + function resolveMostRecentTextCache(text) { + if (caches.length === 0) { + throw Error('Cache does not exist.'); + } else { + // Resolve the most recently created cache. An older cache can by + // resolved with `caches[index].resolve(text)`. + caches[caches.length - 1].resolve(text); + } + } + + const resolveText = resolveMostRecentTextCache; + + // function rejectMostRecentTextCache(text, error) { + // if (caches.length === 0) { + // throw Error('Cache does not exist.'); + // } else { + // // Resolve the most recently created cache. An older cache can by + // // resolved with `caches[index].reject(text, error)`. + // caches[caches.length - 1].reject(text, error); + // } + // } test( 'context change should prevent bailout of memoized component (useMemo -> ' + @@ -203,4 +339,356 @@ describe('ReactLazyContextPropagation', () => { expect(Scheduler).toHaveYielded(['Consumer']); expect(root).toMatchRenderedOutput('0'); }); + + // @gate enableCache + test('context is propagated across retries', async () => { + const root = ReactNoop.createRoot(); + + const Context = React.createContext('A'); + + let setContext; + function App() { + const [value, setValue] = useState('A'); + setContext = setValue; + return ( + + }> + + + + + ); + } + + function Async() { + const value = useContext(Context); + readText(value); + + // When `readText` suspends, we haven't yet visited Indirection and all + // of its children. They won't get rendered until a later retry. + return ; + } + + const Indirection = React.memo(() => { + // This child must always be consistent with the sibling Text component. + return ; + }); + + function DeepChild() { + const value = useContext(Context); + return ; + } + + await seedNextTextCache('A'); + await ReactNoop.act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['A', 'A']); + expect(root).toMatchRenderedOutput('AA'); + + await ReactNoop.act(async () => { + // Intentionally not wrapping in startTransition, so that the fallback + // the fallback displays despite this being a refresh. + setContext('B'); + }); + expect(Scheduler).toHaveYielded(['Suspend! [B]', 'Loading...', 'B']); + expect(root).toMatchRenderedOutput('Loading...B'); + + await ReactNoop.act(async () => { + await resolveText('B'); + }); + expect(Scheduler).toHaveYielded(['B']); + expect(root).toMatchRenderedOutput('BB'); + }); + + // @gate enableCache + test('multiple contexts are propagated across retries', async () => { + // Same as previous test, but with multiple context providers + const root = ReactNoop.createRoot(); + + const Context1 = React.createContext('A'); + const Context2 = React.createContext('A'); + + let setContext; + function App() { + const [value, setValue] = useState('A'); + setContext = setValue; + return ( + + + }> + + + + + + ); + } + + function Async() { + const value = useContext(Context1); + readText(value); + + // When `readText` suspends, we haven't yet visited Indirection and all + // of its children. They won't get rendered until a later retry. + return ( + <> + + + + ); + } + + const Indirection1 = React.memo(() => { + // This child must always be consistent with the sibling Text component. + return ; + }); + + const Indirection2 = React.memo(() => { + // This child must always be consistent with the sibling Text component. + return ; + }); + + function DeepChild1() { + const value = useContext(Context1); + return ; + } + + function DeepChild2() { + const value = useContext(Context2); + return ; + } + + await seedNextTextCache('A'); + await ReactNoop.act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['A', 'A', 'A']); + expect(root).toMatchRenderedOutput('AAA'); + + await ReactNoop.act(async () => { + // Intentionally not wrapping in startTransition, so that the fallback + // the fallback displays despite this being a refresh. + setContext('B'); + }); + expect(Scheduler).toHaveYielded(['Suspend! [B]', 'Loading...', 'B']); + expect(root).toMatchRenderedOutput('Loading...B'); + + await ReactNoop.act(async () => { + await resolveText('B'); + }); + expect(Scheduler).toHaveYielded(['B', 'B']); + expect(root).toMatchRenderedOutput('BBB'); + }); + + // @gate enableCache + test('context is propagated across retries (legacy)', async () => { + const root = ReactNoop.createLegacyRoot(); + + const Context = React.createContext('A'); + + let setContext; + function App() { + const [value, setValue] = useState('A'); + setContext = setValue; + return ( + + }> + + + + + ); + } + + function Async() { + const value = useContext(Context); + readText(value); + + // When `readText` suspends, we haven't yet visited Indirection and all + // of its children. They won't get rendered until a later retry. + return ; + } + + const Indirection = React.memo(() => { + // This child must always be consistent with the sibling Text component. + return ; + }); + + function DeepChild() { + const value = useContext(Context); + return ; + } + + await seedNextTextCache('A'); + await ReactNoop.act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['A', 'A']); + expect(root).toMatchRenderedOutput('AA'); + + await ReactNoop.act(async () => { + // Intentionally not wrapping in startTransition, so that the fallback + // the fallback displays despite this being a refresh. + setContext('B'); + }); + expect(Scheduler).toHaveYielded(['Suspend! [B]', 'Loading...', 'B']); + expect(root).toMatchRenderedOutput('Loading...B'); + + await ReactNoop.act(async () => { + await resolveText('B'); + }); + expect(Scheduler).toHaveYielded(['B']); + expect(root).toMatchRenderedOutput('BB'); + }); + + // @gate enableCache + test('context is propagated across through offscreen trees', async () => { + const LegacyHidden = React.unstable_LegacyHidden; + + const root = ReactNoop.createRoot(); + + const Context = React.createContext('A'); + + let setContext; + function App() { + const [value, setValue] = useState('A'); + setContext = setValue; + return ( + + + + + + + ); + } + + const Indirection = React.memo(() => { + // This child must always be consistent with the sibling Text component. + return ; + }); + + function DeepChild() { + const value = useContext(Context); + return ; + } + + await seedNextTextCache('A'); + await ReactNoop.act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['A', 'A']); + expect(root).toMatchRenderedOutput('AA'); + + await ReactNoop.act(async () => { + setContext('B'); + }); + expect(Scheduler).toHaveYielded(['B', 'B']); + expect(root).toMatchRenderedOutput('BB'); + }); + + // @gate enableCache + test('multiple contexts are propagated across through offscreen trees', async () => { + // Same as previous test, but with multiple context providers + const LegacyHidden = React.unstable_LegacyHidden; + + const root = ReactNoop.createRoot(); + + const Context1 = React.createContext('A'); + const Context2 = React.createContext('A'); + + let setContext; + function App() { + const [value, setValue] = useState('A'); + setContext = setValue; + return ( + + + + + + + + + + ); + } + + const Indirection1 = React.memo(() => { + // This child must always be consistent with the sibling Text component. + return ; + }); + + const Indirection2 = React.memo(() => { + // This child must always be consistent with the sibling Text component. + return ; + }); + + function DeepChild1() { + const value = useContext(Context1); + return ; + } + + function DeepChild2() { + const value = useContext(Context2); + return ; + } + + await seedNextTextCache('A'); + await ReactNoop.act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['A', 'A', 'A']); + expect(root).toMatchRenderedOutput('AAA'); + + await ReactNoop.act(async () => { + setContext('B'); + }); + expect(Scheduler).toHaveYielded(['B', 'B', 'B']); + expect(root).toMatchRenderedOutput('BBB'); + }); + + // @gate enableCache + // @gate experimental + test('contexts are propagated through SuspenseList', async () => { + // This kinda tests an implementation detail. SuspenseList has an early + // bailout that doesn't use `bailoutOnAlreadyFinishedWork`. It probably + // should just use that function, though. + const Context = React.createContext('A'); + + let setContext; + function App() { + const [value, setValue] = useState('A'); + setContext = setValue; + const children = React.useMemo( + () => ( + + + + + ), + [], + ); + return {children}; + } + + function Child() { + const value = useContext(Context); + return ; + } + + const root = ReactNoop.createRoot(); + await ReactNoop.act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['A', 'A']); + expect(root).toMatchRenderedOutput('AA'); + + await ReactNoop.act(async () => { + setContext('B'); + }); + expect(Scheduler).toHaveYielded(['B', 'B']); + expect(root).toMatchRenderedOutput('BB'); + }); }); diff --git a/packages/react-reconciler/src/__tests__/ReactNewContext-test.js b/packages/react-reconciler/src/__tests__/ReactNewContext-test.js index 3f7f4f992955b..43c1e0033c2f3 100644 --- a/packages/react-reconciler/src/__tests__/ReactNewContext-test.js +++ b/packages/react-reconciler/src/__tests__/ReactNewContext-test.js @@ -689,6 +689,7 @@ describe('ReactNewContext', () => { ]); }); + // @gate !enableLazyContextPropagation it('can skip parents with bitmask bailout while updating their children', () => { const Context = React.createContext({foo: 0, bar: 0}, (a, b) => { let result = 0; @@ -1077,10 +1078,13 @@ describe('ReactNewContext', () => { // Update ReactNoop.render(); - expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev( - 'calculateChangedBits: Expected the return value to be a 31-bit ' + - 'integer. Instead received: 4294967295', - ); + + if (gate(flags => !flags.enableLazyContextPropagation)) { + expect(() => expect(Scheduler).toFlushWithoutYielding()).toErrorDev( + 'calculateChangedBits: Expected the return value to be a 31-bit ' + + 'integer. Instead received: 4294967295', + ); + } }); it('warns if no value prop provided', () => { From 1a7f224ce51a3dd687ea02690e955c1f178479cc Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Sat, 27 Feb 2021 15:36:12 -0600 Subject: [PATCH 3/5] Propagate all contexts in single pass Instead of propagating the tree once per changed context, we can check all the contexts in a single propagation. This inverts the two loops so that the faster loop (O(numberOfContexts)) is inside the more expensive loop (O(numberOfFibers * avgContextDepsPerFiber)). This adds a bit of overhead to the case where only a single context changes because you have to unwrap the context from the array. I'm also unsure if this will hurt cache locality. Co-Authored-By: Josh Story --- .../src/ReactFiberNewContext.new.js | 253 +++++++++++++----- .../src/ReactFiberNewContext.old.js | 253 +++++++++++++----- 2 files changed, 372 insertions(+), 134 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberNewContext.new.js b/packages/react-reconciler/src/ReactFiberNewContext.new.js index 55d35510e2299..196042a26a877 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.new.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.new.js @@ -196,6 +196,32 @@ export function propagateContextChange( changedBits: number, renderLanes: Lanes, ): void { + if (enableLazyContextPropagation) { + propagateContextChanges( + workInProgress, + [context, changedBits], + renderLanes, + ); + } else { + propagateContextChange_eager( + workInProgress, + context, + changedBits, + renderLanes, + ); + } +} + +function propagateContextChange_eager( + workInProgress: Fiber, + context: ReactContext, + changedBits: number, + renderLanes: Lanes, +): void { + // Only used by eager implemenation + if (enableLazyContextPropagation) { + return; + } let fiber = workInProgress.child; if (fiber !== null) { // Set the return pointer of the child to the work-in-progress fiber. @@ -217,45 +243,32 @@ export function propagateContextChange( (dependency.observedBits & changedBits) !== 0 ) { // Match! Schedule an update on this fiber. - - if (enableLazyContextPropagation) { - // In the lazy implemenation, don't mark a dirty flag on the - // dependency itself. Not all changes are propagated, so we can't - // rely on the propagation function alone to determine whether - // something has changed; the consumer will check. In the future, - // we could add back a dirty flag as an optimization to avoid - // double checking, but until we have selectors it's not really - // worth the trouble. - } else { - if (fiber.tag === ClassComponent) { - // Schedule a force update on the work-in-progress. - const lane = pickArbitraryLane(renderLanes); - const update = createUpdate(NoTimestamp, lane); - update.tag = ForceUpdate; - // TODO: Because we don't have a work-in-progress, this will add the - // update to the current fiber, too, which means it will persist even if - // this render is thrown away. Since it's a race condition, not sure it's - // worth fixing. - - // Inlined `enqueueUpdate` to remove interleaved update check - const updateQueue = fiber.updateQueue; - if (updateQueue === null) { - // Only occurs if the fiber has been unmounted. + if (fiber.tag === ClassComponent) { + // Schedule a force update on the work-in-progress. + const lane = pickArbitraryLane(renderLanes); + const update = createUpdate(NoTimestamp, lane); + update.tag = ForceUpdate; + // TODO: Because we don't have a work-in-progress, this will add the + // update to the current fiber, too, which means it will persist even if + // this render is thrown away. Since it's a race condition, not sure it's + // worth fixing. + + // Inlined `enqueueUpdate` to remove interleaved update check + const updateQueue = fiber.updateQueue; + if (updateQueue === null) { + // Only occurs if the fiber has been unmounted. + } else { + const sharedQueue: SharedQueue = (updateQueue: any).shared; + const pending = sharedQueue.pending; + if (pending === null) { + // This is the first update. Create a circular list. + update.next = update; } else { - const sharedQueue: SharedQueue = (updateQueue: any).shared; - const pending = sharedQueue.pending; - if (pending === null) { - // This is the first update. Create a circular list. - update.next = update; - } else { - update.next = pending.next; - pending.next = update; - } - sharedQueue.pending = update; + update.next = pending.next; + pending.next = update; } + sharedQueue.pending = update; } - // Mark the updated lanes on the list, too. - list.lanes = mergeLanes(list.lanes, renderLanes); } fiber.lanes = mergeLanes(fiber.lanes, renderLanes); @@ -265,6 +278,9 @@ export function propagateContextChange( } scheduleWorkOnParentPath(fiber.return, renderLanes); + // Mark the updated lanes on the list, too. + list.lanes = mergeLanes(list.lanes, renderLanes); + // Since we already found a match, we can stop traversing the // dependency list. break; @@ -329,36 +345,136 @@ export function propagateContextChange( } } -export function lazilyPropagateParentContextChanges( - current: Fiber, +function propagateContextChanges( workInProgress: Fiber, + contexts: Array, renderLanes: Lanes, -) { - propagateParentContextChanges(current, workInProgress, renderLanes, false); -} +): void { + // Only used by lazy implemenation + if (!enableLazyContextPropagation) { + return; + } + let fiber = workInProgress.child; + if (fiber !== null) { + // Set the return pointer of the child to the work-in-progress fiber. + fiber.return = workInProgress; + } + while (fiber !== null) { + let nextFiber; -export function propagateParentContextChangesToDeferredTree( - current: Fiber, - workInProgress: Fiber, - renderLanes: Lanes, -) { - propagateParentContextChanges(current, workInProgress, renderLanes, true); + // Visit this fiber. + const list = fiber.dependencies; + if (list !== null) { + nextFiber = fiber.child; + + let dep = list.firstContext; + findChangedDep: while (dep !== null) { + // Assigning these to constants to help Flow + const dependency = dep; + const consumer = fiber; + findContext: for (let i = 0; i < contexts.length; i += 2) { + const context: ReactContext = contexts[i]; + const changedBits: number = contexts[i + 1]; + // Check if the context matches. + // TODO: Compare selected values to bail out early. + if ( + dependency.context === context && + (dependency.observedBits & changedBits) !== 0 + ) { + // Match! Schedule an update on this fiber. + + // In the lazy implemenation, don't mark a dirty flag on the + // dependency itself. Not all changes are propagated, so we can't + // rely on the propagation function alone to determine whether + // something has changed; the consumer will check. In the future, we + // could add back a dirty flag as an optimization to avoid double + // checking, but until we have selectors it's not really worth + // the trouble. + consumer.lanes = mergeLanes(consumer.lanes, renderLanes); + const alternate = consumer.alternate; + if (alternate !== null) { + alternate.lanes = mergeLanes(alternate.lanes, renderLanes); + } + scheduleWorkOnParentPath(consumer.return, renderLanes); + + // Since we already found a match, we can stop traversing the + // dependency list. + break findChangedDep; + } + } + dep = dependency.next; + } + } else if ( + enableSuspenseServerRenderer && + fiber.tag === DehydratedFragment + ) { + // If a dehydrated suspense boundary is in this subtree, we don't know + // if it will have any context consumers in it. The best we can do is + // mark it as having updates. + const parentSuspense = fiber.return; + invariant( + parentSuspense !== null, + 'We just came from a parent so we must have had a parent. This is a bug in React.', + ); + parentSuspense.lanes = mergeLanes(parentSuspense.lanes, renderLanes); + const alternate = parentSuspense.alternate; + if (alternate !== null) { + alternate.lanes = mergeLanes(alternate.lanes, renderLanes); + } + // This is intentionally passing this fiber as the parent + // because we want to schedule this fiber as having work + // on its children. We'll use the childLanes on + // this fiber to indicate that a context has changed. + scheduleWorkOnParentPath(parentSuspense, renderLanes); + nextFiber = fiber.sibling; + } else { + // Traverse down. + nextFiber = fiber.child; + } + + if (nextFiber !== null) { + // Set the return pointer of the child to the work-in-progress fiber. + nextFiber.return = fiber; + } else { + // No child. Traverse to next sibling. + nextFiber = fiber; + while (nextFiber !== null) { + if (nextFiber === workInProgress) { + // We're back to the root of this subtree. Exit. + nextFiber = null; + break; + } + const sibling = nextFiber.sibling; + if (sibling !== null) { + // Set the return pointer of the sibling to the work-in-progress fiber. + sibling.return = nextFiber.return; + nextFiber = sibling; + break; + } + // No more siblings. Traverse up. + nextFiber = nextFiber.return; + } + } + fiber = nextFiber; + } } -function propagateParentContextChanges( +// Alias for propagating a deferred tree (Suspense, Offscreen). Currently it's +// the same algorithm but there may be a way to optimize one or the other. +export const propagateParentContextChangesToDeferredTree = lazilyPropagateParentContextChanges; + +export function lazilyPropagateParentContextChanges( current: Fiber, workInProgress: Fiber, renderLanes: Lanes, - // TODO: This argument is currently unused. I think there's a way to optimize - // for the many providers case, where if the first propagation finds a match, - // the second one can avoid scanning down that same path. The trouble is that - // there could be a nested bailout below that. - forcePropagateEntireTree: boolean, ) { if (!enableLazyContextPropagation) { return false; } + // Collect all the parent providers that changed. Since this is usually small + // number, we use an Array instead of Set. + let contexts = null; let parent = workInProgress; while (parent !== null && (parent.flags & DidPropagateContext) === NoFlags) { if (parent.tag === ContextProvider) { @@ -376,14 +492,11 @@ function propagateParentContextChanges( const changedBits = calculateChangedBits(context, newValue, oldValue); if (changedBits !== 0) { - // The context value changed. Search for matching consumers and - // schedule them to update. - propagateContextChange( - workInProgress, - context, - changedBits, - renderLanes, - ); + if (contexts !== null) { + contexts.push(context, changedBits); + } else { + contexts = [context, changedBits]; + } } } } @@ -391,12 +504,18 @@ function propagateParentContextChanges( parent = parent.return; } - // This is an optimization so that we only propagate each provider once per - // subtree. (We will propagate the same provider to different subtrees, though - // — that's why the flag is on the fiber that bailed out, not the provider.) - // If a deeply nested child bails out, and it calls this propagation function, - // it uses this flag to know that the remaining ancestor providers have - // already been propagated. + if (contexts !== null) { + // If there were any changed providers, search through the children and + // propagate their changes. + propagateContextChanges(workInProgress, contexts, renderLanes); + } + + // This is an optimization so that we only propagate once per subtree. (We + // will propagate the same providers to different subtrees, though — that's + // why the flag is on the fiber that bailed out, not the provider.) If a + // deeply nested child bails out, and it calls this propagation function, it + // uses this flag to know that the remaining ancestor providers have already + // been propagated. workInProgress.flags |= DidPropagateContext; } diff --git a/packages/react-reconciler/src/ReactFiberNewContext.old.js b/packages/react-reconciler/src/ReactFiberNewContext.old.js index f6025075bdb87..124865418850a 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.old.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.old.js @@ -196,6 +196,32 @@ export function propagateContextChange( changedBits: number, renderLanes: Lanes, ): void { + if (enableLazyContextPropagation) { + propagateContextChanges( + workInProgress, + [context, changedBits], + renderLanes, + ); + } else { + propagateContextChange_eager( + workInProgress, + context, + changedBits, + renderLanes, + ); + } +} + +function propagateContextChange_eager( + workInProgress: Fiber, + context: ReactContext, + changedBits: number, + renderLanes: Lanes, +): void { + // Only used by eager implemenation + if (enableLazyContextPropagation) { + return; + } let fiber = workInProgress.child; if (fiber !== null) { // Set the return pointer of the child to the work-in-progress fiber. @@ -217,45 +243,32 @@ export function propagateContextChange( (dependency.observedBits & changedBits) !== 0 ) { // Match! Schedule an update on this fiber. - - if (enableLazyContextPropagation) { - // In the lazy implemenation, don't mark a dirty flag on the - // dependency itself. Not all changes are propagated, so we can't - // rely on the propagation function alone to determine whether - // something has changed; the consumer will check. In the future, - // we could add back a dirty flag as an optimization to avoid - // double checking, but until we have selectors it's not really - // worth the trouble. - } else { - if (fiber.tag === ClassComponent) { - // Schedule a force update on the work-in-progress. - const lane = pickArbitraryLane(renderLanes); - const update = createUpdate(NoTimestamp, lane); - update.tag = ForceUpdate; - // TODO: Because we don't have a work-in-progress, this will add the - // update to the current fiber, too, which means it will persist even if - // this render is thrown away. Since it's a race condition, not sure it's - // worth fixing. - - // Inlined `enqueueUpdate` to remove interleaved update check - const updateQueue = fiber.updateQueue; - if (updateQueue === null) { - // Only occurs if the fiber has been unmounted. + if (fiber.tag === ClassComponent) { + // Schedule a force update on the work-in-progress. + const lane = pickArbitraryLane(renderLanes); + const update = createUpdate(NoTimestamp, lane); + update.tag = ForceUpdate; + // TODO: Because we don't have a work-in-progress, this will add the + // update to the current fiber, too, which means it will persist even if + // this render is thrown away. Since it's a race condition, not sure it's + // worth fixing. + + // Inlined `enqueueUpdate` to remove interleaved update check + const updateQueue = fiber.updateQueue; + if (updateQueue === null) { + // Only occurs if the fiber has been unmounted. + } else { + const sharedQueue: SharedQueue = (updateQueue: any).shared; + const pending = sharedQueue.pending; + if (pending === null) { + // This is the first update. Create a circular list. + update.next = update; } else { - const sharedQueue: SharedQueue = (updateQueue: any).shared; - const pending = sharedQueue.pending; - if (pending === null) { - // This is the first update. Create a circular list. - update.next = update; - } else { - update.next = pending.next; - pending.next = update; - } - sharedQueue.pending = update; + update.next = pending.next; + pending.next = update; } + sharedQueue.pending = update; } - // Mark the updated lanes on the list, too. - list.lanes = mergeLanes(list.lanes, renderLanes); } fiber.lanes = mergeLanes(fiber.lanes, renderLanes); @@ -265,6 +278,9 @@ export function propagateContextChange( } scheduleWorkOnParentPath(fiber.return, renderLanes); + // Mark the updated lanes on the list, too. + list.lanes = mergeLanes(list.lanes, renderLanes); + // Since we already found a match, we can stop traversing the // dependency list. break; @@ -329,36 +345,136 @@ export function propagateContextChange( } } -export function lazilyPropagateParentContextChanges( - current: Fiber, +function propagateContextChanges( workInProgress: Fiber, + contexts: Array, renderLanes: Lanes, -) { - propagateParentContextChanges(current, workInProgress, renderLanes, false); -} +): void { + // Only used by lazy implemenation + if (!enableLazyContextPropagation) { + return; + } + let fiber = workInProgress.child; + if (fiber !== null) { + // Set the return pointer of the child to the work-in-progress fiber. + fiber.return = workInProgress; + } + while (fiber !== null) { + let nextFiber; -export function propagateParentContextChangesToDeferredTree( - current: Fiber, - workInProgress: Fiber, - renderLanes: Lanes, -) { - propagateParentContextChanges(current, workInProgress, renderLanes, true); + // Visit this fiber. + const list = fiber.dependencies; + if (list !== null) { + nextFiber = fiber.child; + + let dep = list.firstContext; + findChangedDep: while (dep !== null) { + // Assigning these to constants to help Flow + const dependency = dep; + const consumer = fiber; + findContext: for (let i = 0; i < contexts.length; i += 2) { + const context: ReactContext = contexts[i]; + const changedBits: number = contexts[i + 1]; + // Check if the context matches. + // TODO: Compare selected values to bail out early. + if ( + dependency.context === context && + (dependency.observedBits & changedBits) !== 0 + ) { + // Match! Schedule an update on this fiber. + + // In the lazy implemenation, don't mark a dirty flag on the + // dependency itself. Not all changes are propagated, so we can't + // rely on the propagation function alone to determine whether + // something has changed; the consumer will check. In the future, we + // could add back a dirty flag as an optimization to avoid double + // checking, but until we have selectors it's not really worth + // the trouble. + consumer.lanes = mergeLanes(consumer.lanes, renderLanes); + const alternate = consumer.alternate; + if (alternate !== null) { + alternate.lanes = mergeLanes(alternate.lanes, renderLanes); + } + scheduleWorkOnParentPath(consumer.return, renderLanes); + + // Since we already found a match, we can stop traversing the + // dependency list. + break findChangedDep; + } + } + dep = dependency.next; + } + } else if ( + enableSuspenseServerRenderer && + fiber.tag === DehydratedFragment + ) { + // If a dehydrated suspense boundary is in this subtree, we don't know + // if it will have any context consumers in it. The best we can do is + // mark it as having updates. + const parentSuspense = fiber.return; + invariant( + parentSuspense !== null, + 'We just came from a parent so we must have had a parent. This is a bug in React.', + ); + parentSuspense.lanes = mergeLanes(parentSuspense.lanes, renderLanes); + const alternate = parentSuspense.alternate; + if (alternate !== null) { + alternate.lanes = mergeLanes(alternate.lanes, renderLanes); + } + // This is intentionally passing this fiber as the parent + // because we want to schedule this fiber as having work + // on its children. We'll use the childLanes on + // this fiber to indicate that a context has changed. + scheduleWorkOnParentPath(parentSuspense, renderLanes); + nextFiber = fiber.sibling; + } else { + // Traverse down. + nextFiber = fiber.child; + } + + if (nextFiber !== null) { + // Set the return pointer of the child to the work-in-progress fiber. + nextFiber.return = fiber; + } else { + // No child. Traverse to next sibling. + nextFiber = fiber; + while (nextFiber !== null) { + if (nextFiber === workInProgress) { + // We're back to the root of this subtree. Exit. + nextFiber = null; + break; + } + const sibling = nextFiber.sibling; + if (sibling !== null) { + // Set the return pointer of the sibling to the work-in-progress fiber. + sibling.return = nextFiber.return; + nextFiber = sibling; + break; + } + // No more siblings. Traverse up. + nextFiber = nextFiber.return; + } + } + fiber = nextFiber; + } } -function propagateParentContextChanges( +// Alias for propagating a deferred tree (Suspense, Offscreen). Currently it's +// the same algorithm but there may be a way to optimize one or the other. +export const propagateParentContextChangesToDeferredTree = lazilyPropagateParentContextChanges; + +export function lazilyPropagateParentContextChanges( current: Fiber, workInProgress: Fiber, renderLanes: Lanes, - // TODO: This argument is currently unused. I think there's a way to optimize - // for the many providers case, where if the first propagation finds a match, - // the second one can avoid scanning down that same path. The trouble is that - // there could be a nested bailout below that. - forcePropagateEntireTree: boolean, ) { if (!enableLazyContextPropagation) { return false; } + // Collect all the parent providers that changed. Since this is usually small + // number, we use an Array instead of Set. + let contexts = null; let parent = workInProgress; while (parent !== null && (parent.flags & DidPropagateContext) === NoFlags) { if (parent.tag === ContextProvider) { @@ -376,14 +492,11 @@ function propagateParentContextChanges( const changedBits = calculateChangedBits(context, newValue, oldValue); if (changedBits !== 0) { - // The context value changed. Search for matching consumers and - // schedule them to update. - propagateContextChange( - workInProgress, - context, - changedBits, - renderLanes, - ); + if (contexts !== null) { + contexts.push(context, changedBits); + } else { + contexts = [context, changedBits]; + } } } } @@ -391,12 +504,18 @@ function propagateParentContextChanges( parent = parent.return; } - // This is an optimization so that we only propagate each provider once per - // subtree. (We will propagate the same provider to different subtrees, though - // — that's why the flag is on the fiber that bailed out, not the provider.) - // If a deeply nested child bails out, and it calls this propagation function, - // it uses this flag to know that the remaining ancestor providers have - // already been propagated. + if (contexts !== null) { + // If there were any changed providers, search through the children and + // propagate their changes. + propagateContextChanges(workInProgress, contexts, renderLanes); + } + + // This is an optimization so that we only propagate once per subtree. (We + // will propagate the same providers to different subtrees, though — that's + // why the flag is on the fiber that bailed out, not the provider.) If a + // deeply nested child bails out, and it calls this propagation function, it + // uses this flag to know that the remaining ancestor providers have already + // been propagated. workInProgress.flags |= DidPropagateContext; } From 9146a1713970d12f401c266fc53c507bf21223c4 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 1 Mar 2021 00:16:37 -0600 Subject: [PATCH 4/5] Stop propagating at nearest dependency match MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Because we now propagate all context providers in a single traversal, we can defer context propagation to a subtree without losing information about which context providers we're deferring — it's all of them. Theoretically, this is a big optimization because it means we'll never propagate to any tree that has work scheduled on it, nor will we ever propagate the same tree twice. There's an awkward case related to bailing out of the siblings of a context consumer. Because those siblings don't bail out until after they've already entered the begin phase, we have to do extra work to make sure they don't unecessarily propagate context again. We could avoid this by adding an earlier bailout for sibling nodes, something we've discussed in the past. We should consider this during the next refactor of the fiber tree structure. Co-Authored-By: Josh Story --- .../src/ReactFiberNewContext.new.js | 139 ++++++++++- .../src/ReactFiberNewContext.old.js | 139 ++++++++++- .../src/ReactFiberWorkLoop.new.js | 7 +- .../src/ReactFiberWorkLoop.old.js | 7 +- .../__tests__/ReactContextPropagation-test.js | 234 +++++++++++++++++- scripts/error-codes/codes.json | 4 +- 6 files changed, 502 insertions(+), 28 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberNewContext.new.js b/packages/react-reconciler/src/ReactFiberNewContext.new.js index 196042a26a877..6385d14e2c006 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.new.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.new.js @@ -197,10 +197,15 @@ export function propagateContextChange( renderLanes: Lanes, ): void { if (enableLazyContextPropagation) { + // TODO: This path is only used by Cache components. Update + // lazilyPropagateParentContextChanges to look for Cache components so they + // can take advantage of lazy propagation. + const forcePropagateEntireTree = true; propagateContextChanges( workInProgress, [context, changedBits], renderLanes, + forcePropagateEntireTree, ); } else { propagateContextChange_eager( @@ -349,6 +354,7 @@ function propagateContextChanges( workInProgress: Fiber, contexts: Array, renderLanes: Lanes, + forcePropagateEntireTree: boolean, ): void { // Only used by lazy implemenation if (!enableLazyContextPropagation) { @@ -397,6 +403,22 @@ function propagateContextChanges( } scheduleWorkOnParentPath(consumer.return, renderLanes); + if (!forcePropagateEntireTree) { + // During lazy propagation, when we find a match, we can defer + // propagating changes to the children, because we're going to + // visit them during render. We should continue propagating the + // siblings, though + nextFiber = null; + + // Keep track of subtrees whose propagation we deferred + if (deferredPropagation === null) { + deferredPropagation = new Set([consumer]); + } else { + deferredPropagation.add(consumer); + } + nextFiber = null; + } + // Since we already found a match, we can stop traversing the // dependency list. break findChangedDep; @@ -426,7 +448,7 @@ function propagateContextChanges( // on its children. We'll use the childLanes on // this fiber to indicate that a context has changed. scheduleWorkOnParentPath(parentSuspense, renderLanes); - nextFiber = fiber.sibling; + nextFiber = null; } else { // Traverse down. nextFiber = fiber.child; @@ -459,14 +481,58 @@ function propagateContextChanges( } } -// Alias for propagating a deferred tree (Suspense, Offscreen). Currently it's -// the same algorithm but there may be a way to optimize one or the other. -export const propagateParentContextChangesToDeferredTree = lazilyPropagateParentContextChanges; - export function lazilyPropagateParentContextChanges( current: Fiber, workInProgress: Fiber, renderLanes: Lanes, +) { + const forcePropagateEntireTree = false; + propagateParentContextChanges( + current, + workInProgress, + renderLanes, + forcePropagateEntireTree, + ); +} + +// Used for propagating a deferred tree (Suspense, Offscreen). We must propagate +// to the entire subtree, because we won't revisit it until after the current +// render has completed, at which point we'll have lost track of which providers +// have changed. +export function propagateParentContextChangesToDeferredTree( + current: Fiber, + workInProgress: Fiber, + renderLanes: Lanes, +) { + const forcePropagateEntireTree = true; + propagateParentContextChanges( + current, + workInProgress, + renderLanes, + forcePropagateEntireTree, + ); +} + +// Used by lazy context propagation algorithm. When we find a context dependency +// match, we don't propagate the changes any further into that fiber's subtree. +// We add the matched fibers to this set. Later, if something inside that +// subtree bails out of rendering, the presence of a parent fiber in this Set +// tells us that we need to continue propagating. +// +// This is a set of _current_ fibers, not work-in-progress fibers. That's why +// it's a set instead of a flag on the fiber. +let deferredPropagation: Set | null = null; + +export function resetDeferredContextPropagation() { + // This is called by prepareFreshStack + deferredPropagation = null; +} + +function propagateParentContextChanges( + current: Fiber, + workInProgress: Fiber, + renderLanes: Lanes, + forcePropagateEntireTree: boolean, ) { if (!enableLazyContextPropagation) { return false; @@ -476,9 +542,42 @@ export function lazilyPropagateParentContextChanges( // number, we use an Array instead of Set. let contexts = null; let parent = workInProgress; - while (parent !== null && (parent.flags & DidPropagateContext) === NoFlags) { + let isInsidePropagationBailout = false; + while (parent !== null) { + const currentParent = parent.alternate; + invariant( + currentParent !== null, + 'Should have a current fiber. This is a bug in React.', + ); + + if (!isInsidePropagationBailout) { + if (deferredPropagation === null) { + if ((parent.flags & DidPropagateContext) !== NoFlags) { + break; + } + } else { + if (currentParent !== null && deferredPropagation.has(currentParent)) { + // We're inside a subtree that previously bailed out of propagation. + // We must disregard the the DidPropagateContext flag as we continue + // searching for parent providers. + isInsidePropagationBailout = true; + // We know that none of the providers in between the propagation + // bailout and the nearest render bailout above that could have + // changed. So we can skip those. + do { + parent = parent.return; + invariant( + parent !== null, + 'Expected to find a bailed out fiber. This is a bug in React.', + ); + } while ((parent.flags & DidPropagateContext) === NoFlags); + } else if ((parent.flags & DidPropagateContext) !== NoFlags) { + break; + } + } + } + if (parent.tag === ContextProvider) { - const currentParent = parent.alternate; if (currentParent !== null) { const oldProps = currentParent.memoizedProps; if (oldProps !== null) { @@ -507,15 +606,33 @@ export function lazilyPropagateParentContextChanges( if (contexts !== null) { // If there were any changed providers, search through the children and // propagate their changes. - propagateContextChanges(workInProgress, contexts, renderLanes); + propagateContextChanges( + workInProgress, + contexts, + renderLanes, + forcePropagateEntireTree, + ); } - // This is an optimization so that we only propagate once per subtree. (We - // will propagate the same providers to different subtrees, though — that's - // why the flag is on the fiber that bailed out, not the provider.) If a + // This is an optimization so that we only propagate once per subtree. If a // deeply nested child bails out, and it calls this propagation function, it // uses this flag to know that the remaining ancestor providers have already // been propagated. + // + // NOTE: This optimization is only necessary because we sometimes enter the + // begin phase of nodes that don't have any work scheduled on them — + // specifically, the siblings of a node that _does_ have scheduled work. The + // siblings will bail out and call this function again, even though we already + // propagated content changes to it and its subtree. So we use this flag to + // mark that the parent providers already propagated. + // + // Unfortunately, though, we need to ignore this flag when we're inside a + // tree whose context propagation was deferred — that's what the + // `deferredPropagation` set is for. + // + // If we could instead bail out before entering the siblings' beging phase, + // then we could remove both `DidPropagateContext` and `deferredPropagation`. + // Consider this as part of the next refactor to the fiber tree structure. workInProgress.flags |= DidPropagateContext; } diff --git a/packages/react-reconciler/src/ReactFiberNewContext.old.js b/packages/react-reconciler/src/ReactFiberNewContext.old.js index 124865418850a..5575b4475fbf6 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.old.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.old.js @@ -197,10 +197,15 @@ export function propagateContextChange( renderLanes: Lanes, ): void { if (enableLazyContextPropagation) { + // TODO: This path is only used by Cache components. Update + // lazilyPropagateParentContextChanges to look for Cache components so they + // can take advantage of lazy propagation. + const forcePropagateEntireTree = true; propagateContextChanges( workInProgress, [context, changedBits], renderLanes, + forcePropagateEntireTree, ); } else { propagateContextChange_eager( @@ -349,6 +354,7 @@ function propagateContextChanges( workInProgress: Fiber, contexts: Array, renderLanes: Lanes, + forcePropagateEntireTree: boolean, ): void { // Only used by lazy implemenation if (!enableLazyContextPropagation) { @@ -397,6 +403,22 @@ function propagateContextChanges( } scheduleWorkOnParentPath(consumer.return, renderLanes); + if (!forcePropagateEntireTree) { + // During lazy propagation, when we find a match, we can defer + // propagating changes to the children, because we're going to + // visit them during render. We should continue propagating the + // siblings, though + nextFiber = null; + + // Keep track of subtrees whose propagation we deferred + if (deferredPropagation === null) { + deferredPropagation = new Set([consumer]); + } else { + deferredPropagation.add(consumer); + } + nextFiber = null; + } + // Since we already found a match, we can stop traversing the // dependency list. break findChangedDep; @@ -426,7 +448,7 @@ function propagateContextChanges( // on its children. We'll use the childLanes on // this fiber to indicate that a context has changed. scheduleWorkOnParentPath(parentSuspense, renderLanes); - nextFiber = fiber.sibling; + nextFiber = null; } else { // Traverse down. nextFiber = fiber.child; @@ -459,14 +481,58 @@ function propagateContextChanges( } } -// Alias for propagating a deferred tree (Suspense, Offscreen). Currently it's -// the same algorithm but there may be a way to optimize one or the other. -export const propagateParentContextChangesToDeferredTree = lazilyPropagateParentContextChanges; - export function lazilyPropagateParentContextChanges( current: Fiber, workInProgress: Fiber, renderLanes: Lanes, +) { + const forcePropagateEntireTree = false; + propagateParentContextChanges( + current, + workInProgress, + renderLanes, + forcePropagateEntireTree, + ); +} + +// Used for propagating a deferred tree (Suspense, Offscreen). We must propagate +// to the entire subtree, because we won't revisit it until after the current +// render has completed, at which point we'll have lost track of which providers +// have changed. +export function propagateParentContextChangesToDeferredTree( + current: Fiber, + workInProgress: Fiber, + renderLanes: Lanes, +) { + const forcePropagateEntireTree = true; + propagateParentContextChanges( + current, + workInProgress, + renderLanes, + forcePropagateEntireTree, + ); +} + +// Used by lazy context propagation algorithm. When we find a context dependency +// match, we don't propagate the changes any further into that fiber's subtree. +// We add the matched fibers to this set. Later, if something inside that +// subtree bails out of rendering, the presence of a parent fiber in this Set +// tells us that we need to continue propagating. +// +// This is a set of _current_ fibers, not work-in-progress fibers. That's why +// it's a set instead of a flag on the fiber. +let deferredPropagation: Set | null = null; + +export function resetDeferredContextPropagation() { + // This is called by prepareFreshStack + deferredPropagation = null; +} + +function propagateParentContextChanges( + current: Fiber, + workInProgress: Fiber, + renderLanes: Lanes, + forcePropagateEntireTree: boolean, ) { if (!enableLazyContextPropagation) { return false; @@ -476,9 +542,42 @@ export function lazilyPropagateParentContextChanges( // number, we use an Array instead of Set. let contexts = null; let parent = workInProgress; - while (parent !== null && (parent.flags & DidPropagateContext) === NoFlags) { + let isInsidePropagationBailout = false; + while (parent !== null) { + const currentParent = parent.alternate; + invariant( + currentParent !== null, + 'Should have a current fiber. This is a bug in React.', + ); + + if (!isInsidePropagationBailout) { + if (deferredPropagation === null) { + if ((parent.flags & DidPropagateContext) !== NoFlags) { + break; + } + } else { + if (currentParent !== null && deferredPropagation.has(currentParent)) { + // We're inside a subtree that previously bailed out of propagation. + // We must disregard the the DidPropagateContext flag as we continue + // searching for parent providers. + isInsidePropagationBailout = true; + // We know that none of the providers in between the propagation + // bailout and the nearest render bailout above that could have + // changed. So we can skip those. + do { + parent = parent.return; + invariant( + parent !== null, + 'Expected to find a bailed out fiber. This is a bug in React.', + ); + } while ((parent.flags & DidPropagateContext) === NoFlags); + } else if ((parent.flags & DidPropagateContext) !== NoFlags) { + break; + } + } + } + if (parent.tag === ContextProvider) { - const currentParent = parent.alternate; if (currentParent !== null) { const oldProps = currentParent.memoizedProps; if (oldProps !== null) { @@ -507,15 +606,33 @@ export function lazilyPropagateParentContextChanges( if (contexts !== null) { // If there were any changed providers, search through the children and // propagate their changes. - propagateContextChanges(workInProgress, contexts, renderLanes); + propagateContextChanges( + workInProgress, + contexts, + renderLanes, + forcePropagateEntireTree, + ); } - // This is an optimization so that we only propagate once per subtree. (We - // will propagate the same providers to different subtrees, though — that's - // why the flag is on the fiber that bailed out, not the provider.) If a + // This is an optimization so that we only propagate once per subtree. If a // deeply nested child bails out, and it calls this propagation function, it // uses this flag to know that the remaining ancestor providers have already // been propagated. + // + // NOTE: This optimization is only necessary because we sometimes enter the + // begin phase of nodes that don't have any work scheduled on them — + // specifically, the siblings of a node that _does_ have scheduled work. The + // siblings will bail out and call this function again, even though we already + // propagated content changes to it and its subtree. So we use this flag to + // mark that the parent providers already propagated. + // + // Unfortunately, though, we need to ignore this flag when we're inside a + // tree whose context propagation was deferred — that's what the + // `deferredPropagation` set is for. + // + // If we could instead bail out before entering the siblings' beging phase, + // then we could remove both `DidPropagateContext` and `deferredPropagation`. + // Consider this as part of the next refactor to the fiber tree structure. workInProgress.flags |= DidPropagateContext; } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index 03cb122aca473..eacab6c4471ed 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -197,7 +197,10 @@ import { invokePassiveEffectUnmountInDEV, } from './ReactFiberCommitWork.new'; import {enqueueUpdate} from './ReactUpdateQueue.new'; -import {resetContextDependencies} from './ReactFiberNewContext.new'; +import { + resetContextDependencies, + resetDeferredContextPropagation, +} from './ReactFiberNewContext.new'; import { resetHooksAfterThrow, ContextOnlyDispatcher, @@ -1370,6 +1373,8 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes) { workInProgressRootUpdatedLanes = NoLanes; workInProgressRootPingedLanes = NoLanes; + resetDeferredContextPropagation(); + enqueueInterleavedUpdates(); if (enableSchedulerTracing) { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index d3eeb26d11cb9..f963cc0ad2ccc 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -197,7 +197,10 @@ import { invokePassiveEffectUnmountInDEV, } from './ReactFiberCommitWork.old'; import {enqueueUpdate} from './ReactUpdateQueue.old'; -import {resetContextDependencies} from './ReactFiberNewContext.old'; +import { + resetContextDependencies, + resetDeferredContextPropagation, +} from './ReactFiberNewContext.old'; import { resetHooksAfterThrow, ContextOnlyDispatcher, @@ -1370,6 +1373,8 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes) { workInProgressRootUpdatedLanes = NoLanes; workInProgressRootPingedLanes = NoLanes; + resetDeferredContextPropagation(); + enqueueInterleavedUpdates(); if (enableSchedulerTracing) { diff --git a/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js b/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js index 557cea6931f42..7d72ea717d767 100644 --- a/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js +++ b/packages/react-reconciler/src/__tests__/ReactContextPropagation-test.js @@ -542,8 +542,8 @@ describe('ReactLazyContextPropagation', () => { expect(root).toMatchRenderedOutput('BB'); }); - // @gate enableCache - test('context is propagated across through offscreen trees', async () => { + // @gate experimental + test('context is propagated through offscreen trees', async () => { const LegacyHidden = React.unstable_LegacyHidden; const root = ReactNoop.createRoot(); @@ -588,7 +588,7 @@ describe('ReactLazyContextPropagation', () => { expect(root).toMatchRenderedOutput('BB'); }); - // @gate enableCache + // @gate experimental test('multiple contexts are propagated across through offscreen trees', async () => { // Same as previous test, but with multiple context providers const LegacyHidden = React.unstable_LegacyHidden; @@ -691,4 +691,232 @@ describe('ReactLazyContextPropagation', () => { expect(Scheduler).toHaveYielded(['B', 'B']); expect(root).toMatchRenderedOutput('BB'); }); + + test('nested bailouts', async () => { + // Lazy context propagation will stop propagating when it hits the first + // match. If we bail out again inside that tree, we must resume propagating. + + const Context = React.createContext('A'); + + let setContext; + function App() { + const [value, setValue] = useState('A'); + setContext = setValue; + return ( + + + + ); + } + + const ChildIndirection = React.memo(() => { + return ; + }); + + function Child() { + const value = useContext(Context); + return ( + <> + + + + ); + } + + const DeepChildIndirection = React.memo(() => { + return ; + }); + + function DeepChild() { + const value = useContext(Context); + return ; + } + + const root = ReactNoop.createRoot(); + await ReactNoop.act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['A', 'A']); + expect(root).toMatchRenderedOutput('AA'); + + await ReactNoop.act(async () => { + setContext('B'); + }); + expect(Scheduler).toHaveYielded(['B', 'B']); + expect(root).toMatchRenderedOutput('BB'); + }); + + // @gate experimental + // @gate enableCache + test('nested bailouts across retries', async () => { + // Lazy context propagation will stop propagating when it hits the first + // match. If we bail out again inside that tree, we must resume propagating. + + const Context = React.createContext('A'); + + let setContext; + function App() { + const [value, setValue] = useState('A'); + setContext = setValue; + return ( + + }> + + + + ); + } + + function Async({value}) { + // When this suspends, we won't be able to visit its children during the + // current render. So we must take extra care to propagate the context + // change in such a way that they're aren't lost when we retry in a + // later render. + readText(value); + return ; + } + + function Child() { + const value = useContext(Context); + return ( + <> + + + + ); + } + + const DeepChildIndirection = React.memo(() => { + return ; + }); + + function DeepChild() { + const value = useContext(Context); + return ; + } + + const root = ReactNoop.createRoot(); + await seedNextTextCache('A'); + await ReactNoop.act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['A', 'A']); + expect(root).toMatchRenderedOutput('AA'); + + await ReactNoop.act(async () => { + setContext('B'); + }); + expect(Scheduler).toHaveYielded(['Suspend! [B]', 'Loading...']); + expect(root).toMatchRenderedOutput('Loading...'); + + await ReactNoop.act(async () => { + await resolveText('B'); + }); + expect(Scheduler).toHaveYielded(['B', 'B']); + expect(root).toMatchRenderedOutput('BB'); + }); + + // @gate experimental + test('nested bailouts through offscreen trees', async () => { + // Lazy context propagation will stop propagating when it hits the first + // match. If we bail out again inside that tree, we must resume propagating. + + const LegacyHidden = React.unstable_LegacyHidden; + + const Context = React.createContext('A'); + + let setContext; + function App() { + const [value, setValue] = useState('A'); + setContext = setValue; + return ( + + + + + + ); + } + + function Child() { + const value = useContext(Context); + return ( + <> + + + + ); + } + + const DeepChildIndirection = React.memo(() => { + return ; + }); + + function DeepChild() { + const value = useContext(Context); + return ; + } + + const root = ReactNoop.createRoot(); + await ReactNoop.act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['A', 'A']); + expect(root).toMatchRenderedOutput('AA'); + + await ReactNoop.act(async () => { + setContext('B'); + }); + expect(Scheduler).toHaveYielded(['B', 'B']); + expect(root).toMatchRenderedOutput('BB'); + }); + + test('finds context consumers in multiple sibling branches', async () => { + // This test confirms that when we find a matching context consumer during + // propagation, we continue propagating to its sibling branches. + + const Context = React.createContext('A'); + + let setContext; + function App() { + const [value, setValue] = useState('A'); + setContext = setValue; + return ( + + + + ); + } + + const Blah = React.memo(() => { + return ( + <> + + + + ); + }); + + const Indirection = React.memo(() => { + return ; + }); + + function Child() { + const value = useContext(Context); + return ; + } + + const root = ReactNoop.createRoot(); + await ReactNoop.act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['A', 'A']); + expect(root).toMatchRenderedOutput('AA'); + + await ReactNoop.act(async () => { + setContext('B'); + }); + expect(Scheduler).toHaveYielded(['B', 'B']); + expect(root).toMatchRenderedOutput('BB'); + }); }); diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index c29a136a1e1ec..fc89fb5ca39ba 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -374,5 +374,7 @@ "383": "This query has received fewer parameters than the last time the same query was used. Always pass the exact number of parameters that the query needs.", "384": "Refreshing the cache is not supported in Server Components.", "385": "A mutable source was mutated while the %s component was rendering. This is not supported. Move any mutations into event handlers or effects.", - "386": "The current renderer does not support microtasks. This error is likely caused by a bug in React. Please file an issue." + "386": "The current renderer does not support microtasks. This error is likely caused by a bug in React. Please file an issue.", + "387": "Should have a current fiber. This is a bug in React.", + "388": "Expected to find a bailed out fiber. This is a bug in React." } From 63941d5b6a82eaa516da7708b7ed4b7f1fe7108f Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Mon, 1 Mar 2021 02:37:31 -0600 Subject: [PATCH 5/5] Mark trees that need propagation in readContext Instead of storing matched context consumers in a Set, we can mark when a consumer receives an update inside `readContext`. I hesistated to put anything in this function because it's such a hot path, but so are bail outs. Fortunately, we only need to set this flag once, the first time a context is read. So I think it's a reasonable trade off. In exchange, propagation is faster because we no longer need to accumulate a Set of matched consumers, and fiber bailouts are faster because we don't need to consult that Set. And the code is simpler. --- .../react-reconciler/src/ReactFiberFlags.js | 47 ++++---- .../src/ReactFiberNewContext.new.js | 112 ++++++------------ .../src/ReactFiberNewContext.old.js | 112 ++++++------------ .../src/ReactFiberWorkLoop.new.js | 7 +- .../src/ReactFiberWorkLoop.old.js | 7 +- 5 files changed, 100 insertions(+), 185 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberFlags.js b/packages/react-reconciler/src/ReactFiberFlags.js index 783e637555111..61abbb6122ed4 100644 --- a/packages/react-reconciler/src/ReactFiberFlags.js +++ b/packages/react-reconciler/src/ReactFiberFlags.js @@ -12,50 +12,51 @@ import {enableCreateEventHandleAPI} from 'shared/ReactFeatureFlags'; export type Flags = number; // Don't change these two values. They're used by React Dev Tools. -export const NoFlags = /* */ 0b000000000000000000000; -export const PerformedWork = /* */ 0b000000000000000000001; +export const NoFlags = /* */ 0b0000000000000000000000; +export const PerformedWork = /* */ 0b0000000000000000000001; // You can change the rest (and add more). -export const Placement = /* */ 0b000000000000000000010; -export const Update = /* */ 0b000000000000000000100; +export const Placement = /* */ 0b0000000000000000000010; +export const Update = /* */ 0b0000000000000000000100; export const PlacementAndUpdate = /* */ Placement | Update; -export const Deletion = /* */ 0b000000000000000001000; -export const ChildDeletion = /* */ 0b000000000000000010000; -export const ContentReset = /* */ 0b000000000000000100000; -export const Callback = /* */ 0b000000000000001000000; -export const DidCapture = /* */ 0b000000000000010000000; -export const Ref = /* */ 0b000000000000100000000; -export const Snapshot = /* */ 0b000000000001000000000; -export const Passive = /* */ 0b000000000010000000000; -export const Hydrating = /* */ 0b000000000100000000000; +export const Deletion = /* */ 0b0000000000000000001000; +export const ChildDeletion = /* */ 0b0000000000000000010000; +export const ContentReset = /* */ 0b0000000000000000100000; +export const Callback = /* */ 0b0000000000000001000000; +export const DidCapture = /* */ 0b0000000000000010000000; +export const Ref = /* */ 0b0000000000000100000000; +export const Snapshot = /* */ 0b0000000000001000000000; +export const Passive = /* */ 0b0000000000010000000000; +export const Hydrating = /* */ 0b0000000000100000000000; export const HydratingAndUpdate = /* */ Hydrating | Update; -export const Visibility = /* */ 0b000000001000000000000; +export const Visibility = /* */ 0b0000000001000000000000; export const LifecycleEffectMask = Passive | Update | Callback | Ref | Snapshot; // Union of all commit flags (flags with the lifetime of a particular commit) -export const HostEffectMask = /* */ 0b000000001111111111111; +export const HostEffectMask = /* */ 0b0000000001111111111111; // These are not really side effects, but we still reuse this field. -export const Incomplete = /* */ 0b000000010000000000000; -export const ShouldCapture = /* */ 0b000000100000000000000; +export const Incomplete = /* */ 0b0000000010000000000000; +export const ShouldCapture = /* */ 0b0000000100000000000000; // TODO (effects) Remove this bit once the new reconciler is synced to the old. -export const PassiveUnmountPendingDev = /* */ 0b000001000000000000000; -export const ForceUpdateForLegacySuspense = /* */ 0b000010000000000000000; -export const DidPropagateContext = /* */ 0b000100000000000000000; +export const PassiveUnmountPendingDev = /* */ 0b0000001000000000000000; +export const ForceUpdateForLegacySuspense = /* */ 0b0000010000000000000000; +export const DidPropagateContext = /* */ 0b0000100000000000000000; +export const NeedsPropagation = /* */ 0b0001000000000000000000; // Static tags describe aspects of a fiber that are not specific to a render, // e.g. a fiber uses a passive effect (even if there are no updates on this particular render). // This enables us to defer more work in the unmount case, // since we can defer traversing the tree during layout to look for Passive effects, // and instead rely on the static flag as a signal that there may be cleanup work. -export const PassiveStatic = /* */ 0b001000000000000000000; +export const PassiveStatic = /* */ 0b0010000000000000000000; // These flags allow us to traverse to fibers that have effects on mount // without traversing the entire tree after every commit for // double invoking -export const MountLayoutDev = /* */ 0b010000000000000000000; -export const MountPassiveDev = /* */ 0b100000000000000000000; +export const MountLayoutDev = /* */ 0b0100000000000000000000; +export const MountPassiveDev = /* */ 0b1000000000000000000000; // Groups of flags that are used in the commit phase to skip over trees that // don't contain effects, by checking subtreeFlags. diff --git a/packages/react-reconciler/src/ReactFiberNewContext.new.js b/packages/react-reconciler/src/ReactFiberNewContext.new.js index 6385d14e2c006..29a1eeef8cdd9 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.new.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.new.js @@ -33,7 +33,11 @@ import { mergeLanes, pickArbitraryLane, } from './ReactFiberLane.new'; -import {NoFlags, DidPropagateContext} from './ReactFiberFlags'; +import { + NoFlags, + DidPropagateContext, + NeedsPropagation, +} from './ReactFiberFlags'; import invariant from 'shared/invariant'; import is from 'shared/objectIs'; @@ -409,14 +413,6 @@ function propagateContextChanges( // visit them during render. We should continue propagating the // siblings, though nextFiber = null; - - // Keep track of subtrees whose propagation we deferred - if (deferredPropagation === null) { - deferredPropagation = new Set([consumer]); - } else { - deferredPropagation.add(consumer); - } - nextFiber = null; } // Since we already found a match, we can stop traversing the @@ -513,21 +509,6 @@ export function propagateParentContextChangesToDeferredTree( ); } -// Used by lazy context propagation algorithm. When we find a context dependency -// match, we don't propagate the changes any further into that fiber's subtree. -// We add the matched fibers to this set. Later, if something inside that -// subtree bails out of rendering, the presence of a parent fiber in this Set -// tells us that we need to continue propagating. -// -// This is a set of _current_ fibers, not work-in-progress fibers. That's why -// it's a set instead of a flag on the fiber. -let deferredPropagation: Set | null = null; - -export function resetDeferredContextPropagation() { - // This is called by prepareFreshStack - deferredPropagation = null; -} - function propagateParentContextChanges( current: Fiber, workInProgress: Fiber, @@ -535,7 +516,7 @@ function propagateParentContextChanges( forcePropagateEntireTree: boolean, ) { if (!enableLazyContextPropagation) { - return false; + return; } // Collect all the parent providers that changed. Since this is usually small @@ -544,58 +525,36 @@ function propagateParentContextChanges( let parent = workInProgress; let isInsidePropagationBailout = false; while (parent !== null) { - const currentParent = parent.alternate; - invariant( - currentParent !== null, - 'Should have a current fiber. This is a bug in React.', - ); - if (!isInsidePropagationBailout) { - if (deferredPropagation === null) { - if ((parent.flags & DidPropagateContext) !== NoFlags) { - break; - } - } else { - if (currentParent !== null && deferredPropagation.has(currentParent)) { - // We're inside a subtree that previously bailed out of propagation. - // We must disregard the the DidPropagateContext flag as we continue - // searching for parent providers. - isInsidePropagationBailout = true; - // We know that none of the providers in between the propagation - // bailout and the nearest render bailout above that could have - // changed. So we can skip those. - do { - parent = parent.return; - invariant( - parent !== null, - 'Expected to find a bailed out fiber. This is a bug in React.', - ); - } while ((parent.flags & DidPropagateContext) === NoFlags); - } else if ((parent.flags & DidPropagateContext) !== NoFlags) { - break; - } + if ((parent.flags & NeedsPropagation) !== NoFlags) { + isInsidePropagationBailout = true; + } else if ((parent.flags & DidPropagateContext) !== NoFlags) { + break; } } if (parent.tag === ContextProvider) { - if (currentParent !== null) { - const oldProps = currentParent.memoizedProps; - if (oldProps !== null) { - const providerType: ReactProviderType = parent.type; - const context: ReactContext = providerType._context; - - const newProps = parent.pendingProps; - const newValue = newProps.value; - - const oldValue = oldProps.value; - - const changedBits = calculateChangedBits(context, newValue, oldValue); - if (changedBits !== 0) { - if (contexts !== null) { - contexts.push(context, changedBits); - } else { - contexts = [context, changedBits]; - } + const currentParent = parent.alternate; + invariant( + currentParent !== null, + 'Should have a current fiber. This is a bug in React.', + ); + const oldProps = currentParent.memoizedProps; + if (oldProps !== null) { + const providerType: ReactProviderType = parent.type; + const context: ReactContext = providerType._context; + + const newProps = parent.pendingProps; + const newValue = newProps.value; + + const oldValue = oldProps.value; + + const changedBits = calculateChangedBits(context, newValue, oldValue); + if (changedBits !== 0) { + if (contexts !== null) { + contexts.push(context, changedBits); + } else { + contexts = [context, changedBits]; } } } @@ -628,10 +587,10 @@ function propagateParentContextChanges( // // Unfortunately, though, we need to ignore this flag when we're inside a // tree whose context propagation was deferred — that's what the - // `deferredPropagation` set is for. + // `NeedsPropagation` flag is for. // - // If we could instead bail out before entering the siblings' beging phase, - // then we could remove both `DidPropagateContext` and `deferredPropagation`. + // If we could instead bail out before entering the siblings' begin phase, + // then we could remove both `DidPropagateContext` and `NeedsPropagation`. // Consider this as part of the next refactor to the fiber tree structure. workInProgress.flags |= DidPropagateContext; } @@ -750,6 +709,9 @@ export function readContext( // TODO: This is an old field. Delete it. responders: null, }; + if (enableLazyContextPropagation) { + currentlyRenderingFiber.flags |= NeedsPropagation; + } } else { // Append a new context item. lastContextDependency = lastContextDependency.next = contextItem; diff --git a/packages/react-reconciler/src/ReactFiberNewContext.old.js b/packages/react-reconciler/src/ReactFiberNewContext.old.js index 5575b4475fbf6..0e6ed587b8ddb 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.old.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.old.js @@ -33,7 +33,11 @@ import { mergeLanes, pickArbitraryLane, } from './ReactFiberLane.old'; -import {NoFlags, DidPropagateContext} from './ReactFiberFlags'; +import { + NoFlags, + DidPropagateContext, + NeedsPropagation, +} from './ReactFiberFlags'; import invariant from 'shared/invariant'; import is from 'shared/objectIs'; @@ -409,14 +413,6 @@ function propagateContextChanges( // visit them during render. We should continue propagating the // siblings, though nextFiber = null; - - // Keep track of subtrees whose propagation we deferred - if (deferredPropagation === null) { - deferredPropagation = new Set([consumer]); - } else { - deferredPropagation.add(consumer); - } - nextFiber = null; } // Since we already found a match, we can stop traversing the @@ -513,21 +509,6 @@ export function propagateParentContextChangesToDeferredTree( ); } -// Used by lazy context propagation algorithm. When we find a context dependency -// match, we don't propagate the changes any further into that fiber's subtree. -// We add the matched fibers to this set. Later, if something inside that -// subtree bails out of rendering, the presence of a parent fiber in this Set -// tells us that we need to continue propagating. -// -// This is a set of _current_ fibers, not work-in-progress fibers. That's why -// it's a set instead of a flag on the fiber. -let deferredPropagation: Set | null = null; - -export function resetDeferredContextPropagation() { - // This is called by prepareFreshStack - deferredPropagation = null; -} - function propagateParentContextChanges( current: Fiber, workInProgress: Fiber, @@ -535,7 +516,7 @@ function propagateParentContextChanges( forcePropagateEntireTree: boolean, ) { if (!enableLazyContextPropagation) { - return false; + return; } // Collect all the parent providers that changed. Since this is usually small @@ -544,58 +525,36 @@ function propagateParentContextChanges( let parent = workInProgress; let isInsidePropagationBailout = false; while (parent !== null) { - const currentParent = parent.alternate; - invariant( - currentParent !== null, - 'Should have a current fiber. This is a bug in React.', - ); - if (!isInsidePropagationBailout) { - if (deferredPropagation === null) { - if ((parent.flags & DidPropagateContext) !== NoFlags) { - break; - } - } else { - if (currentParent !== null && deferredPropagation.has(currentParent)) { - // We're inside a subtree that previously bailed out of propagation. - // We must disregard the the DidPropagateContext flag as we continue - // searching for parent providers. - isInsidePropagationBailout = true; - // We know that none of the providers in between the propagation - // bailout and the nearest render bailout above that could have - // changed. So we can skip those. - do { - parent = parent.return; - invariant( - parent !== null, - 'Expected to find a bailed out fiber. This is a bug in React.', - ); - } while ((parent.flags & DidPropagateContext) === NoFlags); - } else if ((parent.flags & DidPropagateContext) !== NoFlags) { - break; - } + if ((parent.flags & NeedsPropagation) !== NoFlags) { + isInsidePropagationBailout = true; + } else if ((parent.flags & DidPropagateContext) !== NoFlags) { + break; } } if (parent.tag === ContextProvider) { - if (currentParent !== null) { - const oldProps = currentParent.memoizedProps; - if (oldProps !== null) { - const providerType: ReactProviderType = parent.type; - const context: ReactContext = providerType._context; - - const newProps = parent.pendingProps; - const newValue = newProps.value; - - const oldValue = oldProps.value; - - const changedBits = calculateChangedBits(context, newValue, oldValue); - if (changedBits !== 0) { - if (contexts !== null) { - contexts.push(context, changedBits); - } else { - contexts = [context, changedBits]; - } + const currentParent = parent.alternate; + invariant( + currentParent !== null, + 'Should have a current fiber. This is a bug in React.', + ); + const oldProps = currentParent.memoizedProps; + if (oldProps !== null) { + const providerType: ReactProviderType = parent.type; + const context: ReactContext = providerType._context; + + const newProps = parent.pendingProps; + const newValue = newProps.value; + + const oldValue = oldProps.value; + + const changedBits = calculateChangedBits(context, newValue, oldValue); + if (changedBits !== 0) { + if (contexts !== null) { + contexts.push(context, changedBits); + } else { + contexts = [context, changedBits]; } } } @@ -628,10 +587,10 @@ function propagateParentContextChanges( // // Unfortunately, though, we need to ignore this flag when we're inside a // tree whose context propagation was deferred — that's what the - // `deferredPropagation` set is for. + // `NeedsPropagation` flag is for. // - // If we could instead bail out before entering the siblings' beging phase, - // then we could remove both `DidPropagateContext` and `deferredPropagation`. + // If we could instead bail out before entering the siblings' begin phase, + // then we could remove both `DidPropagateContext` and `NeedsPropagation`. // Consider this as part of the next refactor to the fiber tree structure. workInProgress.flags |= DidPropagateContext; } @@ -750,6 +709,9 @@ export function readContext( // TODO: This is an old field. Delete it. responders: null, }; + if (enableLazyContextPropagation) { + currentlyRenderingFiber.flags |= NeedsPropagation; + } } else { // Append a new context item. lastContextDependency = lastContextDependency.next = contextItem; diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js index eacab6c4471ed..03cb122aca473 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.new.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.new.js @@ -197,10 +197,7 @@ import { invokePassiveEffectUnmountInDEV, } from './ReactFiberCommitWork.new'; import {enqueueUpdate} from './ReactUpdateQueue.new'; -import { - resetContextDependencies, - resetDeferredContextPropagation, -} from './ReactFiberNewContext.new'; +import {resetContextDependencies} from './ReactFiberNewContext.new'; import { resetHooksAfterThrow, ContextOnlyDispatcher, @@ -1373,8 +1370,6 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes) { workInProgressRootUpdatedLanes = NoLanes; workInProgressRootPingedLanes = NoLanes; - resetDeferredContextPropagation(); - enqueueInterleavedUpdates(); if (enableSchedulerTracing) { diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js index f963cc0ad2ccc..d3eeb26d11cb9 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.old.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.old.js @@ -197,10 +197,7 @@ import { invokePassiveEffectUnmountInDEV, } from './ReactFiberCommitWork.old'; import {enqueueUpdate} from './ReactUpdateQueue.old'; -import { - resetContextDependencies, - resetDeferredContextPropagation, -} from './ReactFiberNewContext.old'; +import {resetContextDependencies} from './ReactFiberNewContext.old'; import { resetHooksAfterThrow, ContextOnlyDispatcher, @@ -1373,8 +1370,6 @@ function prepareFreshStack(root: FiberRoot, lanes: Lanes) { workInProgressRootUpdatedLanes = NoLanes; workInProgressRootPingedLanes = NoLanes; - resetDeferredContextPropagation(); - enqueueInterleavedUpdates(); if (enableSchedulerTracing) {