Skip to content

Commit afd1041

Browse files
acdliteacdlite
committed
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.
1 parent 0cf9fc1 commit afd1041

22 files changed

+507
-83
lines changed

‎packages/react-dom/src/__tests__/ReactLegacyContextDisabled-test.internal.js

+9-1
Original file line numberDiff line numberDiff line change
@@ -226,7 +226,15 @@ describe('ReactLegacyContextDisabled', () => {
226226
container,
227227
);
228228
expect(container.textContent).toBe('bbb');
229-
expect(lifecycleContextLog).toEqual(['b', 'b']); // sCU skipped due to changed context value.
229+
if (gate(flags => flags.enableLazyContextPropagation)) {
230+
// In the lazy propagation implementation, we don't check if context
231+
// changed until after shouldComponentUpdate is run.
232+
expect(lifecycleContextLog).toEqual(['b', 'b', 'b']);
233+
} else {
234+
// In the eager implementation, a dirty flag was set when the parent
235+
// changed, so we skipped sCU.
236+
expect(lifecycleContextLog).toEqual(['b', 'b']);
237+
}
230238
ReactDOM.unmountComponentAtNode(container);
231239
});
232240
});

‎packages/react-reconciler/src/ReactFiberBeginWork.new.js

+4
Original file line numberDiff line numberDiff line change
@@ -3073,6 +3073,10 @@ export function markWorkInProgressReceivedUpdate() {
30733073
didReceiveUpdate = true;
30743074
}
30753075

