Skip to content

Commit

Permalink
Clean up enableLazyContextPropagation (facebook#31810)
Browse files Browse the repository at this point in the history
This flag has shipped everywhere, let's clean it up.
  • Loading branch information
jackpope authored Dec 17, 2024
1 parent d428725 commit 34ee391
Show file tree
Hide file tree
Showing 15 changed files with 53 additions and 331 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -233,15 +233,7 @@ describe('ReactLegacyContextDisabled', () => {
);
});
expect(container.textContent).toBe('bbb');
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']);
}
expect(lifecycleContextLog).toEqual(['b', 'b', 'b']);
root.unmount();
});
});
71 changes: 6 additions & 65 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,6 @@ import type {
import type {UpdateQueue} from './ReactFiberClassUpdateQueue';
import type {RootState} from './ReactFiberRoot';
import type {TracingMarkerInstance} from './ReactFiberTracingMarkerComponent';
import type {TransitionStatus} from './ReactFiberConfig';
import type {Hook} from './ReactFiberHooks';

import {
markComponentRenderStarted,
Expand Down Expand Up @@ -100,7 +98,6 @@ import {
enableProfilerCommitHooks,
enableProfilerTimer,
enableScopeAPI,
enableLazyContextPropagation,
enableSchedulingProfiler,
enableTransitionTracing,
enableLegacyHidden,
Expand Down Expand Up @@ -272,7 +269,6 @@ import {
createClassErrorUpdate,
initializeClassErrorUpdate,
} from './ReactFiberThrow';
import is from 'shared/objectIs';
import {
getForksAtLevel,
isForkedChild,
Expand Down Expand Up @@ -843,7 +839,7 @@ function deferHiddenOffscreenComponent(

pushOffscreenSuspenseHandler(workInProgress);

if (enableLazyContextPropagation && current !== null) {
if (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.
Expand Down Expand Up @@ -1636,26 +1632,6 @@ function updateHostComponent(
} else {
HostTransitionContext._currentValue2 = newState;
}
if (enableLazyContextPropagation) {
// In the lazy propagation implementation, we don't scan for matching
// consumers until something bails out.
} else {
if (didReceiveUpdate) {
if (current !== null) {
const oldStateHook: Hook = current.memoizedState;
const oldState: TransitionStatus = oldStateHook.memoizedState;
// This uses regular equality instead of Object.is because we assume
// that host transition state doesn't include NaN as a valid type.
if (oldState !== newState) {
propagateContextChange(
workInProgress,
HostTransitionContext,
renderLanes,
);
}
}
}
}
}

markRef(current, workInProgress);
Expand Down Expand Up @@ -2706,10 +2682,7 @@ function updateDehydratedSuspenseComponent(
}

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
Expand Down Expand Up @@ -3300,8 +3273,6 @@ function updateContextProvider(
context = workInProgress.type._context;
}
const newProps = workInProgress.pendingProps;
const oldProps = workInProgress.memoizedProps;

const newValue = newProps.value;

if (__DEV__) {
Expand All @@ -3317,34 +3288,6 @@ function updateContextProvider(

pushProvider(workInProgress, context, newValue);

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;
if (is(oldValue, newValue)) {
// 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, renderLanes);
}
}
}

const newChildren = newProps.children;
reconcileChildren(current, workInProgress, newChildren, renderLanes);
return workInProgress.child;
Expand Down Expand Up @@ -3463,7 +3406,7 @@ function bailoutOnAlreadyFinishedWork(
// 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.

if (enableLazyContextPropagation && current !== null) {
if (current !== null) {
// Before bailing out, check if there are any context changes in
// the children.
lazilyPropagateParentContextChanges(current, workInProgress, renderLanes);
Expand Down Expand Up @@ -3564,11 +3507,9 @@ function checkScheduledUpdateOrContext(
}
// No pending update, but because context is propagated lazily, we need
// to check for a context change before we bail out.
if (enableLazyContextPropagation) {
const dependencies = current.dependencies;
if (dependencies !== null && checkIfContextChanged(dependencies)) {
return true;
}
const dependencies = current.dependencies;
if (dependencies !== null && checkIfContextChanged(dependencies)) {
return true;
}
return false;
}
Expand Down Expand Up @@ -3706,7 +3647,7 @@ function attemptEarlyBailoutIfNoScheduledUpdate(
workInProgress.childLanes,
);

if (enableLazyContextPropagation && !hasChildWork) {
if (!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
Expand Down
5 changes: 1 addition & 4 deletions packages/react-reconciler/src/ReactFiberClassComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ import {
debugRenderPhaseSideEffectsForStrictMode,
disableLegacyContext,
enableSchedulingProfiler,
enableLazyContextPropagation,
disableDefaultPropsExceptForClasses,
} from 'shared/ReactFeatureFlags';
import ReactStrictModeWarnings from './ReactStrictModeWarnings';
Expand Down Expand Up @@ -1092,7 +1091,6 @@ function updateClassInstance(
!hasContextChanged() &&
!checkHasForceUpdateAfterProcessing() &&
!(
enableLazyContextPropagation &&
current !== null &&
current.dependencies !== null &&
checkIfContextChanged(current.dependencies)
Expand Down Expand Up @@ -1144,8 +1142,7 @@ function updateClassInstance(
// 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 !== null &&
current.dependencies !== null &&
checkIfContextChanged(current.dependencies));

Expand Down
33 changes: 15 additions & 18 deletions packages/react-reconciler/src/ReactFiberHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@ import {
import ReactSharedInternals from 'shared/ReactSharedInternals';
import {
enableSchedulingProfiler,
enableLazyContextPropagation,
enableTransitionTracing,
enableUseEffectEventHook,
enableUseResourceEffectHook,
Expand Down Expand Up @@ -743,23 +742,21 @@ function finishRenderingHooks<Props, SecondArg>(
);
}

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();
}
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();
}
}
}
Expand Down
Loading

0 comments on commit 34ee391

Please sign in to comment.