Skip to content
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
16 changes: 1 addition & 15 deletions packages/react-reconciler/src/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,6 @@ import type {ViewTransitionState} from './ReactFiberViewTransitionComponent';
import {
alwaysThrottleRetries,
enableCreateEventHandleAPI,
enableEffectEventMutationPhase,
enableHiddenSubtreeInsertionEffectCleanup,
enableProfilerTimer,
enableProfilerCommitHooks,
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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,
Expand Down
14 changes: 5 additions & 9 deletions packages/react-reconciler/src/ReactFiberFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -7,10 +7,7 @@
* @flow
*/

import {
enableCreateEventHandleAPI,
enableEffectEventMutationPhase,
} from 'shared/ReactFeatureFlags';
import {enableCreateEventHandleAPI} from 'shared/ReactFeatureFlags';

export type Flags = number;

Expand Down Expand Up @@ -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.
Expand Down
208 changes: 0 additions & 208 deletions packages/react-reconciler/src/__tests__/useEffectEvent-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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 <Child value={value} />;
}

function Child({value}) {
const childEvent = useEffectEvent(() => {
Scheduler.log('Child event: ' + value);
});

useInsertionEffect(() => {
Scheduler.log('Child insertion');
childEvent();
}, [value]);

return null;
}

ReactNoop.render(<Parent value={1} />);
await waitForAll([
'Child insertion',
'Child event: 1',
'Parent insertion',
'Parent event: 1',
]);

await act(() => ReactNoop.render(<Parent value={2} />));
// 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(
<React.Fragment>
<CounterA count={0} />
<CounterB count={0} />
</React.Fragment>,
);
// 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(
<React.Fragment>
<CounterA count={1} />
<CounterB count={1} />
</React.Fragment>,
);
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(() => {
Expand Down
4 changes: 0 additions & 4 deletions packages/shared/ReactFeatureFlags.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,3 @@ export const enableFragmentRefs = __VARIANT__;
export const enableFragmentRefsScrollIntoView = __VARIANT__;
export const enableFragmentRefsInstanceHandles = __VARIANT__;
export const enableComponentPerformanceTrack = __VARIANT__;
export const enableEffectEventMutationPhase = __VARIANT__;
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-fb.js
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.native-oss.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.test-renderer.js
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down
2 changes: 0 additions & 2 deletions packages/shared/forks/ReactFeatureFlags.www-dynamic.js
Original file line number Diff line number Diff line change
Expand Up @@ -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__.
Expand Down
1 change: 0 additions & 1 deletion packages/shared/forks/ReactFeatureFlags.www.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,6 @@ export const {
alwaysThrottleRetries,
disableLegacyContextForFunctionComponents,
disableSchedulerTimeoutInWorkLoop,
enableEffectEventMutationPhase,
enableHiddenSubtreeInsertionEffectCleanup,
enableInfiniteRenderLoopDetection,
enableNoCloningMemoCache,
Expand Down
Loading