3076+
export function checkIfWorkInProgressReceivedUpdate() {
3077+
return didReceiveUpdate;
3078+
}
3079+
30763080
function bailoutOnAlreadyFinishedWork(
30773081
current: Fiber | null,
30783082
workInProgress: Fiber,

‎packages/react-reconciler/src/ReactFiberBeginWork.old.js

+4
Original file line numberDiff line numberDiff line change
@@ -3073,6 +3073,10 @@ export function markWorkInProgressReceivedUpdate() {
30733073
didReceiveUpdate = true;
30743074
}
30753075

3076+
export function checkIfWorkInProgressReceivedUpdate() {
3077+
return didReceiveUpdate;
3078+
}
3079+
30763080
function bailoutOnAlreadyFinishedWork(
30773081
current: Fiber | null,
30783082
workInProgress: Fiber,

‎packages/react-reconciler/src/ReactFiberClassComponent.new.js

+18-3
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
enableSchedulingProfiler,
2121
warnAboutDeprecatedLifecycles,
2222
enableStrictEffects,
23+
enableLazyContextPropagation,
2324
} from 'shared/ReactFeatureFlags';
2425
import ReactStrictModeWarnings from './ReactStrictModeWarnings.new';
2526
import {isMounted} from './ReactFiberTreeReflection';
@@ -57,7 +58,7 @@ import {
5758
hasContextChanged,
5859
emptyContextObject,
5960
} from './ReactFiberContext.new';
60-
import {readContext} from './ReactFiberNewContext.new';
61+
import {readContext, checkIfContextChanged} from './ReactFiberNewContext.new';
6162
import {
6263
requestEventTime,
6364
requestUpdateLane,
@@ -1149,7 +1150,13 @@ function updateClassInstance(
11491150
unresolvedOldProps === unresolvedNewProps &&
11501151
oldState === newState &&
11511152
!hasContextChanged() &&
1152-
!checkHasForceUpdateAfterProcessing()
1153+
!checkHasForceUpdateAfterProcessing() &&
1154+
!(
1155+
enableLazyContextPropagation &&
1156+
current !== null &&
1157+
current.dependencies !== null &&
1158+
checkIfContextChanged(current.dependencies)
1159+
)
11531160
) {
11541161
// If an update was already in progress, we should schedule an Update
11551162
// effect even though we're bailing out, so that cWU/cDU are called.
@@ -1192,7 +1199,15 @@ function updateClassInstance(
11921199
oldState,
11931200
newState,
11941201
nextContext,
1195-
);
1202+
) ||
1203+
// TODO: In some cases, we'll end up checking if context has changed twice,
1204+
// both before and after `shouldComponentUpdate` has been called. Not ideal,
1205+
// but I'm loath to refactor this function. This only happens for memoized
1206+
// components so it's not that common.
1207+
(enableLazyContextPropagation &&
1208+
current !== null &&
1209+
current.dependencies !== null &&
1210+
checkIfContextChanged(current.dependencies));
11961211

11971212
if (shouldUpdate) {
11981213
// In order to support react-lifecycles-compat polyfilled components,

‎packages/react-reconciler/src/ReactFiberClassComponent.old.js

+18-3
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ import {
2020
enableSchedulingProfiler,
2121
warnAboutDeprecatedLifecycles,
2222
enableStrictEffects,
23+
enableLazyContextPropagation,
2324
} from 'shared/ReactFeatureFlags';
2425
import ReactStrictModeWarnings from './ReactStrictModeWarnings.old';
2526
import {isMounted} from './ReactFiberTreeReflection';
@@ -57,7 +58,7 @@ import {
5758
hasContextChanged,
5859
emptyContextObject,
5960
} from './ReactFiberContext.old';
60-
import {readContext} from './ReactFiberNewContext.old';
61+
import {readContext, checkIfContextChanged} from './ReactFiberNewContext.old';
6162
import {
6263
requestEventTime,
6364
requestUpdateLane,
@@ -1149,7 +1150,13 @@ function updateClassInstance(
11491150
unresolvedOldProps === unresolvedNewProps &&
11501151
oldState === newState &&
11511152
!hasContextChanged() &&
1152-
!checkHasForceUpdateAfterProcessing()
1153+
!checkHasForceUpdateAfterProcessing() &&
1154+
!(
1155+
enableLazyContextPropagation &&
1156+
current !== null &&
1157+
current.dependencies !== null &&
1158+
checkIfContextChanged(current.dependencies)
1159+
)
11531160
) {
11541161
// If an update was already in progress, we should schedule an Update
11551162
// effect even though we're bailing out, so that cWU/cDU are called.
@@ -1192,7 +1199,15 @@ function updateClassInstance(
11921199
oldState,
11931200
newState,
11941201
nextContext,
1195-
);
1202+
) ||
1203+
// TODO: In some cases, we'll end up checking if context has changed twice,
1204+
// both before and after `shouldComponentUpdate` has been called. Not ideal,
1205+
// but I'm loath to refactor this function. This only happens for memoized
1206+
// components so it's not that common.
1207+
(enableLazyContextPropagation &&
1208+
current !== null &&
1209+
current.dependencies !== null &&
1210+
checkIfContextChanged(current.dependencies));
11961211

11971212
if (shouldUpdate) {
11981213
// In order to support react-lifecycles-compat polyfilled components,

‎packages/react-reconciler/src/ReactFiberHooks.new.js

+27-2
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
decoupleUpdatePriorityFromScheduler,
3131
enableUseRefAccessWarning,
3232
enableStrictEffects,
33+
enableLazyContextPropagation,
3334
} from 'shared/ReactFeatureFlags';
3435

3536
import {
@@ -54,7 +55,7 @@ import {
5455
higherLanePriority,
5556
DefaultLanePriority,
5657
} from './ReactFiberLane.new';
57-
import {readContext} from './ReactFiberNewContext.new';
58+
import {readContext, checkIfContextChanged} from './ReactFiberNewContext.new';
5859
import {HostRoot, CacheComponent} from './ReactWorkTags';
5960
import {
6061
Update as UpdateEffect,
@@ -83,7 +84,10 @@ import {
8384
import invariant from 'shared/invariant';
8485
import getComponentName from 'shared/getComponentName';
8586
import is from 'shared/objectIs';
86-
import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork.new';
87+
import {
88+
markWorkInProgressReceivedUpdate,
89+
checkIfWorkInProgressReceivedUpdate,
90+
} from './ReactFiberBeginWork.new';
8791
import {
8892
UserBlockingPriority,
8993
NormalPriority,
@@ -496,6 +500,27 @@ export function renderWithHooks<Props, SecondArg>(
496500
'early return statement.',
497501
);
498502

503+
if (enableLazyContextPropagation) {
504+
if (current !== null) {
505+
if (!checkIfWorkInProgressReceivedUpdate()) {
506+
// If there were no changes to props or state, we need to check if there
507+
// was a context change. We didn't already do this because there's no
508+
// 1:1 correspondence between dependencies and hooks. Although, because
509+
// there almost always is in the common case (`readContext` is an
510+
// internal API), we could compare in there. OTOH, we only hit this case
511+
// if everything else bails out, so on the whole it might be better to
512+
// keep the comparison out of the common path.
513+
const currentDependencies = current.dependencies;
514+
if (
515+
currentDependencies !== null &&
516+
checkIfContextChanged(currentDependencies)
517+
) {
518+
markWorkInProgressReceivedUpdate();
519+
}
520+
}
521+
}
522+
}
523+
499524
return children;
500525
}
501526

‎packages/react-reconciler/src/ReactFiberHooks.old.js

+27-2
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ import {
3030
decoupleUpdatePriorityFromScheduler,
3131
enableUseRefAccessWarning,
3232
enableStrictEffects,
33+
enableLazyContextPropagation,
3334
} from 'shared/ReactFeatureFlags';
3435

3536
import {
@@ -54,7 +55,7 @@ import {
5455
higherLanePriority,
5556
DefaultLanePriority,
5657
} from './ReactFiberLane.old';
57-
import {readContext} from './ReactFiberNewContext.old';
58+
import {readContext, checkIfContextChanged} from './ReactFiberNewContext.old';
5859
import {HostRoot, CacheComponent} from './ReactWorkTags';
5960
import {
6061
Update as UpdateEffect,
@@ -83,7 +84,10 @@ import {
8384
import invariant from 'shared/invariant';
8485
import getComponentName from 'shared/getComponentName';
8586
import is from 'shared/objectIs';
86-
import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork.old';
87+
import {
88+
markWorkInProgressReceivedUpdate,
89+
checkIfWorkInProgressReceivedUpdate,
90+
} from './ReactFiberBeginWork.old';
8791
import {
8892
UserBlockingPriority,
8993
NormalPriority,
@@ -496,6 +500,27 @@ export function renderWithHooks<Props, SecondArg>(
496500
'early return statement.',
497501
);
498502

503+
if (enableLazyContextPropagation) {
504+
if (current !== null) {
505+
if (!checkIfWorkInProgressReceivedUpdate()) {
506+
// If there were no changes to props or state, we need to check if there
507+
// was a context change. We didn't already do this because there's no
508+
// 1:1 correspondence between dependencies and hooks. Although, because
509+
// there almost always is in the common case (`readContext` is an
510+
// internal API), we could compare in there. OTOH, we only hit this case
511+
// if everything else bails out, so on the whole it might be better to
512+
// keep the comparison out of the common path.
513+
const currentDependencies = current.dependencies;
514+
if (
515+
currentDependencies !== null &&
516+
checkIfContextChanged(currentDependencies)
517+
) {
518+
markWorkInProgressReceivedUpdate();
519+
}
520+
}
521+
}
522+
}
523+
499524
return children;
500525
}
501526

0 commit comments

Comments
 (0)