From 39be94b1454d3a1da9e2e18bca2b3ca83366cfc5 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Mon, 3 Feb 2025 11:54:25 -0500 Subject: [PATCH] [crud] Narrow resource type Small refactor to the `resource` type to narrow it to an arbitrary object or void/null instead of the top type. This makes the overload on useEffect simpler since the return type of create is no longer widened to the top type when we merge their definitions. --- .../react-debug-tools/src/ReactDebugHooks.js | 6 +- .../src/ReactFiberCallUserSpace.js | 2 +- .../src/ReactFiberCommitEffects.js | 8 +- .../react-reconciler/src/ReactFiberHooks.js | 78 +++++++++---------- .../src/ReactInternalTypes.js | 6 +- packages/react/src/ReactHooks.js | 6 +- 6 files changed, 54 insertions(+), 52 deletions(-) diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index f30489b7f655f..9c310723b2605 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -739,11 +739,11 @@ function useHostTransitionStatus(): TransitionStatus { } function useResourceEffect( - create: () => mixed, + create: () => {...} | void | null, createDeps: Array | void | null, - update: ((resource: mixed) => void) | void, + update: ((resource: {...} | void | null) => void) | void, updateDeps: Array | void | null, - destroy: ((resource: mixed) => void) | void, + destroy: ((resource: {...} | void | null) => void) | void, ) { nextHook(); hookLog.push({ diff --git a/packages/react-reconciler/src/ReactFiberCallUserSpace.js b/packages/react-reconciler/src/ReactFiberCallUserSpace.js index a1b86c7560a2a..dd7beecd93582 100644 --- a/packages/react-reconciler/src/ReactFiberCallUserSpace.js +++ b/packages/react-reconciler/src/ReactFiberCallUserSpace.js @@ -183,7 +183,7 @@ export const callComponentWillUnmountInDEV: ( const callCreate = { 'react-stack-bottom-frame': function ( effect: Effect, - ): (() => void) | mixed | void { + ): (() => void) | {...} | void | null { if (!enableUseResourceEffectHook) { if (effect.resourceKind != null) { if (__DEV__) { diff --git a/packages/react-reconciler/src/ReactFiberCommitEffects.js b/packages/react-reconciler/src/ReactFiberCommitEffects.js index 6873bdf9e7a63..a8e08721eed5f 100644 --- a/packages/react-reconciler/src/ReactFiberCommitEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitEffects.js @@ -274,6 +274,7 @@ export function commitHookEffectListMount( addendum = ' You returned null. If your effect does not require clean ' + 'up, return undefined (or nothing).'; + // $FlowFixMe (@poteto) this check is safe on arbitrary non-null/void objects } else if (typeof destroy.then === 'function') { addendum = '\n\nIt looks like you wrote ' + @@ -1036,10 +1037,10 @@ function safelyCallDestroy( function safelyCallDestroyWithResource( current: Fiber, nearestMountedAncestor: Fiber | null, - destroy: mixed => void, - resource: mixed, + destroy: ({...}) => void, + resource: {...}, ) { - const destroy_ = resource == null ? destroy : destroy.bind(null, resource); + const destroy_ = destroy.bind(null, resource); if (__DEV__) { runWithFiberInDEV( current, @@ -1050,6 +1051,7 @@ function safelyCallDestroyWithResource( ); } else { try { + // $FlowFixMe(incompatible-call) Already bound to resource destroy_(); } catch (error) { captureCommitPhaseError(current, nearestMountedAncestor, error); diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index f3a581a97476a..9d97710003317 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -205,8 +205,8 @@ export type Hook = { // the additional memory and we can follow up with performance // optimizations later. type EffectInstance = { - resource: mixed, - destroy: void | (() => void) | ((resource: mixed) => void), + resource: {...} | void | null, + destroy: void | (() => void) | ((resource: {...} | void | null) => void), }; export const ResourceEffectIdentityKind: 0 = 0; @@ -229,7 +229,7 @@ export type ResourceEffectIdentity = { resourceKind: typeof ResourceEffectIdentityKind, tag: HookFlags, inst: EffectInstance, - create: () => mixed, + create: () => {...} | void | null, deps: Array | void | null, next: Effect, }; @@ -237,7 +237,7 @@ export type ResourceEffectUpdate = { resourceKind: typeof ResourceEffectUpdateKind, tag: HookFlags, inst: EffectInstance, - update: ((resource: mixed) => void) | void, + update: ((resource: {...} | void | null) => void) | void, deps: Array | void | null, next: Effect, identity: ResourceEffectIdentity, @@ -2540,9 +2540,9 @@ function pushResourceEffect( identityTag: HookFlags, updateTag: HookFlags, inst: EffectInstance, - create: () => mixed, + create: () => {...} | void | null, createDeps: Array | void | null, - update: ((resource: mixed) => void) | void, + update: ((resource: {...} | void | null) => void) | void, updateDeps: Array | void | null, ): Effect { const effectIdentity: ResourceEffectIdentity = { @@ -2694,11 +2694,11 @@ function updateEffect( } function mountResourceEffect( - create: () => mixed, + create: () => {...} | void | null, createDeps: Array | void | null, - update: ((resource: mixed) => void) | void, + update: ((resource: {...} | void | null) => void) | void, updateDeps: Array | void | null, - destroy: ((resource: mixed) => void) | void, + destroy: ((resource: {...} | void | null) => void) | void, ) { if ( __DEV__ && @@ -2730,11 +2730,11 @@ function mountResourceEffect( function mountResourceEffectImpl( fiberFlags: Flags, hookFlags: HookFlags, - create: () => mixed, + create: () => {...} | void | null, createDeps: Array | void | null, - update: ((resource: mixed) => void) | void, + update: ((resource: {...} | void | null) => void) | void, updateDeps: Array | void | null, - destroy: ((resource: mixed) => void) | void, + destroy: ((resource: {...} | void | null) => void) | void, ) { const hook = mountWorkInProgressHook(); currentlyRenderingFiber.flags |= fiberFlags; @@ -2752,11 +2752,11 @@ function mountResourceEffectImpl( } function updateResourceEffect( - create: () => mixed, + create: () => {...} | void | null, createDeps: Array | void | null, - update: ((resource: mixed) => void) | void, + update: ((resource: {...} | void | null) => void) | void, updateDeps: Array | void | null, - destroy: ((resource: mixed) => void) | void, + destroy: ((resource: {...} | void | null) => void) | void, ) { updateResourceEffectImpl( PassiveEffect, @@ -2772,11 +2772,11 @@ function updateResourceEffect( function updateResourceEffectImpl( fiberFlags: Flags, hookFlags: HookFlags, - create: () => mixed, + create: () => {...} | void | null, createDeps: Array | void | null, - update: ((resource: mixed) => void) | void, + update: ((resource: {...} | void | null) => void) | void, updateDeps: Array | void | null, - destroy: ((resource: mixed) => void) | void, + destroy: ((resource: {...} | void | null) => void) | void, ) { const hook = updateWorkInProgressHook(); const effect: Effect = hook.memoizedState; @@ -4245,11 +4245,11 @@ if (__DEV__) { if (enableUseResourceEffectHook) { (HooksDispatcherOnMountInDEV: Dispatcher).useResourceEffect = function useResourceEffect( - create: () => mixed, + create: () => {...} | void | null, createDeps: Array | void | null, - update: ((resource: mixed) => void) | void, + update: ((resource: {...} | void | null) => void) | void, updateDeps: Array | void | null, - destroy: ((resource: mixed) => void) | void, + destroy: ((resource: {...} | void | null) => void) | void, ): void { currentHookNameInDev = 'useResourceEffect'; mountHookTypesDev(); @@ -4433,11 +4433,11 @@ if (__DEV__) { if (enableUseResourceEffectHook) { (HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher).useResourceEffect = function useResourceEffect( - create: () => mixed, + create: () => {...} | void | null, createDeps: Array | void | null, - update: ((resource: mixed) => void) | void, + update: ((resource: {...} | void | null) => void) | void, updateDeps: Array | void | null, - destroy: ((resource: mixed) => void) | void, + destroy: ((resource: {...} | void | null) => void) | void, ): void { currentHookNameInDev = 'useResourceEffect'; updateHookTypesDev(); @@ -4620,11 +4620,11 @@ if (__DEV__) { if (enableUseResourceEffectHook) { (HooksDispatcherOnUpdateInDEV: Dispatcher).useResourceEffect = function useResourceEffect( - create: () => mixed, + create: () => {...} | void | null, createDeps: Array | void | null, - update: ((resource: mixed) => void) | void, + update: ((resource: {...} | void | null) => void) | void, updateDeps: Array | void | null, - destroy: ((resource: mixed) => void) | void, + destroy: ((resource: {...} | void | null) => void) | void, ) { currentHookNameInDev = 'useResourceEffect'; updateHookTypesDev(); @@ -4807,11 +4807,11 @@ if (__DEV__) { if (enableUseResourceEffectHook) { (HooksDispatcherOnRerenderInDEV: Dispatcher).useResourceEffect = function useResourceEffect( - create: () => mixed, + create: () => {...} | void | null, createDeps: Array | void | null, - update: ((resource: mixed) => void) | void, + update: ((resource: {...} | void | null) => void) | void, updateDeps: Array | void | null, - destroy: ((resource: mixed) => void) | void, + destroy: ((resource: {...} | void | null) => void) | void, ) { currentHookNameInDev = 'useResourceEffect'; updateHookTypesDev(); @@ -5019,11 +5019,11 @@ if (__DEV__) { if (enableUseResourceEffectHook) { (InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher).useResourceEffect = function useResourceEffect( - create: () => mixed, + create: () => {...} | void | null, createDeps: Array | void | null, - update: ((resource: mixed) => void) | void, + update: ((resource: {...} | void | null) => void) | void, updateDeps: Array | void | null, - destroy: ((resource: mixed) => void) | void, + destroy: ((resource: {...} | void | null) => void) | void, ): void { currentHookNameInDev = 'useResourceEffect'; warnInvalidHookAccess(); @@ -5232,11 +5232,11 @@ if (__DEV__) { if (enableUseResourceEffectHook) { (InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher).useResourceEffect = function useResourceEffect( - create: () => mixed, + create: () => {...} | void | null, createDeps: Array | void | null, - update: ((resource: mixed) => void) | void, + update: ((resource: {...} | void | null) => void) | void, updateDeps: Array | void | null, - destroy: ((resource: mixed) => void) | void, + destroy: ((resource: {...} | void | null) => void) | void, ) { currentHookNameInDev = 'useResourceEffect'; warnInvalidHookAccess(); @@ -5445,11 +5445,11 @@ if (__DEV__) { if (enableUseResourceEffectHook) { (InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher).useResourceEffect = function useResourceEffect( - create: () => mixed, + create: () => {...} | void | null, createDeps: Array | void | null, - update: ((resource: mixed) => void) | void, + update: ((resource: {...} | void | null) => void) | void, updateDeps: Array | void | null, - destroy: ((resource: mixed) => void) | void, + destroy: ((resource: {...} | void | null) => void) | void, ) { currentHookNameInDev = 'useResourceEffect'; warnInvalidHookAccess(); diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index 0c5504cab468e..e31b7cefd1ed7 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -398,11 +398,11 @@ export type Dispatcher = { useEffectEvent?: ) => mixed>(callback: F) => F, // TODO: Non-nullable once `enableUseResourceEffectHook` is on everywhere. useResourceEffect?: ( - create: () => mixed, + create: () => {...} | void | null, createDeps: Array | void | null, - update: ((resource: mixed) => void) | void, + update: ((resource: {...} | void | null) => void) | void, updateDeps: Array | void | null, - destroy: ((resource: mixed) => void) | void, + destroy: ((resource: {...} | void | null) => void) | void, ) => void, useInsertionEffect( create: () => (() => void) | void, diff --git a/packages/react/src/ReactHooks.js b/packages/react/src/ReactHooks.js index 32f49268880f0..ff45b5415c416 100644 --- a/packages/react/src/ReactHooks.js +++ b/packages/react/src/ReactHooks.js @@ -202,11 +202,11 @@ export function useEffectEvent) => mixed>( } export function useResourceEffect( - create: () => mixed, + create: () => {...} | void | null, createDeps: Array | void | null, - update: ((resource: mixed) => void) | void, + update: ((resource: {...} | void | null) => void) | void, updateDeps: Array | void | null, - destroy: ((resource: mixed) => void) | void, + destroy: ((resource: {...} | void | null) => void) | void, ): void { if (!enableUseResourceEffectHook) { throw new Error('Not implemented.');