Skip to content

Commit be3fb29

Browse files
rickhanloniirickhanlonii
authored
[internal] revert change merged accidentally (facebook#35546)
I accidentally pushed this to new flag to facebook#35541 and then merged it. Reverting it so I can submit a review.
1 parent 23e5edd commit be3fb29

12 files changed

+6
-245
lines changed

‎packages/react-reconciler/src/ReactFiberCommitWork.js‎

Lines changed: 1 addition & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ import type {ViewTransitionState} from './ReactFiberViewTransitionComponent';
4747
import {
4848
alwaysThrottleRetries,
4949
enableCreateEventHandleAPI,
50-
enableEffectEventMutationPhase,
5150
enableHiddenSubtreeInsertionEffectCleanup,
5251
enableProfilerTimer,
5352
enableProfilerCommitHooks,
@@ -500,7 +499,7 @@ function commitBeforeMutationEffectsOnFiber(
500499
case FunctionComponent:
501500
case ForwardRef:
502501
case SimpleMemoComponent: {
503-
if (!enableEffectEventMutationPhase && (flags & Update) !== NoFlags) {
502+
if ((flags & Update) !== NoFlags) {
504503
const updateQueue: FunctionComponentUpdateQueue | null =
505504
(finishedWork.updateQueue: any);
506505
const eventPayloads = updateQueue !== null ? updateQueue.events : null;
@@ -2047,19 +2046,6 @@ function commitMutationEffectsOnFiber(
20472046
commitReconciliationEffects(finishedWork, lanes);
20482047

20492048
if (flags & Update) {
2050-
// Mutate event effect callbacks before insertion effects.
2051-
if (enableEffectEventMutationPhase) {
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-
}
20632049
commitHookEffectListUnmount(
20642050
HookInsertion | HookHasEffect,
20652051
finishedWork,

‎packages/react-reconciler/src/ReactFiberFlags.js‎

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

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

1512
export type Flags = number;
1613

@@ -102,11 +99,10 @@ export const BeforeMutationMask: number =
10299
// TODO: Only need to visit Deletions during BeforeMutation phase if an
103100
// element is focused.
104101
Update | ChildDeletion | Visibility
105-
: // useEffectEvent uses the snapshot phase,
106-
// but we're moving it to the mutation phase.
107-
enableEffectEventMutationPhase
108-
? 0
109-
: Update);
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);
110106

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

‎packages/react-reconciler/src/__tests__/useEffectEvent-test.js‎

Lines changed: 0 additions & 208 deletions
Original file line numberDiff line numberDiff line change
@@ -595,214 +595,6 @@ describe('useEffectEvent', () => {
595595
assertLog(['Effect value: 2', 'Event value: 2']);
596596
});
597597

598-
it('updates parent and child event effects before their respective effect lifecycles', async () => {
599-
function Parent({value}) {
600-
const parentEvent = useEffectEvent(() => {
601-
Scheduler.log('Parent event: ' + value);
602-
});
603-
604-
useInsertionEffect(() => {
605-
Scheduler.log('Parent insertion');
606-
parentEvent();
607-
}, [value]);
608-
609-
return <Child value={value} />;
610-
}
611-
612-
function Child({value}) {
613-
const childEvent = useEffectEvent(() => {
614-
Scheduler.log('Child event: ' + value);
615-
});
616-
617-
useInsertionEffect(() => {
618-
Scheduler.log('Child insertion');
619-
childEvent();
620-
}, [value]);
621-
622-
return null;
623-
}
624-
625-
ReactNoop.render(<Parent value={1} />);
626-
await waitForAll([
627-
'Child insertion',
628-
'Child event: 1',
629-
'Parent insertion',
630-
'Parent event: 1',
631-
]);
632-
633-
await act(() => ReactNoop.render(<Parent value={2} />));
634-
// Each component's event is updated before its own insertion effect runs
635-
assertLog([
636-
'Child insertion',
637-
'Child event: 2',
638-
'Parent insertion',
639-
'Parent event: 2',
640-
]);
641-
});
642-
643-
it('fires all insertion effects (interleaved) with useEffectEvent before firing any layout effects', async () => {
644-
// This test mirrors the 'fires all insertion effects (interleaved) before firing any layout effects'
645-
// test in ReactHooksWithNoopRenderer-test.js, but adds useEffectEvent to verify that
646-
// event payloads are updated before each component's insertion effects run.
647-
// It also includes passive effects to verify the full effect lifecycle.
648-
let committedA = '(empty)';
649-
let committedB = '(empty)';
650-
651-
function CounterA(props) {
652-
const onEvent = useEffectEvent(() => {
653-
return `Event A [A: ${committedA}, B: ${committedB}]`;
654-
});
655-
656-
useInsertionEffect(() => {
657-
// Call the event function to verify it sees the latest value
658-
Scheduler.log(
659-
`Create Insertion A [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`,
660-
);
661-
committedA = String(props.count);
662-
return () => {
663-
Scheduler.log(
664-
`Destroy Insertion A [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`,
665-
);
666-
};
667-
});
668-
669-
useLayoutEffect(() => {
670-
Scheduler.log(
671-
`Create Layout A [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`,
672-
);
673-
return () => {
674-
Scheduler.log(
675-
`Destroy Layout A [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`,
676-
);
677-
};
678-
});
679-
680-
useEffect(() => {
681-
Scheduler.log(
682-
`Create Passive A [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`,
683-
);
684-
return () => {
685-
Scheduler.log(
686-
`Destroy Passive A [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`,
687-
);
688-
};
689-
});
690-
691-
return null;
692-
}
693-
694-
function CounterB(props) {
695-
const onEvent = useEffectEvent(() => {
696-
return `Event B [A: ${committedA}, B: ${committedB}]`;
697-
});
698-
699-
useInsertionEffect(() => {
700-
// Call the event function to verify it sees the latest value
701-
Scheduler.log(
702-
`Create Insertion B [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`,
703-
);
704-
committedB = String(props.count);
705-
return () => {
706-
Scheduler.log(
707-
`Destroy Insertion B [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`,
708-
);
709-
};
710-
});
711-
712-
useLayoutEffect(() => {
713-
Scheduler.log(
714-
`Create Layout B [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`,
715-
);
716-
return () => {
717-
Scheduler.log(
718-
`Destroy Layout B [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`,
719-
);
720-
};
721-
});
722-
723-
useEffect(() => {
724-
Scheduler.log(
725-
`Create Passive B [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`,
726-
);
727-
return () => {
728-
Scheduler.log(
729-
`Destroy Passive B [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`,
730-
);
731-
};
732-
});
733-
734-
return null;
735-
}
736-
737-
await act(async () => {
738-
ReactNoop.render(
739-
<React.Fragment>
740-
<CounterA count={0} />
741-
<CounterB count={0} />
742-
</React.Fragment>,
743-
);
744-
// All insertion effects fire before all layout effects, then passive effects
745-
// Event functions should see the state AT THE TIME they're called
746-
await waitForAll([
747-
// Insertion effects (mutation phase)
748-
'Create Insertion A [A: (empty), B: (empty)], event: Event A [A: (empty), B: (empty)]',
749-
'Create Insertion B [A: 0, B: (empty)], event: Event B [A: 0, B: (empty)]',
750-
// Layout effects
751-
'Create Layout A [A: 0, B: 0], event: Event A [A: 0, B: 0]',
752-
'Create Layout B [A: 0, B: 0], event: Event B [A: 0, B: 0]',
753-
// Passive effects
754-
'Create Passive A [A: 0, B: 0], event: Event A [A: 0, B: 0]',
755-
'Create Passive B [A: 0, B: 0], event: Event B [A: 0, B: 0]',
756-
]);
757-
expect([committedA, committedB]).toEqual(['0', '0']);
758-
});
759-
760-
await act(async () => {
761-
ReactNoop.render(
762-
<React.Fragment>
763-
<CounterA count={1} />
764-
<CounterB count={1} />
765-
</React.Fragment>,
766-
);
767-
await waitForAll([
768-
// Component A: insertion destroy, then create
769-
'Destroy Insertion A [A: 0, B: 0], event: Event A [A: 0, B: 0]',
770-
'Create Insertion A [A: 0, B: 0], event: Event A [A: 0, B: 0]',
771-
// Component A: layout destroy (after insertion updated committedA)
772-
'Destroy Layout A [A: 1, B: 0], event: Event A [A: 1, B: 0]',
773-
// Component B: insertion destroy, then create
774-
'Destroy Insertion B [A: 1, B: 0], event: Event B [A: 1, B: 0]',
775-
'Create Insertion B [A: 1, B: 0], event: Event B [A: 1, B: 0]',
776-
// Component B: layout destroy (after insertion updated committedB)
777-
'Destroy Layout B [A: 1, B: 1], event: Event B [A: 1, B: 1]',
778-
// Layout creates
779-
'Create Layout A [A: 1, B: 1], event: Event A [A: 1, B: 1]',
780-
'Create Layout B [A: 1, B: 1], event: Event B [A: 1, B: 1]',
781-
// Passive destroys then creates
782-
'Destroy Passive A [A: 1, B: 1], event: Event A [A: 1, B: 1]',
783-
'Destroy Passive B [A: 1, B: 1], event: Event B [A: 1, B: 1]',
784-
'Create Passive A [A: 1, B: 1], event: Event A [A: 1, B: 1]',
785-
'Create Passive B [A: 1, B: 1], event: Event B [A: 1, B: 1]',
786-
]);
787-
expect([committedA, committedB]).toEqual(['1', '1']);
788-
});
789-
790-
// Unmount everything
791-
await act(async () => {
792-
ReactNoop.render(null);
793-
await waitForAll([
794-
// Insertion and layout destroys (mutation/layout phase)
795-
'Destroy Insertion A [A: 1, B: 1], event: Event A [A: 1, B: 1]',
796-
'Destroy Layout A [A: 1, B: 1], event: Event A [A: 1, B: 1]',
797-
'Destroy Insertion B [A: 1, B: 1], event: Event B [A: 1, B: 1]',
798-
'Destroy Layout B [A: 1, B: 1], event: Event B [A: 1, B: 1]',
799-
// Passive destroys
800-
'Destroy Passive A [A: 1, B: 1], event: Event A [A: 1, B: 1]',
801-
'Destroy Passive B [A: 1, B: 1], event: Event B [A: 1, B: 1]',
802-
]);
803-
});
804-
});
805-
806598
it("doesn't provide a stable identity", async () => {
807599
function Counter({shouldRender, value}) {
808600
const onClick = useEffectEvent(() => {

‎packages/shared/ReactFeatureFlags.js‎

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -125,10 +125,6 @@ export const enableFizzExternalRuntime = __EXPERIMENTAL__;
125125

126126
export const alwaysThrottleRetries: boolean = true;
127127

128-
// Gate whether useEffectEvent uses the mutation phase (true) or before-mutation
129-
// phase (false) for updating event function references.
130-
export const enableEffectEventMutationPhase: boolean = false;
131-
132128
export const passChildrenWhenCloningPersistedNodes: boolean = false;
133129

134130
export const enableEagerAlternateStateNodeCleanup: boolean = true;

‎packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -27,4 +27,3 @@ export const enableFragmentRefs = __VARIANT__;
2727
export const enableFragmentRefsScrollIntoView = __VARIANT__;
2828
export const enableFragmentRefsInstanceHandles = __VARIANT__;
2929
export const enableComponentPerformanceTrack = __VARIANT__;
30-
export const enableEffectEventMutationPhase = __VARIANT__;

‎packages/shared/forks/ReactFeatureFlags.native-fb.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -20,7 +20,6 @@ const dynamicFlags: DynamicExportsType = (dynamicFlagsUntyped: any);
2020
// the exports object every time a flag is read.
2121
export const {
2222
alwaysThrottleRetries,
23-
enableEffectEventMutationPhase,
2423
enableHiddenSubtreeInsertionEffectCleanup,
2524
enableObjectFiber,
2625
enableEagerAlternateStateNodeCleanup,

‎packages/shared/forks/ReactFeatureFlags.native-oss.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,6 @@ export const enableSchedulingProfiler: boolean =
4646
!enableComponentPerformanceTrack && __PROFILE__;
4747
export const enableScopeAPI: boolean = false;
4848
export const enableEagerAlternateStateNodeCleanup: boolean = true;
49-
export const enableEffectEventMutationPhase: boolean = false;
5049
export const enableSuspenseAvoidThisFallback: boolean = false;
5150
export const enableSuspenseCallback: boolean = false;
5251
export const enableTaint: boolean = true;

‎packages/shared/forks/ReactFeatureFlags.test-renderer.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -58,7 +58,6 @@ export const enableInfiniteRenderLoopDetection: boolean = false;
5858

5959
export const renameElementSymbol: boolean = true;
6060
export const enableEagerAlternateStateNodeCleanup: boolean = true;
61-
export const enableEffectEventMutationPhase: boolean = false;
6261

6362
export const enableYieldingBeforePassive: boolean = true;
6463

‎packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,6 @@ export const enableComponentPerformanceTrack = false;
4343
export const enablePerformanceIssueReporting = false;
4444
export const enableScopeAPI = false;
4545
export const enableEagerAlternateStateNodeCleanup = true;
46-
export const enableEffectEventMutationPhase = false;
4746
export const enableSuspenseAvoidThisFallback = false;
4847
export const enableSuspenseCallback = false;
4948
export const enableTaint = true;

‎packages/shared/forks/ReactFeatureFlags.test-renderer.www.js‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -64,7 +64,6 @@ export const renameElementSymbol: boolean = false;
6464

6565
export const enableObjectFiber: boolean = false;
6666
export const enableEagerAlternateStateNodeCleanup: boolean = true;
67-
export const enableEffectEventMutationPhase: boolean = false;
6867

6968
export const enableHydrationLaneScheduling: boolean = true;
7069

0 commit comments

Comments
 (0)