Skip to content

Refactor useEvent #25336

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 5 commits into from
Sep 29, 2022
Merged

Refactor useEvent #25336

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
5158b13
Refactor useEvent
poteto Sep 27, 2022
a015335
Naming changes
poteto Sep 29, 2022
a731886
Flow types
poteto Sep 29, 2022
436d6a5
Flatten EventFunctionPayload
poteto Sep 29, 2022
2abf7db
Apparently Flow doesn't like variables called event
poteto Sep 29, 2022
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
21 changes: 17 additions & 4 deletions packages/react-reconciler/src/ReactFiberCommitWork.new.js
View file
Open in desktop
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
View file
Open in desktop
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
View file
Open in desktop
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,
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

useMemoCache defines an optional property on the queue hence the compile time check for the flag. But it's not easy to add another compile time check for enableUseEventHook in a way that lets the various permutations of the flags coexist. So I'm opting to just initialize the property on this object in every case for hidden classes and with the assumption that we'll eventually release useEvent (after deciding what to do with naming etc)

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