Skip to content

Commit

Permalink
Refactor useEvent (#25336)
Browse files Browse the repository at this point in the history
* Refactor useEvent

Previously, the useEvent implementation made use of effect infra under
the hood. This was a lot of extra overhead for functionality we didn't
use (events have no deps, and no clean up functions). This PR refactors
the implementation to instead use a queue to ensure that the callback is
stable across renders.

Additionally, the function signature was updated to infer the callback's argument types and return value. While this doesn't affect anything internal it more accurately describes what's being passed.
  • Loading branch information
poteto authored Sep 29, 2022
1 parent abd7bcd commit 3517bd9
Show file tree
Hide file tree
Showing 8 changed files with 257 additions and 117 deletions.
21 changes: 17 additions & 4 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import type {
UpdatePayload,
} from './ReactFiberHostConfig';
import type {Fiber} from './ReactInternalTypes';
import type {FiberRoot} from './ReactInternalTypes';
import type {FiberRoot, EventFunctionWrapper} from './ReactInternalTypes';
import type {Lanes} from './ReactFiberLane.new';
import type {SuspenseState} from './ReactFiberSuspenseComponent.new';
import type {UpdateQueue} from './ReactFiberClassUpdateQueue.new';
Expand Down Expand Up @@ -164,7 +164,6 @@ import {
Layout as HookLayout,
Insertion as HookInsertion,
Passive as HookPassive,
Snapshot as HookSnapshot,
} from './ReactHookEffectTags';
import {didWarnAboutReassigningProps} from './ReactFiberBeginWork.new';
import {doesFiberContain} from './ReactFiberTreeReflection';
Expand Down Expand Up @@ -416,8 +415,7 @@ function commitBeforeMutationEffectsOnFiber(finishedWork: Fiber) {
case FunctionComponent: {
if (enableUseEventHook) {
if ((flags & Update) !== NoFlags) {
// useEvent doesn't need to be cleaned up
commitHookEffectListMount(HookSnapshot | HookHasEffect, finishedWork);
commitUseEventMount(finishedWork);
}
}
break;
Expand Down Expand Up @@ -665,6 +663,21 @@ function commitHookEffectListMount(flags: HookFlags, finishedWork: Fiber) {
}
}

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;
}
}
}

export function commitPassiveEffectDurations(
finishedRoot: FiberRoot,
finishedWork: Fiber,
Expand Down
21 changes: 17 additions & 4 deletions packages/react-reconciler/src/ReactFiberCommitWork.old.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import type {
UpdatePayload,
} from './ReactFiberHostConfig';
import type {Fiber} from './ReactInternalTypes';
import type {FiberRoot} from './ReactInternalTypes';
import type {FiberRoot, EventFunctionWrapper} from './ReactInternalTypes';
import type {Lanes} from './ReactFiberLane.old';
import type {SuspenseState} from './ReactFiberSuspenseComponent.old';
import type {UpdateQueue} from './ReactFiberClassUpdateQueue.old';
Expand Down Expand Up @@ -164,7 +164,6 @@ import {
Layout as HookLayout,
Insertion as HookInsertion,
Passive as HookPassive,
Snapshot as HookSnapshot,
} from './ReactHookEffectTags';
import {didWarnAboutReassigningProps} from './ReactFiberBeginWork.old';
import {doesFiberContain} from './ReactFiberTreeReflection';
Expand Down Expand Up @@ -416,8 +415,7 @@ function commitBeforeMutationEffectsOnFiber(finishedWork: Fiber) {
case FunctionComponent: {
if (enableUseEventHook) {
if ((flags & Update) !== NoFlags) {
// useEvent doesn't need to be cleaned up
commitHookEffectListMount(HookSnapshot | HookHasEffect, finishedWork);
commitUseEventMount(finishedWork);
}
}
break;
Expand Down Expand Up @@ -665,6 +663,21 @@ function commitHookEffectListMount(flags: HookFlags, finishedWork: Fiber) {
}
}

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;
}
}
}

