Skip to content

Commit

Permalink
[useEvent] Non-stable function identity (#25473)
Browse files Browse the repository at this point in the history 
* [useEvent] Non-stable function identity

Since useEvent shouldn't go in the dependency list of whatever is
consuming it (which is enforced by the fact that useEvent functions are
always locally created and never passed by reference), its identity
doesn't matter. Effectively, this PR is a runtime assertion
that you can't rely on the return value of useEvent to be stable.

* Test: Events should see latest bindings

The key feature of useEvent that makes it different from useCallback
is that events always see the latest committed values. There's no such
thing as a "stale" event handler.

* Don't queue a commit effect on mount

* Inline event function wrapping

- Inlines wrapping of the callback
- Use a mutable ref-style object instead of a callable object
- Fix types

Co-authored-by: Andrew Clark <git@andrewclark.io>
  • Loading branch information
@poteto @acdlite
poteto and acdlite committed Oct 19, 2022
1 parent 9872928 commit 3cc792b
Show file tree
Hide file tree
Showing 7 changed files with 179 additions and 99 deletions.
16 changes: 4 additions & 12 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,7 @@ import type {
ChildSet,
UpdatePayload,
} from './ReactFiberHostConfig';
import type {
Fiber,
FiberRoot,
EventFunctionWrapper,
} from './ReactInternalTypes';
import type {Fiber, FiberRoot} from './ReactInternalTypes';
import type {Lanes} from './ReactFiberLane.new';
import type {SuspenseState} from './ReactFiberSuspenseComponent.new';
import type {UpdateQueue} from './ReactFiberClassUpdateQueue.new';
Expand Down Expand Up @@ -689,13 +685,9 @@ function commitUseEventMount(finishedWork: Fiber) {
const updateQueue: FunctionComponentUpdateQueue | null = (finishedWork.updateQueue: any);
const eventPayloads = updateQueue !== null ? updateQueue.events : null;
if (eventPayloads !== null) {
// FunctionComponentUpdateQueue.events is a flat array of
// [EventFunctionWrapper, EventFunction, ...], so increment by 2 each iteration to find the next
// pair.
for (let ii = 0; ii < eventPayloads.length; ii += 2) {
const eventFn: EventFunctionWrapper<any, any, any> = eventPayloads[ii];
const nextImpl = eventPayloads[ii + 1];
eventFn._impl = nextImpl;
for (let ii = 0; ii < eventPayloads.length; ii++) {
const {ref, nextImpl} = eventPayloads[ii];
ref.impl = nextImpl;
}
}
}
Expand Down
16 changes: 4 additions & 12 deletions packages/react-reconciler/src/ReactFiberCommitWork.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,11 +15,7 @@ import type {
ChildSet,
UpdatePayload,
} from './ReactFiberHostConfig';
import type {
Fiber,
FiberRoot,
EventFunctionWrapper,
} from './ReactInternalTypes';
import type {Fiber, FiberRoot} from './ReactInternalTypes';
import type {Lanes} from './ReactFiberLane.old';
import type {SuspenseState} from './ReactFiberSuspenseComponent.old';
import type {UpdateQueue} from './ReactFiberClassUpdateQueue.old';
Expand Down Expand Up @@ -689,13 +685,9 @@ function commitUseEventMount(finishedWork: Fiber) {
const updateQueue: FunctionComponentUpdateQueue | null = (finishedWork.updateQueue: any);
const eventPayloads = updateQueue !== null ? updateQueue.events : null;
if (eventPayloads !== null) {
// FunctionComponentUpdateQueue.events is a flat array of
// [EventFunctionWrapper, EventFunction, ...], so increment by 2 each iteration to find the next
// pair.
for (let ii = 0; ii < eventPayloads.length; ii += 2) {
const eventFn: EventFunctionWrapper<any, any, any> = eventPayloads[ii];
const nextImpl = eventPayloads[ii + 1];
eventFn._impl = nextImpl;
for (let ii = 0; ii < eventPayloads.length; ii++) {
const {ref, nextImpl} = eventPayloads[ii];
ref.impl = nextImpl;
}
}
}
Expand Down
65 changes: 38 additions & 27 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import type {
Dispatcher,
HookType,
MemoCache,
EventFunctionWrapper,
} from './ReactInternalTypes';
import type {Lanes, Lane} from './ReactFiberLane.new';
import type {HookFlags} from './ReactHookEffectTags';
Expand Down Expand Up @@ -189,9 +188,17 @@ type StoreConsistencyCheck<T> = {
getSnapshot: () => T,
};

type EventFunctionPayload<Args, Return, F: (...Array<Args>) => Return> = {
ref: {
eventFn: F,
impl: F,
},
nextImpl: F,
};

export type FunctionComponentUpdateQueue = {
lastEffect: Effect | null,
events: Array<() => mixed> | null,
events: Array<EventFunctionPayload<any, any, any>> | null,
stores: Array<StoreConsistencyCheck<any>> | null,
// NOTE: optional, only set when enableUseMemoCacheHook is enabled
memoCache?: MemoCache | null,
Expand Down Expand Up @@ -1909,52 +1916,56 @@ function updateEffect(
}

function useEventImpl<Args, Return, F: (...Array<Args>) => Return>(
event: EventFunctionWrapper<Args, Return, F>,
nextImpl: F,
payload: EventFunctionPayload<Args, Return, F>,
) {
currentlyRenderingFiber.flags |= UpdateEffect;
let componentUpdateQueue: null | FunctionComponentUpdateQueue = (currentlyRenderingFiber.updateQueue: any);
if (componentUpdateQueue === null) {
componentUpdateQueue = createFunctionComponentUpdateQueue();
currentlyRenderingFiber.updateQueue = (componentUpdateQueue: any);
componentUpdateQueue.events = [event, nextImpl];
componentUpdateQueue.events = [payload];
} else {
const events = componentUpdateQueue.events;
if (events === null) {
componentUpdateQueue.events = [event, nextImpl];
componentUpdateQueue.events = [payload];
} else {
events.push(event, nextImpl);
events.push(payload);
}
}
}

function mountEvent<Args, Return, F: (...Array<Args>) => Return>(
callback: F,
): EventFunctionWrapper<Args, Return, F> {
): F {
const hook = mountWorkInProgressHook();
const eventFn: EventFunctionWrapper<Args, Return, F> = function eventFn() {
const ref = {impl: callback};
hook.memoizedState = ref;
// $FlowIgnore[incompatible-return]
return function eventFn() {
if (isInvalidExecutionContextForEventFunction()) {
throw new Error(
"A function wrapped in useEvent can't be called during rendering.",
);
}
// $FlowFixMe[prop-missing] found when upgrading Flow
return eventFn._impl.apply(undefined, arguments);
return ref.impl.apply(undefined, arguments);
};
eventFn._impl = callback;

useEventImpl(eventFn, callback);
hook.memoizedState = eventFn;
return eventFn;
}

function updateEvent<Args, Return, F: (...Array<Args>) => Return>(
callback: F,
): EventFunctionWrapper<Args, Return, F> {
): F {
const hook = updateWorkInProgressHook();
const eventFn = hook.memoizedState;
useEventImpl(eventFn, callback);
return eventFn;
const ref = hook.memoizedState;
useEventImpl({ref, nextImpl: callback});
// $FlowIgnore[incompatible-return]
return function eventFn() {
if (isInvalidExecutionContextForEventFunction()) {
throw new Error(
"A function wrapped in useEvent can't be called during rendering.",
);
}
return ref.impl.apply(undefined, arguments);
};
}

function mountInsertionEffect(
Expand Down Expand Up @@ -2916,7 +2927,7 @@ if (__DEV__) {
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
>(callback: F): F {
currentHookNameInDev = 'useEvent';
mountHookTypesDev();
return mountEvent(callback);
Expand Down Expand Up @@ -3073,7 +3084,7 @@ if (__DEV__) {
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
>(callback: F): F {
currentHookNameInDev = 'useEvent';
updateHookTypesDev();
return mountEvent(callback);
Expand Down Expand Up @@ -3230,7 +3241,7 @@ if (__DEV__) {
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
>(callback: F): F {
currentHookNameInDev = 'useEvent';
updateHookTypesDev();
return updateEvent(callback);
Expand Down Expand Up @@ -3388,7 +3399,7 @@ if (__DEV__) {
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
>(callback: F): F {
currentHookNameInDev = 'useEvent';
updateHookTypesDev();
return updateEvent(callback);
Expand Down Expand Up @@ -3572,7 +3583,7 @@ if (__DEV__) {
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
>(callback: F): F {
currentHookNameInDev = 'useEvent';
warnInvalidHookAccess();
mountHookTypesDev();
Expand Down Expand Up @@ -3757,7 +3768,7 @@ if (__DEV__) {
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
>(callback: F): F {
currentHookNameInDev = 'useEvent';
warnInvalidHookAccess();
updateHookTypesDev();
Expand Down Expand Up @@ -3943,7 +3954,7 @@ if (__DEV__) {
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
>(callback: F): F {
currentHookNameInDev = 'useEvent';
warnInvalidHookAccess();
updateHookTypesDev();
Expand Down
65 changes: 38 additions & 27 deletions packages/react-reconciler/src/ReactFiberHooks.old.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,6 @@ import type {
Dispatcher,
HookType,
MemoCache,
EventFunctionWrapper,
} from './ReactInternalTypes';
import type {Lanes, Lane} from './ReactFiberLane.old';
import type {HookFlags} from './ReactHookEffectTags';
Expand Down Expand Up @@ -189,9 +188,17 @@ type StoreConsistencyCheck<T> = {
getSnapshot: () => T,
};

type EventFunctionPayload<Args, Return, F: (...Array<Args>) => Return> = {
ref: {
eventFn: F,
impl: F,
},
nextImpl: F,
};

export type FunctionComponentUpdateQueue = {
lastEffect: Effect | null,
events: Array<() => mixed> | null,
events: Array<EventFunctionPayload<any, any, any>> | null,
stores: Array<StoreConsistencyCheck<any>> | null,
// NOTE: optional, only set when enableUseMemoCacheHook is enabled
memoCache?: MemoCache | null,
Expand Down Expand Up @@ -1909,52 +1916,56 @@ function updateEffect(
}

function useEventImpl<Args, Return, F: (...Array<Args>) => Return>(
event: EventFunctionWrapper<Args, Return, F>,
nextImpl: F,
payload: EventFunctionPayload<Args, Return, F>,
) {
currentlyRenderingFiber.flags |= UpdateEffect;
let componentUpdateQueue: null | FunctionComponentUpdateQueue = (currentlyRenderingFiber.updateQueue: any);
if (componentUpdateQueue === null) {
componentUpdateQueue = createFunctionComponentUpdateQueue();
currentlyRenderingFiber.updateQueue = (componentUpdateQueue: any);
componentUpdateQueue.events = [event, nextImpl];
componentUpdateQueue.events = [payload];
} else {
const events = componentUpdateQueue.events;
if (events === null) {
componentUpdateQueue.events = [event, nextImpl];
componentUpdateQueue.events = [payload];
} else {
events.push(event, nextImpl);
events.push(payload);
}
}
}

function mountEvent<Args, Return, F: (...Array<Args>) => Return>(
callback: F,
): EventFunctionWrapper<Args, Return, F> {
): F {
const hook = mountWorkInProgressHook();
const eventFn: EventFunctionWrapper<Args, Return, F> = function eventFn() {
const ref = {impl: callback};
hook.memoizedState = ref;
// $FlowIgnore[incompatible-return]
return function eventFn() {
if (isInvalidExecutionContextForEventFunction()) {
throw new Error(
"A function wrapped in useEvent can't be called during rendering.",
);
}
// $FlowFixMe[prop-missing] found when upgrading Flow
return eventFn._impl.apply(undefined, arguments);
return ref.impl.apply(undefined, arguments);
};
eventFn._impl = callback;

useEventImpl(eventFn, callback);
hook.memoizedState = eventFn;
return eventFn;
}

function updateEvent<Args, Return, F: (...Array<Args>) => Return>(
callback: F,
): EventFunctionWrapper<Args, Return, F> {
): F {
const hook = updateWorkInProgressHook();
const eventFn = hook.memoizedState;
useEventImpl(eventFn, callback);
return eventFn;
const ref = hook.memoizedState;
useEventImpl({ref, nextImpl: callback});
// $FlowIgnore[incompatible-return]
return function eventFn() {
if (isInvalidExecutionContextForEventFunction()) {
throw new Error(
"A function wrapped in useEvent can't be called during rendering.",
);
}
return ref.impl.apply(undefined, arguments);
};
}

function mountInsertionEffect(
Expand Down Expand Up @@ -2916,7 +2927,7 @@ if (__DEV__) {
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
>(callback: F): F {
currentHookNameInDev = 'useEvent';
mountHookTypesDev();
return mountEvent(callback);
Expand Down Expand Up @@ -3073,7 +3084,7 @@ if (__DEV__) {
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
>(callback: F): F {
currentHookNameInDev = 'useEvent';
updateHookTypesDev();
return mountEvent(callback);
Expand Down Expand Up @@ -3230,7 +3241,7 @@ if (__DEV__) {
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
>(callback: F): F {
currentHookNameInDev = 'useEvent';
updateHookTypesDev();
return updateEvent(callback);
Expand Down Expand Up @@ -3388,7 +3399,7 @@ if (__DEV__) {
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
>(callback: F): F {
currentHookNameInDev = 'useEvent';
updateHookTypesDev();
return updateEvent(callback);
Expand Down Expand Up @@ -3572,7 +3583,7 @@ if (__DEV__) {
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
>(callback: F): F {
currentHookNameInDev = 'useEvent';
warnInvalidHookAccess();
mountHookTypesDev();
Expand Down Expand Up @@ -3757,7 +3768,7 @@ if (__DEV__) {
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
>(callback: F): F {
currentHookNameInDev = 'useEvent';
warnInvalidHookAccess();
updateHookTypesDev();
Expand Down Expand Up @@ -3943,7 +3954,7 @@ if (__DEV__) {
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
>(callback: F): F {
currentHookNameInDev = 'useEvent';
warnInvalidHookAccess();
updateHookTypesDev();
Expand Down
Loading

0 comments on commit 3cc792b

Please sign in to comment.