diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index 946e758c1dc..43db744007d 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -47,7 +47,6 @@ import type {ViewTransitionState} from './ReactFiberViewTransitionComponent'; import { alwaysThrottleRetries, enableCreateEventHandleAPI, - enableEffectEventMutationPhase, enableHiddenSubtreeInsertionEffectCleanup, enableProfilerTimer, enableProfilerCommitHooks, @@ -500,7 +499,7 @@ function commitBeforeMutationEffectsOnFiber( case FunctionComponent: case ForwardRef: case SimpleMemoComponent: { - if (!enableEffectEventMutationPhase && (flags & Update) !== NoFlags) { + if ((flags & Update) !== NoFlags) { const updateQueue: FunctionComponentUpdateQueue | null = (finishedWork.updateQueue: any); const eventPayloads = updateQueue !== null ? updateQueue.events : null; @@ -2047,19 +2046,6 @@ function commitMutationEffectsOnFiber( commitReconciliationEffects(finishedWork, lanes); if (flags & Update) { - // Mutate event effect callbacks before insertion effects. - if (enableEffectEventMutationPhase) { - const updateQueue: FunctionComponentUpdateQueue | null = - (finishedWork.updateQueue: any); - const eventPayloads = - updateQueue !== null ? updateQueue.events : null; - if (eventPayloads !== null) { - for (let ii = 0; ii < eventPayloads.length; ii++) { - const {ref, nextImpl} = eventPayloads[ii]; - ref.impl = nextImpl; - } - } - } commitHookEffectListUnmount( HookInsertion | HookHasEffect, finishedWork, diff --git a/packages/react-reconciler/src/ReactFiberFlags.js b/packages/react-reconciler/src/ReactFiberFlags.js index 9f85897fb05..2d8d3f7fd16 100644 --- a/packages/react-reconciler/src/ReactFiberFlags.js +++ b/packages/react-reconciler/src/ReactFiberFlags.js @@ -7,10 +7,7 @@ * @flow */ -import { - enableCreateEventHandleAPI, - enableEffectEventMutationPhase, -} from 'shared/ReactFeatureFlags'; +import {enableCreateEventHandleAPI} from 'shared/ReactFeatureFlags'; export type Flags = number; @@ -102,11 +99,10 @@ export const BeforeMutationMask: number = // TODO: Only need to visit Deletions during BeforeMutation phase if an // element is focused. Update | ChildDeletion | Visibility - : // useEffectEvent uses the snapshot phase, - // but we're moving it to the mutation phase. - enableEffectEventMutationPhase - ? 0 - : Update); + : // TODO: The useEffectEvent hook uses the snapshot phase for clean up but it + // really should use the mutation phase for this or at least schedule an + // explicit Snapshot phase flag for this. + Update); // For View Transition support we use the snapshot phase to scan the tree for potentially // affected ViewTransition components. diff --git a/packages/react-reconciler/src/__tests__/useEffectEvent-test.js b/packages/react-reconciler/src/__tests__/useEffectEvent-test.js index 0d903d59e4a..2b5af2206b9 100644 --- a/packages/react-reconciler/src/__tests__/useEffectEvent-test.js +++ b/packages/react-reconciler/src/__tests__/useEffectEvent-test.js @@ -595,214 +595,6 @@ describe('useEffectEvent', () => { assertLog(['Effect value: 2', 'Event value: 2']); }); - it('updates parent and child event effects before their respective effect lifecycles', async () => { - function Parent({value}) { - const parentEvent = useEffectEvent(() => { - Scheduler.log('Parent event: ' + value); - }); - - useInsertionEffect(() => { - Scheduler.log('Parent insertion'); - parentEvent(); - }, [value]); - - return ; - } - - function Child({value}) { - const childEvent = useEffectEvent(() => { - Scheduler.log('Child event: ' + value); - }); - - useInsertionEffect(() => { - Scheduler.log('Child insertion'); - childEvent(); - }, [value]); - - return null; - } - - ReactNoop.render(); - await waitForAll([ - 'Child insertion', - 'Child event: 1', - 'Parent insertion', - 'Parent event: 1', - ]); - - await act(() => ReactNoop.render()); - // Each component's event is updated before its own insertion effect runs - assertLog([ - 'Child insertion', - 'Child event: 2', - 'Parent insertion', - 'Parent event: 2', - ]); - }); - - it('fires all insertion effects (interleaved) with useEffectEvent before firing any layout effects', async () => { - // This test mirrors the 'fires all insertion effects (interleaved) before firing any layout effects' - // test in ReactHooksWithNoopRenderer-test.js, but adds useEffectEvent to verify that - // event payloads are updated before each component's insertion effects run. - // It also includes passive effects to verify the full effect lifecycle. - let committedA = '(empty)'; - let committedB = '(empty)'; - - function CounterA(props) { - const onEvent = useEffectEvent(() => { - return `Event A [A: ${committedA}, B: ${committedB}]`; - }); - - useInsertionEffect(() => { - // Call the event function to verify it sees the latest value - Scheduler.log( - `Create Insertion A [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`, - ); - committedA = String(props.count); - return () => { - Scheduler.log( - `Destroy Insertion A [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`, - ); - }; - }); - - useLayoutEffect(() => { - Scheduler.log( - `Create Layout A [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`, - ); - return () => { - Scheduler.log( - `Destroy Layout A [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`, - ); - }; - }); - - useEffect(() => { - Scheduler.log( - `Create Passive A [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`, - ); - return () => { - Scheduler.log( - `Destroy Passive A [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`, - ); - }; - }); - - return null; - } - - function CounterB(props) { - const onEvent = useEffectEvent(() => { - return `Event B [A: ${committedA}, B: ${committedB}]`; - }); - - useInsertionEffect(() => { - // Call the event function to verify it sees the latest value - Scheduler.log( - `Create Insertion B [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`, - ); - committedB = String(props.count); - return () => { - Scheduler.log( - `Destroy Insertion B [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`, - ); - }; - }); - - useLayoutEffect(() => { - Scheduler.log( - `Create Layout B [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`, - ); - return () => { - Scheduler.log( - `Destroy Layout B [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`, - ); - }; - }); - - useEffect(() => { - Scheduler.log( - `Create Passive B [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`, - ); - return () => { - Scheduler.log( - `Destroy Passive B [A: ${committedA}, B: ${committedB}], event: ${onEvent()}`, - ); - }; - }); - - return null; - } - - await act(async () => { - ReactNoop.render( - - - - , - ); - // All insertion effects fire before all layout effects, then passive effects - // Event functions should see the state AT THE TIME they're called - await waitForAll([ - // Insertion effects (mutation phase) - 'Create Insertion A [A: (empty), B: (empty)], event: Event A [A: (empty), B: (empty)]', - 'Create Insertion B [A: 0, B: (empty)], event: Event B [A: 0, B: (empty)]', - // Layout effects - 'Create Layout A [A: 0, B: 0], event: Event A [A: 0, B: 0]', - 'Create Layout B [A: 0, B: 0], event: Event B [A: 0, B: 0]', - // Passive effects - 'Create Passive A [A: 0, B: 0], event: Event A [A: 0, B: 0]', - 'Create Passive B [A: 0, B: 0], event: Event B [A: 0, B: 0]', - ]); - expect([committedA, committedB]).toEqual(['0', '0']); - }); - - await act(async () => { - ReactNoop.render( - - - - , - ); - await waitForAll([ - // Component A: insertion destroy, then create - 'Destroy Insertion A [A: 0, B: 0], event: Event A [A: 0, B: 0]', - 'Create Insertion A [A: 0, B: 0], event: Event A [A: 0, B: 0]', - // Component A: layout destroy (after insertion updated committedA) - 'Destroy Layout A [A: 1, B: 0], event: Event A [A: 1, B: 0]', - // Component B: insertion destroy, then create - 'Destroy Insertion B [A: 1, B: 0], event: Event B [A: 1, B: 0]', - 'Create Insertion B [A: 1, B: 0], event: Event B [A: 1, B: 0]', - // Component B: layout destroy (after insertion updated committedB) - 'Destroy Layout B [A: 1, B: 1], event: Event B [A: 1, B: 1]', - // Layout creates - 'Create Layout A [A: 1, B: 1], event: Event A [A: 1, B: 1]', - 'Create Layout B [A: 1, B: 1], event: Event B [A: 1, B: 1]', - // Passive destroys then creates - 'Destroy Passive A [A: 1, B: 1], event: Event A [A: 1, B: 1]', - 'Destroy Passive B [A: 1, B: 1], event: Event B [A: 1, B: 1]', - 'Create Passive A [A: 1, B: 1], event: Event A [A: 1, B: 1]', - 'Create Passive B [A: 1, B: 1], event: Event B [A: 1, B: 1]', - ]); - expect([committedA, committedB]).toEqual(['1', '1']); - }); - - // Unmount everything - await act(async () => { - ReactNoop.render(null); - await waitForAll([ - // Insertion and layout destroys (mutation/layout phase) - 'Destroy Insertion A [A: 1, B: 1], event: Event A [A: 1, B: 1]', - 'Destroy Layout A [A: 1, B: 1], event: Event A [A: 1, B: 1]', - 'Destroy Insertion B [A: 1, B: 1], event: Event B [A: 1, B: 1]', - 'Destroy Layout B [A: 1, B: 1], event: Event B [A: 1, B: 1]', - // Passive destroys - 'Destroy Passive A [A: 1, B: 1], event: Event A [A: 1, B: 1]', - 'Destroy Passive B [A: 1, B: 1], event: Event B [A: 1, B: 1]', - ]); - }); - }); - it("doesn't provide a stable identity", async () => { function Counter({shouldRender, value}) { const onClick = useEffectEvent(() => { diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 3e68d8c8991..587e4e6a1da 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -125,10 +125,6 @@ export const enableFizzExternalRuntime = __EXPERIMENTAL__; export const alwaysThrottleRetries: boolean = true; -// Gate whether useEffectEvent uses the mutation phase (true) or before-mutation -// phase (false) for updating event function references. -export const enableEffectEventMutationPhase: boolean = false; - export const passChildrenWhenCloningPersistedNodes: boolean = false; export const enableEagerAlternateStateNodeCleanup: boolean = true; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js index 389d46be497..f139a3fcf28 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js @@ -27,4 +27,3 @@ export const enableFragmentRefs = __VARIANT__; export const enableFragmentRefsScrollIntoView = __VARIANT__; export const enableFragmentRefsInstanceHandles = __VARIANT__; export const enableComponentPerformanceTrack = __VARIANT__; -export const enableEffectEventMutationPhase = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index cb0b0b66f60..e2674b70fee 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -20,7 +20,6 @@ const dynamicFlags: DynamicExportsType = (dynamicFlagsUntyped: any); // the exports object every time a flag is read. export const { alwaysThrottleRetries, - enableEffectEventMutationPhase, enableHiddenSubtreeInsertionEffectCleanup, enableObjectFiber, enableEagerAlternateStateNodeCleanup, diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index f1e697b7b21..672c3755108 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -46,7 +46,6 @@ export const enableSchedulingProfiler: boolean = !enableComponentPerformanceTrack && __PROFILE__; export const enableScopeAPI: boolean = false; export const enableEagerAlternateStateNodeCleanup: boolean = true; -export const enableEffectEventMutationPhase: boolean = false; export const enableSuspenseAvoidThisFallback: boolean = false; export const enableSuspenseCallback: boolean = false; export const enableTaint: boolean = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 8e65aea17b2..67c08d94367 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -58,7 +58,6 @@ export const enableInfiniteRenderLoopDetection: boolean = false; export const renameElementSymbol: boolean = true; export const enableEagerAlternateStateNodeCleanup: boolean = true; -export const enableEffectEventMutationPhase: boolean = false; export const enableYieldingBeforePassive: boolean = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index 120a20160f8..a0ee01baa7d 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -43,7 +43,6 @@ export const enableComponentPerformanceTrack = false; export const enablePerformanceIssueReporting = false; export const enableScopeAPI = false; export const enableEagerAlternateStateNodeCleanup = true; -export const enableEffectEventMutationPhase = false; export const enableSuspenseAvoidThisFallback = false; export const enableSuspenseCallback = false; export const enableTaint = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index dee7642449a..058fd752d6e 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -64,7 +64,6 @@ export const renameElementSymbol: boolean = false; export const enableObjectFiber: boolean = false; export const enableEagerAlternateStateNodeCleanup: boolean = true; -export const enableEffectEventMutationPhase: boolean = false; export const enableHydrationLaneScheduling: boolean = true; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index 206e9ec3a36..1d174609cb9 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -40,8 +40,6 @@ export const enableAsyncDebugInfo: boolean = __VARIANT__; export const enableInternalInstanceMap: boolean = __VARIANT__; -export const enableEffectEventMutationPhase: boolean = __VARIANT__; - // TODO: These flags are hard-coded to the default values used in open source. // Update the tests so that they pass in either mode, then set these // to __VARIANT__. diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 1db222df0bc..0752f266afc 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -18,7 +18,6 @@ export const { alwaysThrottleRetries, disableLegacyContextForFunctionComponents, disableSchedulerTimeoutInWorkLoop, - enableEffectEventMutationPhase, enableHiddenSubtreeInsertionEffectCleanup, enableInfiniteRenderLoopDetection, enableNoCloningMemoCache,