Skip to content

Commit 3aaab92

Browse files
authored
[flags] add enableEffectEventMutationPhase (#35548)
Small optimization for useEffectEvent. Not sure we even need a flag for it, but it will be a nice killswitch. As an added benefit, it fixes a bug when `enableViewTransition` is on, where we were not updating the useEffectEvent callback when a tree went from hidden to visible.
1 parent 087a346 commit 3aaab92

12 files changed

+458
-6
lines changed

packages/react-reconciler/src/ReactFiberCommitWork.js

Lines changed: 20 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import type {ViewTransitionState} from './ReactFiberViewTransitionComponent';
4747
import {
4848
alwaysThrottleRetries,
4949
enableCreateEventHandleAPI,
50+
enableEffectEventMutationPhase,
5051
enableHiddenSubtreeInsertionEffectCleanup,
5152
enableProfilerTimer,
5253
enableProfilerCommitHooks,
@@ -499,7 +500,7 @@ function commitBeforeMutationEffectsOnFiber(
499500
case FunctionComponent:
500501
case ForwardRef:
501502
case SimpleMemoComponent: {
502-
if ((flags & Update) !== NoFlags) {
503+
if (!enableEffectEventMutationPhase && (flags & Update) !== NoFlags) {
503504
const updateQueue: FunctionComponentUpdateQueue | null =
504505
(finishedWork.updateQueue: any);
505506
const eventPayloads = updateQueue !== null ? updateQueue.events : null;
@@ -2042,6 +2043,24 @@ function commitMutationEffectsOnFiber(
20422043
case ForwardRef:
20432044
case MemoComponent:
20442045
case SimpleMemoComponent: {
2046+
// Mutate event effect callbacks on the way down, before mutation effects.
2047+
// This ensures that parent event effects are mutated before child effects.
2048+
// This isn't a supported use case, so we can re-consider it,
2049+
// but this was the behavior we originally shipped.
2050+
if (enableEffectEventMutationPhase) {
2051+
if (flags & Update) {
2052+
const updateQueue: FunctionComponentUpdateQueue | null =
2053+
(finishedWork.updateQueue: any);
2054+
const eventPayloads =
2055+
updateQueue !== null ? updateQueue.events : null;
2056+
if (eventPayloads !== null) {
2057+
for (let ii = 0; ii < eventPayloads.length; ii++) {
2058+
const {ref, nextImpl} = eventPayloads[ii];
2059+
ref.impl = nextImpl;
2060+
}
2061+
}
2062+
}
2063+
}
20452064
recursivelyTraverseMutationEffects(root, finishedWork, lanes);
20462065
commitReconciliationEffects(finishedWork, lanes);
20472066

packages/react-reconciler/src/ReactFiberFlags.js

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,10 @@
77
* @flow
88
*/
99

10-
import {enableCreateEventHandleAPI} from 'shared/ReactFeatureFlags';
10+
import {
11+
enableCreateEventHandleAPI,
12+
enableEffectEventMutationPhase,
13+
} from 'shared/ReactFeatureFlags';
1114

1215
export type Flags = number;
1316

@@ -99,10 +102,11 @@ export const BeforeMutationMask: number =
99102
// TODO: Only need to visit Deletions during BeforeMutation phase if an
100103
// element is focused.
101104
Update | ChildDeletion | Visibility
102-
: // TODO: The useEffectEvent hook uses the snapshot phase for clean up but it
103-
// really should use the mutation phase for this or at least schedule an
104-
// explicit Snapshot phase flag for this.
105-
Update);
105+
: // useEffectEvent uses the snapshot phase,
106+
// but we're moving it to the mutation phase.
107+
enableEffectEventMutationPhase
108+
? 0
109+
: Update);
106110

107111
// For View Transition support we use the snapshot phase to scan the tree for potentially
108112
// affected ViewTransition components.

0 commit comments

Comments
 (0)