From 5028b63c2e1e4eba74337bfcd234f2e8ef06cee3 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Tue, 11 Feb 2025 13:33:36 -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 f30489b7f6..9c310723b2 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 a1b86c7560..dd7beecd93 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 6873bdf9e7..a8e08721ee 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 f3a581a974..9d97710003 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 0c5504cab4..e31b7cefd1 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 32f4926888..ff45b5415c 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.');