export function commitPassiveEffectDurations(
finishedRoot: FiberRoot,
finishedWork: Fiber,
Expand Down
120 changes: 70 additions & 50 deletions packages/react-reconciler/src/ReactFiberHooks.new.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@ import type {
Dispatcher,
HookType,
MemoCache,
EventFunctionWrapper,
} from './ReactInternalTypes';
import type {Lanes, Lane} from './ReactFiberLane.new';
import type {HookFlags} from './ReactHookEffectTags';
Expand Down Expand Up @@ -86,7 +87,6 @@ import {
Layout as HookLayout,
Passive as HookPassive,
Insertion as HookInsertion,
Snapshot as HookSnapshot,
} from './ReactHookEffectTags';
import {
getWorkInProgressRoot,
Expand Down Expand Up @@ -184,6 +184,7 @@ type StoreConsistencyCheck<T> = {

export type FunctionComponentUpdateQueue = {
lastEffect: Effect | null,
events: Array<() => mixed> | null,
stores: Array<StoreConsistencyCheck<any>> | null,
// NOTE: optional, only set when enableUseMemoCacheHook is enabled
memoCache?: MemoCache | null,
Expand Down Expand Up @@ -727,6 +728,7 @@ if (enableUseMemoCacheHook) {
createFunctionComponentUpdateQueue = () => {
return {
lastEffect: null,
events: null,
stores: null,
memoCache: null,
};
Expand All @@ -735,6 +737,7 @@ if (enableUseMemoCacheHook) {
createFunctionComponentUpdateQueue = () => {
return {
lastEffect: null,
events: null,
stores: null,
};
};
Expand Down Expand Up @@ -1871,49 +1874,52 @@ function updateEffect(
return updateEffectImpl(PassiveEffect, HookPassive, create, deps);
}

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

function event() {
function mountEvent<Args, Return, F: (...Array<Args>) => Return>(
callback: F,
): EventFunctionWrapper<Args, Return, F> {
const hook = mountWorkInProgressHook();
const eventFn: EventFunctionWrapper<Args, Return, F> = function eventFn() {
if (isInvalidExecutionContextForEventFunction()) {
throw new Error(
"A function wrapped in useEvent can't be called during rendering.",
);
}
return ref.current.apply(undefined, arguments);
}

// TODO: We don't need all the overhead of an effect object since there are no deps and no
// clean up functions.
mountEffectImpl(
UpdateEffect,
HookSnapshot,
() => {
ref.current = callback;
},
[ref, callback],
);

hook.memoizedState = [ref, event];
return eventFn._impl.apply(undefined, arguments);
};
eventFn._impl = callback;

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

function updateEvent<T>(callback: () => T): () => T {
function updateEvent<Args, Return, F: (...Array<Args>) => Return>(
callback: F,
): EventFunctionWrapper<Args, Return, F> {
const hook = updateWorkInProgressHook();
const ref = hook.memoizedState[0];

updateEffectImpl(
UpdateEffect,
HookSnapshot,
() => {
ref.current = callback;
},
[ref, callback],
);

return hook.memoizedState[1];
const eventFn = hook.memoizedState;
useEventImpl(eventFn, callback);
return eventFn;
}

function mountInsertionEffect(
Expand Down Expand Up @@ -2890,9 +2896,11 @@ if (__DEV__) {
(HooksDispatcherOnMountInDEV: Dispatcher).useMemoCache = useMemoCache;
}
if (enableUseEventHook) {
(HooksDispatcherOnMountInDEV: Dispatcher).useEvent = function useEvent<T>(
callback: () => T,
): () => T {
(HooksDispatcherOnMountInDEV: Dispatcher).useEvent = function useEvent<
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
currentHookNameInDev = 'useEvent';
mountHookTypesDev();
return mountEvent(callback);
Expand Down Expand Up @@ -3048,8 +3056,10 @@ if (__DEV__) {
}
if (enableUseEventHook) {
(HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher).useEvent = function useEvent<
T,
>(callback: () => T): () => T {
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
currentHookNameInDev = 'useEvent';
updateHookTypesDev();
return mountEvent(callback);
Expand Down Expand Up @@ -3204,9 +3214,11 @@ if (__DEV__) {
(HooksDispatcherOnUpdateInDEV: Dispatcher).useMemoCache = useMemoCache;
}
if (enableUseEventHook) {
(HooksDispatcherOnUpdateInDEV: Dispatcher).useEvent = function useEvent<T>(
callback: () => T,
): () => T {
(HooksDispatcherOnUpdateInDEV: Dispatcher).useEvent = function useEvent<
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
currentHookNameInDev = 'useEvent';
updateHookTypesDev();
return updateEvent(callback);
Expand Down Expand Up @@ -3363,8 +3375,10 @@ if (__DEV__) {
}
if (enableUseEventHook) {
(HooksDispatcherOnRerenderInDEV: Dispatcher).useEvent = function useEvent<
T,
>(callback: () => T): () => T {
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
currentHookNameInDev = 'useEvent';
updateHookTypesDev();
return updateEvent(callback);
Expand Down Expand Up @@ -3547,8 +3561,10 @@ if (__DEV__) {
}
if (enableUseEventHook) {
(InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher).useEvent = function useEvent<
T,
>(callback: () => T): () => T {
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
currentHookNameInDev = 'useEvent';
warnInvalidHookAccess();
mountHookTypesDev();
Expand Down Expand Up @@ -3732,8 +3748,10 @@ if (__DEV__) {
}
if (enableUseEventHook) {
(InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher).useEvent = function useEvent<
T,
>(callback: () => T): () => T {
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
currentHookNameInDev = 'useEvent';
warnInvalidHookAccess();
updateHookTypesDev();
Expand Down Expand Up @@ -3918,8 +3936,10 @@ if (__DEV__) {
}
if (enableUseEventHook) {
(InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher).useEvent = function useEvent<
T,
>(callback: () => T): () => T {
Args,
Return,
F: (...Array<Args>) => Return,
>(callback: F): EventFunctionWrapper<Args, Return, F> {
currentHookNameInDev = 'useEvent';
warnInvalidHookAccess();
updateHookTypesDev();
Expand Down
Loading

0 comments on commit 3517bd9

Please sign in to comment.