Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Clean up enableLazyContextPropagation #31810

Merged
merged 1 commit into from
Dec 17, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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
Loading