diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.new.js b/packages/react-reconciler/src/ReactFiberCommitWork.new.js index ea8e0cf26354b..e16aa04507961 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.new.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.new.js @@ -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'; @@ -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 = 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; } } } diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.old.js b/packages/react-reconciler/src/ReactFiberCommitWork.old.js index b33a051ee09d0..4aa073d0db31a 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.old.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.old.js @@ -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'; @@ -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 = 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; } } } diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index 8183ddd1557c0..2e399e7897794 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -22,7 +22,6 @@ import type { Dispatcher, HookType, MemoCache, - EventFunctionWrapper, } from './ReactInternalTypes'; import type {Lanes, Lane} from './ReactFiberLane.new'; import type {HookFlags} from './ReactHookEffectTags'; @@ -189,9 +188,17 @@ type StoreConsistencyCheck = { getSnapshot: () => T, }; +type EventFunctionPayload) => Return> = { + ref: { + eventFn: F, + impl: F, + }, + nextImpl: F, +}; + export type FunctionComponentUpdateQueue = { lastEffect: Effect | null, - events: Array<() => mixed> | null, + events: Array> | null, stores: Array> | null, // NOTE: optional, only set when enableUseMemoCacheHook is enabled memoCache?: MemoCache | null, @@ -1909,52 +1916,56 @@ function updateEffect( } function useEventImpl) => Return>( - event: EventFunctionWrapper, - nextImpl: F, + payload: EventFunctionPayload, ) { 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) => Return>( callback: F, -): EventFunctionWrapper { +): F { const hook = mountWorkInProgressHook(); - const eventFn: EventFunctionWrapper = 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) => Return>( callback: F, -): EventFunctionWrapper { +): 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( @@ -2916,7 +2927,7 @@ if (__DEV__) { Args, Return, F: (...Array) => Return, - >(callback: F): EventFunctionWrapper { + >(callback: F): F { currentHookNameInDev = 'useEvent'; mountHookTypesDev(); return mountEvent(callback); @@ -3073,7 +3084,7 @@ if (__DEV__) { Args, Return, F: (...Array) => Return, - >(callback: F): EventFunctionWrapper { + >(callback: F): F { currentHookNameInDev = 'useEvent'; updateHookTypesDev(); return mountEvent(callback); @@ -3230,7 +3241,7 @@ if (__DEV__) { Args, Return, F: (...Array) => Return, - >(callback: F): EventFunctionWrapper { + >(callback: F): F { currentHookNameInDev = 'useEvent'; updateHookTypesDev(); return updateEvent(callback); @@ -3388,7 +3399,7 @@ if (__DEV__) { Args, Return, F: (...Array) => Return, - >(callback: F): EventFunctionWrapper { + >(callback: F): F { currentHookNameInDev = 'useEvent'; updateHookTypesDev(); return updateEvent(callback); @@ -3572,7 +3583,7 @@ if (__DEV__) { Args, Return, F: (...Array) => Return, - >(callback: F): EventFunctionWrapper { + >(callback: F): F { currentHookNameInDev = 'useEvent'; warnInvalidHookAccess(); mountHookTypesDev(); @@ -3757,7 +3768,7 @@ if (__DEV__) { Args, Return, F: (...Array) => Return, - >(callback: F): EventFunctionWrapper { + >(callback: F): F { currentHookNameInDev = 'useEvent'; warnInvalidHookAccess(); updateHookTypesDev(); @@ -3943,7 +3954,7 @@ if (__DEV__) { Args, Return, F: (...Array) => Return, - >(callback: F): EventFunctionWrapper { + >(callback: F): F { currentHookNameInDev = 'useEvent'; warnInvalidHookAccess(); updateHookTypesDev(); diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index 97aa7dcd4a41a..7c4a705517346 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -22,7 +22,6 @@ import type { Dispatcher, HookType, MemoCache, - EventFunctionWrapper, } from './ReactInternalTypes'; import type {Lanes, Lane} from './ReactFiberLane.old'; import type {HookFlags} from './ReactHookEffectTags'; @@ -189,9 +188,17 @@ type StoreConsistencyCheck = { getSnapshot: () => T, }; +type EventFunctionPayload) => Return> = { + ref: { + eventFn: F, + impl: F, + }, + nextImpl: F, +}; + export type FunctionComponentUpdateQueue = { lastEffect: Effect | null, - events: Array<() => mixed> | null, + events: Array> | null, stores: Array> | null, // NOTE: optional, only set when enableUseMemoCacheHook is enabled memoCache?: MemoCache | null, @@ -1909,52 +1916,56 @@ function updateEffect( } function useEventImpl) => Return>( - event: EventFunctionWrapper, - nextImpl: F, + payload: EventFunctionPayload, ) { 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) => Return>( callback: F, -): EventFunctionWrapper { +): F { const hook = mountWorkInProgressHook(); - const eventFn: EventFunctionWrapper = 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) => Return>( callback: F, -): EventFunctionWrapper { +): 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( @@ -2916,7 +2927,7 @@ if (__DEV__) { Args, Return, F: (...Array) => Return, - >(callback: F): EventFunctionWrapper { + >(callback: F): F { currentHookNameInDev = 'useEvent'; mountHookTypesDev(); return mountEvent(callback); @@ -3073,7 +3084,7 @@ if (__DEV__) { Args, Return, F: (...Array) => Return, - >(callback: F): EventFunctionWrapper { + >(callback: F): F { currentHookNameInDev = 'useEvent'; updateHookTypesDev(); return mountEvent(callback); @@ -3230,7 +3241,7 @@ if (__DEV__) { Args, Return, F: (...Array) => Return, - >(callback: F): EventFunctionWrapper { + >(callback: F): F { currentHookNameInDev = 'useEvent'; updateHookTypesDev(); return updateEvent(callback); @@ -3388,7 +3399,7 @@ if (__DEV__) { Args, Return, F: (...Array) => Return, - >(callback: F): EventFunctionWrapper { + >(callback: F): F { currentHookNameInDev = 'useEvent'; updateHookTypesDev(); return updateEvent(callback); @@ -3572,7 +3583,7 @@ if (__DEV__) { Args, Return, F: (...Array) => Return, - >(callback: F): EventFunctionWrapper { + >(callback: F): F { currentHookNameInDev = 'useEvent'; warnInvalidHookAccess(); mountHookTypesDev(); @@ -3757,7 +3768,7 @@ if (__DEV__) { Args, Return, F: (...Array) => Return, - >(callback: F): EventFunctionWrapper { + >(callback: F): F { currentHookNameInDev = 'useEvent'; warnInvalidHookAccess(); updateHookTypesDev(); @@ -3943,7 +3954,7 @@ if (__DEV__) { Args, Return, F: (...Array) => Return, - >(callback: F): EventFunctionWrapper { + >(callback: F): F { currentHookNameInDev = 'useEvent'; warnInvalidHookAccess(); updateHookTypesDev(); diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index 10c4c9853a300..a5f28fd5d2450 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -290,17 +290,6 @@ type SuspenseCallbackOnlyFiberRootProperties = { hydrationCallbacks: null | SuspenseHydrationCallbacks, }; -// A wrapper callable object around a useEvent callback that throws if the callback is called during -// rendering. The _impl property points to the actual implementation. -export type EventFunctionWrapper< - Args, - Return, - F: (...Array) => Return, -> = { - (): F, - _impl: F, -}; - export type TransitionTracingCallbacks = { onTransitionStart?: (transitionName: string, startTime: number) => void, onTransitionProgress?: ( @@ -390,9 +379,7 @@ export type Dispatcher = { create: () => (() => void) | void, deps: Array | void | null, ): void, - useEvent?: ) => Return>( - callback: F, - ) => EventFunctionWrapper, + useEvent?: ) => Return>(callback: F) => F, useInsertionEffect( create: () => (() => void) | void, deps: Array | void | null, diff --git a/packages/react-reconciler/src/__tests__/useEvent-test.js b/packages/react-reconciler/src/__tests__/useEvent-test.js index e3b0d872f857e..fecf99679d263 100644 --- a/packages/react-reconciler/src/__tests__/useEvent-test.js +++ b/packages/react-reconciler/src/__tests__/useEvent-test.js @@ -557,6 +557,95 @@ describe('useEvent', () => { expect(Scheduler).toHaveYielded(['Effect value: 2', 'Event value: 2']); }); + // @gate enableUseEventHook + it("doesn't provide a stable identity", () => { + function Counter({shouldRender, value}) { + const onClick = useEvent(() => { + Scheduler.unstable_yieldValue( + 'onClick, shouldRender=' + shouldRender + ', value=' + value, + ); + }); + + // onClick doesn't have a stable function identity so this effect will fire on every render. + // In a real app useEvent functions should *not* be passed as a dependency, this is for + // testing purposes only. + useEffect(() => { + onClick(); + }, [onClick]); + + useEffect(() => { + onClick(); + }, [shouldRender]); + + return <>; + } + + ReactNoop.render(); + expect(Scheduler).toFlushAndYield([ + 'onClick, shouldRender=true, value=0', + 'onClick, shouldRender=true, value=0', + ]); + + ReactNoop.render(); + expect(Scheduler).toFlushAndYield(['onClick, shouldRender=true, value=1']); + + ReactNoop.render(); + expect(Scheduler).toFlushAndYield([ + 'onClick, shouldRender=false, value=2', + 'onClick, shouldRender=false, value=2', + ]); + }); + + // @gate enableUseEventHook + it('event handlers always see the latest committed value', async () => { + let committedEventHandler = null; + + function App({value}) { + const event = useEvent(() => { + return 'Value seen by useEvent: ' + value; + }); + + // Set up an effect that registers the event handler with an external + // event system (e.g. addEventListener). + useEffect( + () => { + // Log when the effect fires. In the test below, we'll assert that this + // only happens during initial render, not during updates. + Scheduler.unstable_yieldValue('Commit new event handler'); + committedEventHandler = event; + return () => { + committedEventHandler = null; + }; + }, + // Note that we've intentionally omitted the event from the dependency + // array. But it will still be able to see the latest `value`. This is the + // key feature of useEvent that makes it different from a regular closure. + [], + ); + return 'Latest rendered value ' + value; + } + + // Initial render + const root = ReactNoop.createRoot(); + await act(async () => { + root.render(); + }); + expect(Scheduler).toHaveYielded(['Commit new event handler']); + expect(root).toMatchRenderedOutput('Latest rendered value 1'); + expect(committedEventHandler()).toBe('Value seen by useEvent: 1'); + + // Update + await act(async () => { + root.render(); + }); + // No new event handler should be committed, because it was omitted from + // the dependency array. + expect(Scheduler).toHaveYielded([]); + // But the event handler should still be able to see the latest value. + expect(root).toMatchRenderedOutput('Latest rendered value 2'); + expect(committedEventHandler()).toBe('Value seen by useEvent: 2'); + }); + // @gate enableUseEventHook it('integration: implements docs chat room example', () => { function createConnection() { @@ -597,7 +686,7 @@ describe('useEvent', () => { }); connection.connect(); return () => connection.disconnect(); - }, [roomId, onConnected]); + }, [roomId]); return ; } @@ -676,7 +765,7 @@ describe('useEvent', () => { const onVisit = useEvent(visitedUrl => { Scheduler.unstable_yieldValue( - 'url: ' + url + ', numberOfItems: ' + numberOfItems, + 'url: ' + visitedUrl + ', numberOfItems: ' + numberOfItems, ); }); diff --git a/packages/react-server/src/ReactFizzHooks.js b/packages/react-server/src/ReactFizzHooks.js index c8ceeddc400e4..a9295b3143008 100644 --- a/packages/react-server/src/ReactFizzHooks.js +++ b/packages/react-server/src/ReactFizzHooks.js @@ -7,10 +7,7 @@ * @flow */ -import type { - Dispatcher, - EventFunctionWrapper, -} from 'react-reconciler/src/ReactInternalTypes'; +import type {Dispatcher} from 'react-reconciler/src/ReactInternalTypes'; import type { MutableSource, @@ -522,7 +519,8 @@ function throwOnUseEventCall() { export function useEvent) => Return>( callback: F, -): EventFunctionWrapper { +): F { + // $FlowIgnore[incompatible-return] return throwOnUseEventCall; }