From 899e3d1297ec15a5aa8d73e2f1bd478918090a12 Mon Sep 17 00:00:00 2001 From: lauren Date: Tue, 11 Feb 2025 13:52:25 -0500 Subject: [PATCH 1/4] [crud] Narrow resource type (#32203) 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. --- [//]: # (BEGIN SAPLING FOOTER) Stack created with [Sapling](https://sapling-scm.com). Best reviewed with [ReviewStack](https://reviewstack.dev/facebook/react/pull/32203). * #32206 * #32205 * #32204 * __->__ #32203 --- .../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.'); From 725b262d7f1155248144f3daed25a22e3c1f92a0 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Tue, 11 Feb 2025 13:53:00 -0500 Subject: [PATCH 2/4] [crud] Rename useResourceEffect flag Rename the flag in preparation for the overload. --- .../ReactDOMServerIntegrationHooks-test.js | 2 +- .../src/ReactFiberCallUserSpace.js | 6 ++-- .../src/ReactFiberCommitEffects.js | 16 +++++----- .../react-reconciler/src/ReactFiberHooks.js | 24 +++++++------- .../src/ReactInternalTypes.js | 2 +- .../ReactHooksWithNoopRenderer-test.js | 32 +++++++++---------- packages/react-server/src/ReactFizzHooks.js | 4 +-- packages/react/src/ReactClient.js | 4 +-- packages/react/src/ReactHooks.js | 4 +-- packages/shared/ReactFeatureFlags.js | 2 +- .../ReactFeatureFlags.native-fb-dynamic.js | 2 +- .../forks/ReactFeatureFlags.native-fb.js | 2 +- .../forks/ReactFeatureFlags.native-oss.js | 2 +- .../forks/ReactFeatureFlags.test-renderer.js | 2 +- ...actFeatureFlags.test-renderer.native-fb.js | 2 +- .../ReactFeatureFlags.test-renderer.www.js | 2 +- .../forks/ReactFeatureFlags.www-dynamic.js | 2 +- .../shared/forks/ReactFeatureFlags.www.js | 2 +- 18 files changed, 56 insertions(+), 56 deletions(-) diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js index b79e59ad00..bce830ddf0 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js @@ -657,7 +657,7 @@ describe('ReactDOMServerHooks', () => { describe('useResourceEffect', () => { gate(flags => { - if (flags.enableUseResourceEffectHook) { + if (flags.enableUseEffectCRUDOverload) { const yields = []; itRenders( 'should ignore resource effects on the server', diff --git a/packages/react-reconciler/src/ReactFiberCallUserSpace.js b/packages/react-reconciler/src/ReactFiberCallUserSpace.js index dd7beecd93..25ede645b5 100644 --- a/packages/react-reconciler/src/ReactFiberCallUserSpace.js +++ b/packages/react-reconciler/src/ReactFiberCallUserSpace.js @@ -18,7 +18,7 @@ import { ResourceEffectIdentityKind, ResourceEffectUpdateKind, } from './ReactFiberHooks'; -import {enableUseResourceEffectHook} from 'shared/ReactFeatureFlags'; +import {enableUseEffectCRUDOverload} from 'shared/ReactFeatureFlags'; // These indirections exists so we can exclude its stack frame in DEV (and anything below it). // TODO: Consider marking the whole bundle instead of these boundaries. @@ -184,11 +184,11 @@ const callCreate = { 'react-stack-bottom-frame': function ( effect: Effect, ): (() => void) | {...} | void | null { - if (!enableUseResourceEffectHook) { + if (!enableUseEffectCRUDOverload) { if (effect.resourceKind != null) { if (__DEV__) { console.error( - 'Expected only SimpleEffects when enableUseResourceEffectHook is disabled, ' + + 'Expected only SimpleEffects when enableUseEffectCRUDOverload is disabled, ' + 'got %s', effect.resourceKind, ); diff --git a/packages/react-reconciler/src/ReactFiberCommitEffects.js b/packages/react-reconciler/src/ReactFiberCommitEffects.js index a8e08721ee..29e5de2817 100644 --- a/packages/react-reconciler/src/ReactFiberCommitEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitEffects.js @@ -22,7 +22,7 @@ import { enableProfilerCommitHooks, enableProfilerNestedUpdatePhase, enableSchedulingProfiler, - enableUseResourceEffectHook, + enableUseEffectCRUDOverload, enableViewTransition, } from 'shared/ReactFeatureFlags'; import { @@ -160,7 +160,7 @@ export function commitHookEffectListMount( // Mount let destroy; - if (enableUseResourceEffectHook) { + if (enableUseEffectCRUDOverload) { if (effect.resourceKind === ResourceEffectIdentityKind) { if (__DEV__) { effect.inst.resource = runWithFiberInDEV( @@ -200,7 +200,7 @@ export function commitHookEffectListMount( if ((flags & HookInsertion) !== NoHookEffect) { setIsRunningInsertionEffect(true); } - if (enableUseResourceEffectHook) { + if (enableUseEffectCRUDOverload) { if (effect.resourceKind == null) { destroy = runWithFiberInDEV( finishedWork, @@ -219,7 +219,7 @@ export function commitHookEffectListMount( setIsRunningInsertionEffect(false); } } else { - if (enableUseResourceEffectHook) { + if (enableUseEffectCRUDOverload) { if (effect.resourceKind == null) { const create = effect.create; const inst = effect.inst; @@ -230,7 +230,7 @@ export function commitHookEffectListMount( if (effect.resourceKind != null) { if (__DEV__) { console.error( - 'Expected only SimpleEffects when enableUseResourceEffectHook is disabled, ' + + 'Expected only SimpleEffects when enableUseEffectCRUDOverload is disabled, ' + 'got %s', effect.resourceKind, ); @@ -262,7 +262,7 @@ export function commitHookEffectListMount( } else if ((effect.tag & HookInsertion) !== NoFlags) { hookName = 'useInsertionEffect'; } else if ( - enableUseResourceEffectHook && + enableUseEffectCRUDOverload && effect.resourceKind != null ) { hookName = 'useResourceEffect'; @@ -338,7 +338,7 @@ export function commitHookEffectListUnmount( const inst = effect.inst; const destroy = inst.destroy; if (destroy !== undefined) { - if (enableUseResourceEffectHook) { + if (enableUseEffectCRUDOverload) { if (effect.resourceKind == null) { inst.destroy = undefined; } @@ -358,7 +358,7 @@ export function commitHookEffectListUnmount( setIsRunningInsertionEffect(true); } } - if (enableUseResourceEffectHook) { + if (enableUseEffectCRUDOverload) { if ( effect.resourceKind === ResourceEffectIdentityKind && effect.inst.resource != null diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 9d97710003..87f601dae5 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -38,7 +38,7 @@ import { enableSchedulingProfiler, enableTransitionTracing, enableUseEffectEventHook, - enableUseResourceEffectHook, + enableUseEffectCRUDOverload, enableLegacyCache, disableLegacyMode, enableNoCloningMemoCache, @@ -3938,7 +3938,7 @@ export const ContextOnlyDispatcher: Dispatcher = { if (enableUseEffectEventHook) { (ContextOnlyDispatcher: Dispatcher).useEffectEvent = throwInvalidHookError; } -if (enableUseResourceEffectHook) { +if (enableUseEffectCRUDOverload) { (ContextOnlyDispatcher: Dispatcher).useResourceEffect = throwInvalidHookError; } @@ -3971,7 +3971,7 @@ const HooksDispatcherOnMount: Dispatcher = { if (enableUseEffectEventHook) { (HooksDispatcherOnMount: Dispatcher).useEffectEvent = mountEvent; } -if (enableUseResourceEffectHook) { +if (enableUseEffectCRUDOverload) { (HooksDispatcherOnMount: Dispatcher).useResourceEffect = mountResourceEffect; } @@ -4004,7 +4004,7 @@ const HooksDispatcherOnUpdate: Dispatcher = { if (enableUseEffectEventHook) { (HooksDispatcherOnUpdate: Dispatcher).useEffectEvent = updateEvent; } -if (enableUseResourceEffectHook) { +if (enableUseEffectCRUDOverload) { (HooksDispatcherOnUpdate: Dispatcher).useResourceEffect = updateResourceEffect; } @@ -4038,7 +4038,7 @@ const HooksDispatcherOnRerender: Dispatcher = { if (enableUseEffectEventHook) { (HooksDispatcherOnRerender: Dispatcher).useEffectEvent = updateEvent; } -if (enableUseResourceEffectHook) { +if (enableUseEffectCRUDOverload) { (HooksDispatcherOnRerender: Dispatcher).useResourceEffect = updateResourceEffect; } @@ -4242,7 +4242,7 @@ if (__DEV__) { return mountEvent(callback); }; } - if (enableUseResourceEffectHook) { + if (enableUseEffectCRUDOverload) { (HooksDispatcherOnMountInDEV: Dispatcher).useResourceEffect = function useResourceEffect( create: () => {...} | void | null, @@ -4430,7 +4430,7 @@ if (__DEV__) { return mountEvent(callback); }; } - if (enableUseResourceEffectHook) { + if (enableUseEffectCRUDOverload) { (HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher).useResourceEffect = function useResourceEffect( create: () => {...} | void | null, @@ -4617,7 +4617,7 @@ if (__DEV__) { return updateEvent(callback); }; } - if (enableUseResourceEffectHook) { + if (enableUseEffectCRUDOverload) { (HooksDispatcherOnUpdateInDEV: Dispatcher).useResourceEffect = function useResourceEffect( create: () => {...} | void | null, @@ -4804,7 +4804,7 @@ if (__DEV__) { return updateEvent(callback); }; } - if (enableUseResourceEffectHook) { + if (enableUseEffectCRUDOverload) { (HooksDispatcherOnRerenderInDEV: Dispatcher).useResourceEffect = function useResourceEffect( create: () => {...} | void | null, @@ -5016,7 +5016,7 @@ if (__DEV__) { return mountEvent(callback); }; } - if (enableUseResourceEffectHook) { + if (enableUseEffectCRUDOverload) { (InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher).useResourceEffect = function useResourceEffect( create: () => {...} | void | null, @@ -5229,7 +5229,7 @@ if (__DEV__) { return updateEvent(callback); }; } - if (enableUseResourceEffectHook) { + if (enableUseEffectCRUDOverload) { (InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher).useResourceEffect = function useResourceEffect( create: () => {...} | void | null, @@ -5442,7 +5442,7 @@ if (__DEV__) { return updateEvent(callback); }; } - if (enableUseResourceEffectHook) { + if (enableUseEffectCRUDOverload) { (InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher).useResourceEffect = function useResourceEffect( create: () => {...} | void | null, diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index e31b7cefd1..9ac0680fb6 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -396,7 +396,7 @@ export type Dispatcher = { ): void, // TODO: Non-nullable once `enableUseEffectEventHook` is on everywhere. useEffectEvent?: ) => mixed>(callback: F) => F, - // TODO: Non-nullable once `enableUseResourceEffectHook` is on everywhere. + // TODO: Non-nullable once `enableUseEffectCRUDOverload` is on everywhere. useResourceEffect?: ( create: () => {...} | void | null, createDeps: Array | void | null, diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index e4febdb3e2..27bff1e080 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -3311,7 +3311,7 @@ describe('ReactHooksWithNoopRenderer', () => { }); }); - // @gate enableUseResourceEffectHook + // @gate enableUseEffectCRUDOverload describe('useResourceEffect', () => { class Resource { isDeleted: false; @@ -3333,12 +3333,12 @@ describe('ReactHooksWithNoopRenderer', () => { } } - // @gate !enableUseResourceEffectHook + // @gate !enableUseEffectCRUDOverload it('is null when flag is disabled', async () => { expect(useResourceEffect).toBeUndefined(); }); - // @gate enableUseResourceEffectHook + // @gate enableUseEffectCRUDOverload it('validates create return value', async () => { function App({id}) { useResourceEffect(() => { @@ -3359,7 +3359,7 @@ describe('ReactHooksWithNoopRenderer', () => { ); }); - // @gate enableUseResourceEffectHook + // @gate enableUseEffectCRUDOverload it('validates non-empty update deps', async () => { function App({id}) { useResourceEffect( @@ -3386,7 +3386,7 @@ describe('ReactHooksWithNoopRenderer', () => { ]); }); - // @gate enableUseResourceEffectHook + // @gate enableUseEffectCRUDOverload it('simple mount and update', async () => { function App({id, username}) { const opts = useMemo(() => { @@ -3443,7 +3443,7 @@ describe('ReactHooksWithNoopRenderer', () => { assertLog(['destroy(2, Jack)']); }); - // @gate enableUseResourceEffectHook + // @gate enableUseEffectCRUDOverload it('simple mount with no update', async () => { function App({id, username}) { const opts = useMemo(() => { @@ -3480,7 +3480,7 @@ describe('ReactHooksWithNoopRenderer', () => { assertLog(['destroy(1, Jack)']); }); - // @gate enableUseResourceEffectHook + // @gate enableUseEffectCRUDOverload it('calls update on every render if no deps are specified', async () => { function App({id, username}) { const opts = useMemo(() => { @@ -3523,7 +3523,7 @@ describe('ReactHooksWithNoopRenderer', () => { assertLog(['update(2, Lauren)']); }); - // @gate enableUseResourceEffectHook + // @gate enableUseEffectCRUDOverload it('does not unmount previous useResourceEffect between updates', async () => { function App({id}) { useResourceEffect( @@ -3562,7 +3562,7 @@ describe('ReactHooksWithNoopRenderer', () => { assertLog(['update(0)']); }); - // @gate enableUseResourceEffectHook + // @gate enableUseEffectCRUDOverload it('unmounts only on deletion', async () => { function App({id}) { useResourceEffect( @@ -3596,7 +3596,7 @@ describe('ReactHooksWithNoopRenderer', () => { expect(ReactNoop).toMatchRenderedOutput(null); }); - // @gate enableUseResourceEffectHook + // @gate enableUseEffectCRUDOverload it('unmounts on deletion', async () => { function Wrapper(props) { return ; @@ -3650,7 +3650,7 @@ describe('ReactHooksWithNoopRenderer', () => { expect(ReactNoop).toMatchRenderedOutput(null); }); - // @gate enableUseResourceEffectHook + // @gate enableUseEffectCRUDOverload it('handles errors in create on mount', async () => { function App({id}) { useResourceEffect( @@ -3700,7 +3700,7 @@ describe('ReactHooksWithNoopRenderer', () => { expect(ReactNoop).toMatchRenderedOutput(null); }); - // @gate enableUseResourceEffectHook + // @gate enableUseEffectCRUDOverload it('handles errors in create on update', async () => { function App({id}) { useResourceEffect( @@ -3744,7 +3744,7 @@ describe('ReactHooksWithNoopRenderer', () => { }).rejects.toThrow('Oops error!'); }); - // @gate enableUseResourceEffectHook + // @gate enableUseEffectCRUDOverload it('handles errors in destroy on update', async () => { function App({id, username}) { const opts = useMemo(() => { @@ -3800,7 +3800,7 @@ describe('ReactHooksWithNoopRenderer', () => { expect(ReactNoop).toMatchRenderedOutput(null); }); - // @gate enableUseResourceEffectHook && enableActivity + // @gate enableUseEffectCRUDOverload && enableActivity it('composes with activity', async () => { function App({id, username}) { const opts = useMemo(() => { @@ -3873,7 +3873,7 @@ describe('ReactHooksWithNoopRenderer', () => { assertLog(['destroy(0, Lauren)']); }); - // @gate enableUseResourceEffectHook + // @gate enableUseEffectCRUDOverload it('composes with suspense', async () => { function TextBox({text}) { return ; @@ -3991,7 +3991,7 @@ describe('ReactHooksWithNoopRenderer', () => { ); }); - // @gate enableUseResourceEffectHook + // @gate enableUseEffectCRUDOverload it('composes with other kinds of effects', async () => { let rerender; function App({id, username}) { diff --git a/packages/react-server/src/ReactFizzHooks.js b/packages/react-server/src/ReactFizzHooks.js index 0db0b00b3b..63e8576ca7 100644 --- a/packages/react-server/src/ReactFizzHooks.js +++ b/packages/react-server/src/ReactFizzHooks.js @@ -40,7 +40,7 @@ import {createFastHash} from './ReactServerStreamConfig'; import { enableUseEffectEventHook, - enableUseResourceEffectHook, + enableUseEffectCRUDOverload, } from 'shared/ReactFeatureFlags'; import is from 'shared/objectIs'; import { @@ -866,7 +866,7 @@ export const HooksDispatcher: Dispatcher = supportsClientAPIs if (enableUseEffectEventHook) { HooksDispatcher.useEffectEvent = useEffectEvent; } -if (enableUseResourceEffectHook) { +if (enableUseEffectCRUDOverload) { HooksDispatcher.useResourceEffect = supportsClientAPIs ? noop : clientHookNotSupported; diff --git a/packages/react/src/ReactClient.js b/packages/react/src/ReactClient.js index f633c7617d..2bacbc8567 100644 --- a/packages/react/src/ReactClient.js +++ b/packages/react/src/ReactClient.js @@ -65,7 +65,7 @@ import {addTransitionType} from './ReactTransitionType'; import {act} from './ReactAct'; import {captureOwnerStack} from './ReactOwnerStack'; import * as ReactCompilerRuntime from './ReactCompilerRuntime'; -import {enableUseResourceEffectHook} from 'shared/ReactFeatureFlags'; +import {enableUseEffectCRUDOverload} from 'shared/ReactFeatureFlags'; const Children = { map, @@ -134,4 +134,4 @@ export { }; export const experimental_useResourceEffect: typeof useResourceEffect | void = - enableUseResourceEffectHook ? useResourceEffect : undefined; + enableUseEffectCRUDOverload ? useResourceEffect : undefined; diff --git a/packages/react/src/ReactHooks.js b/packages/react/src/ReactHooks.js index ff45b5415c..677e0d705b 100644 --- a/packages/react/src/ReactHooks.js +++ b/packages/react/src/ReactHooks.js @@ -18,7 +18,7 @@ import {REACT_CONSUMER_TYPE} from 'shared/ReactSymbols'; import ReactSharedInternals from 'shared/ReactSharedInternals'; -import {enableUseResourceEffectHook} from 'shared/ReactFeatureFlags'; +import {enableUseEffectCRUDOverload} from 'shared/ReactFeatureFlags'; type BasicStateAction = (S => S) | S; type Dispatch = A => void; @@ -208,7 +208,7 @@ export function useResourceEffect( updateDeps: Array | void | null, destroy: ((resource: {...} | void | null) => void) | void, ): void { - if (!enableUseResourceEffectHook) { + if (!enableUseEffectCRUDOverload) { throw new Error('Not implemented.'); } const dispatcher = resolveDispatcher(); diff --git a/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 290d5ba159..1cb1cb4cf7 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -150,7 +150,7 @@ export const enableInfiniteRenderLoopDetection = false; /** * Experimental new hook for better managing resources in effects. */ -export const enableUseResourceEffectHook = false; +export const enableUseEffectCRUDOverload = false; export const enableFastAddPropertiesInDiffing = true; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js index 6aaefdfa33..001b3d64c0 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb-dynamic.js @@ -24,7 +24,7 @@ export const enablePersistedModeClonedFlag = __VARIANT__; export const enableShallowPropDiffing = __VARIANT__; export const passChildrenWhenCloningPersistedNodes = __VARIANT__; export const enableSiblingPrerendering = __VARIANT__; -export const enableUseResourceEffectHook = __VARIANT__; +export const enableUseEffectCRUDOverload = __VARIANT__; export const enableOwnerStacks = __VARIANT__; export const enableRemoveConsolePatches = __VARIANT__; export const enableFastAddPropertiesInDiffing = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index 878d344a42..6e82185b5d 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -24,7 +24,7 @@ export const { enableObjectFiber, enablePersistedModeClonedFlag, enableShallowPropDiffing, - enableUseResourceEffectHook, + enableUseEffectCRUDOverload, passChildrenWhenCloningPersistedNodes, enableSiblingPrerendering, enableOwnerStacks, diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index a3202c0b80..3f373e998c 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -63,7 +63,7 @@ export const retryLaneExpirationMs = 5000; export const syncLaneExpirationMs = 250; export const transitionLaneExpirationMs = 5000; export const enableSiblingPrerendering = true; -export const enableUseResourceEffectHook = false; +export const enableUseEffectCRUDOverload = false; export const enableHydrationLaneScheduling = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index 6bc085f540..da9b8a2612 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -64,7 +64,7 @@ export const renameElementSymbol = true; export const enableShallowPropDiffing = false; export const enableSiblingPrerendering = true; -export const enableUseResourceEffectHook = false; +export const enableUseEffectCRUDOverload = false; export const enableYieldingBeforePassive = true; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index d62a59122f..8c20cc28de 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -62,7 +62,7 @@ export const retryLaneExpirationMs = 5000; export const syncLaneExpirationMs = 250; export const transitionLaneExpirationMs = 5000; export const enableSiblingPrerendering = true; -export const enableUseResourceEffectHook = true; +export const enableUseEffectCRUDOverload = true; export const enableHydrationLaneScheduling = true; export const enableYieldingBeforePassive = false; export const enableThrottledScheduling = false; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 301c01a018..bc30992eb6 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -74,7 +74,7 @@ export const enableOwnerStacks = false; export const enableShallowPropDiffing = false; export const enableSiblingPrerendering = true; -export const enableUseResourceEffectHook = false; +export const enableUseEffectCRUDOverload = false; export const enableHydrationLaneScheduling = true; diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index e6fd46d0b1..3cab1318c4 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -36,7 +36,7 @@ export const enableSchedulingProfiler = __VARIANT__; export const enableInfiniteRenderLoopDetection = __VARIANT__; export const enableSiblingPrerendering = __VARIANT__; -export const enableUseResourceEffectHook = __VARIANT__; +export const enableUseEffectCRUDOverload = __VARIANT__; export const enableRemoveConsolePatches = __VARIANT__; export const enableFastAddPropertiesInDiffing = __VARIANT__; export const enableViewTransition = __VARIANT__; diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index dcb77f9d84..57be83577c 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -29,7 +29,7 @@ export const { enableSiblingPrerendering, enableTransitionTracing, enableTrustedTypesIntegration, - enableUseResourceEffectHook, + enableUseEffectCRUDOverload, favorSafetyOverHydrationPerf, renameElementSymbol, retryLaneExpirationMs, From eea70a101905ad1f2d7547f0e56d6e8cc2cf80b8 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Tue, 11 Feb 2025 13:53:00 -0500 Subject: [PATCH 3/4] [crud] Merge useResourceEffect into useEffect Merges the useResourceEffect API into useEffect while keeping the underlying implementation the same. useResourceEffect will be removed in the next diff. To fork between behavior we rely on a `typeof` check for the updater or destroy function in addition to the CRUD feature flag. This does now have to be checked every time (instead of inlined statically like before due to them being different hooks) which will incur some non-zero amount (possibly negligble) of overhead for every effect. --- .../react-debug-tools/src/ReactDebugHooks.js | 7 +- .../ReactDOMServerIntegrationHooks-test.js | 6 +- .../src/ReactFiberCallUserSpace.js | 2 +- .../src/ReactFiberCommitEffects.js | 41 +-- .../react-reconciler/src/ReactFiberHooks.js | 268 ++++++++++++++---- .../src/ReactInternalTypes.js | 7 +- .../ReactHooksWithNoopRenderer-test.js | 70 +++-- packages/react/src/ReactHooks.js | 26 +- scripts/error-codes/codes.json | 3 +- 9 files changed, 296 insertions(+), 134 deletions(-) diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index 9c310723b2..f45ccfecdc 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -373,8 +373,11 @@ function useInsertionEffect( } function useEffect( - create: () => (() => void) | void, - inputs: Array | void | null, + create: (() => (() => void) | void) | (() => {...} | void | null), + createDeps: Array | void | null, + update?: ((resource: {...} | void | null) => void) | void, + updateDeps?: Array | void | null, + destroy?: ((resource: {...} | void | null) => void) | void, ): void { nextHook(); hookLog.push({ diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js index bce830ddf0..840d6c5b15 100644 --- a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js @@ -27,7 +27,6 @@ let useRef; let useImperativeHandle; let useInsertionEffect; let useLayoutEffect; -let useResourceEffect; let useDebugValue; let forwardRef; let yieldedValues; @@ -52,7 +51,6 @@ function initModules() { useImperativeHandle = React.useImperativeHandle; useInsertionEffect = React.useInsertionEffect; useLayoutEffect = React.useLayoutEffect; - useResourceEffect = React.experimental_useResourceEffect; forwardRef = React.forwardRef; yieldedValues = []; @@ -655,7 +653,7 @@ describe('ReactDOMServerHooks', () => { }); }); - describe('useResourceEffect', () => { + describe('useEffect with CRUD overload', () => { gate(flags => { if (flags.enableUseEffectCRUDOverload) { const yields = []; @@ -663,7 +661,7 @@ describe('ReactDOMServerHooks', () => { 'should ignore resource effects on the server', async render => { function Counter(props) { - useResourceEffect( + useEffect( () => { yieldValue('created on client'); return {resource_counter: props.count}; diff --git a/packages/react-reconciler/src/ReactFiberCallUserSpace.js b/packages/react-reconciler/src/ReactFiberCallUserSpace.js index 25ede645b5..a9b5590f38 100644 --- a/packages/react-reconciler/src/ReactFiberCallUserSpace.js +++ b/packages/react-reconciler/src/ReactFiberCallUserSpace.js @@ -254,7 +254,7 @@ const callDestroy = { export const callDestroyInDEV: ( current: Fiber, nearestMountedAncestor: Fiber | null, - destroy: () => void, + destroy: (() => void) | (({...}) => void), ) => void = __DEV__ ? // We use this technique to trick minifiers to preserve the function name. (callDestroy['react-stack-bottom-frame'].bind(callDestroy): any) diff --git a/packages/react-reconciler/src/ReactFiberCommitEffects.js b/packages/react-reconciler/src/ReactFiberCommitEffects.js index 29e5de2817..05cf17a342 100644 --- a/packages/react-reconciler/src/ReactFiberCommitEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitEffects.js @@ -170,8 +170,9 @@ export function commitHookEffectListMount( ); if (effect.inst.resource == null) { console.error( - 'useResourceEffect must provide a callback which returns a resource. ' + - 'If a managed resource is not needed here, use useEffect. Received %s', + 'useEffect must provide a callback which returns a resource. ' + + 'If a managed resource is not needed here, do not provide an updater or ' + + 'destroy callback. Received %s', effect.inst.resource, ); } @@ -261,11 +262,6 @@ export function commitHookEffectListMount( hookName = 'useLayoutEffect'; } else if ((effect.tag & HookInsertion) !== NoFlags) { hookName = 'useInsertionEffect'; - } else if ( - enableUseEffectCRUDOverload && - effect.resourceKind != null - ) { - hookName = 'useResourceEffect'; } else { hookName = 'useEffect'; } @@ -363,7 +359,7 @@ export function commitHookEffectListUnmount( effect.resourceKind === ResourceEffectIdentityKind && effect.inst.resource != null ) { - safelyCallDestroyWithResource( + safelyCallDestroy( finishedWork, nearestMountedAncestor, destroy, @@ -1015,32 +1011,11 @@ export function safelyDetachRef( function safelyCallDestroy( current: Fiber, nearestMountedAncestor: Fiber | null, - destroy: () => void, + destroy: (() => void) | (({...}) => void), + resource?: {...} | void | null, ) { - if (__DEV__) { - runWithFiberInDEV( - current, - callDestroyInDEV, - current, - nearestMountedAncestor, - destroy, - ); - } else { - try { - destroy(); - } catch (error) { - captureCommitPhaseError(current, nearestMountedAncestor, error); - } - } -} - -function safelyCallDestroyWithResource( - current: Fiber, - nearestMountedAncestor: Fiber | null, - destroy: ({...}) => void, - resource: {...}, -) { - const destroy_ = destroy.bind(null, resource); + // $FlowFixMe[extra-arg] @poteto this is safe either way because the extra arg is ignored if it's not a CRUD effect + const destroy_ = resource == null ? destroy : destroy.bind(null, resource); if (__DEV__) { runWithFiberInDEV( current, diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 87f601dae5..976657d4c6 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -2523,12 +2523,15 @@ function pushSimpleEffect( tag: HookFlags, inst: EffectInstance, create: () => (() => void) | void, - deps: Array | void | null, + createDeps: Array | void | null, + update?: ((resource: {...} | void | null) => void) | void, + updateDeps?: Array | void | null, + destroy?: ((resource: {...} | void | null) => void) | void, ): Effect { const effect: Effect = { tag, create, - deps, + deps: createDeps, inst, // Circular next: (null: any), @@ -2608,10 +2611,13 @@ function mountEffectImpl( fiberFlags: Flags, hookFlags: HookFlags, create: () => (() => void) | void, - deps: Array | void | null, + createDeps: Array | void | null, + update?: ((resource: {...} | void | null) => void) | void, + updateDeps?: Array | void | null, + destroy?: ((resource: {...} | void | null) => void) | void, ): void { const hook = mountWorkInProgressHook(); - const nextDeps = deps === undefined ? null : deps; + const nextDeps = createDeps === undefined ? null : createDeps; currentlyRenderingFiber.flags |= fiberFlags; hook.memoizedState = pushSimpleEffect( HookHasEffect | hookFlags, @@ -2662,35 +2668,89 @@ function updateEffectImpl( } function mountEffect( - create: () => (() => void) | void, - deps: Array | void | null, + create: (() => (() => void) | void) | (() => {...} | void | null), + createDeps: Array | void | null, + update?: ((resource: {...} | void | null) => void) | void, + updateDeps?: Array | void | null, + destroy?: ((resource: {...} | void | null) => void) | void, ): void { if ( __DEV__ && (currentlyRenderingFiber.mode & StrictEffectsMode) !== NoMode && (currentlyRenderingFiber.mode & NoStrictPassiveEffectsMode) === NoMode ) { - mountEffectImpl( - MountPassiveDevEffect | PassiveEffect | PassiveStaticEffect, - HookPassive, - create, - deps, - ); + if ( + enableUseEffectCRUDOverload && + (typeof update === 'function' || typeof destroy === 'function') + ) { + mountResourceEffectImpl( + MountPassiveDevEffect | PassiveEffect | PassiveStaticEffect, + HookPassive, + create, + createDeps, + update, + updateDeps, + destroy, + ); + } else { + mountEffectImpl( + MountPassiveDevEffect | PassiveEffect | PassiveStaticEffect, + HookPassive, + // $FlowFixMe[incompatible-call] @poteto it's not possible to narrow `create` without calling it. + create, + createDeps, + ); + } } else { - mountEffectImpl( - PassiveEffect | PassiveStaticEffect, - HookPassive, - create, - deps, - ); + if ( + enableUseEffectCRUDOverload && + (typeof update === 'function' || typeof destroy === 'function') + ) { + mountResourceEffectImpl( + PassiveEffect | PassiveStaticEffect, + HookPassive, + create, + createDeps, + update, + updateDeps, + destroy, + ); + } else { + mountEffectImpl( + PassiveEffect | PassiveStaticEffect, + HookPassive, + // $FlowFixMe[incompatible-call] @poteto it's not possible to narrow `create` without calling it. + create, + createDeps, + ); + } } } function updateEffect( - create: () => (() => void) | void, - deps: Array | void | null, + create: (() => (() => void) | void) | (() => {...} | void | null), + createDeps: Array | void | null, + update?: ((resource: {...} | void | null) => void) | void, + updateDeps?: Array | void | null, + destroy?: ((resource: {...} | void | null) => void) | void, ): void { - updateEffectImpl(PassiveEffect, HookPassive, create, deps); + if ( + enableUseEffectCRUDOverload && + (typeof update === 'function' || typeof destroy === 'function') + ) { + updateResourceEffectImpl( + PassiveEffect, + HookPassive, + create, + createDeps, + update, + updateDeps, + destroy, + ); + } else { + // $FlowFixMe[incompatible-call] @poteto it's not possible to narrow `create` without calling it. + updateEffectImpl(PassiveEffect, HookPassive, create, createDeps); + } } function mountResourceEffect( @@ -2705,15 +2765,6 @@ function mountResourceEffect( (currentlyRenderingFiber.mode & StrictEffectsMode) !== NoMode && (currentlyRenderingFiber.mode & NoStrictPassiveEffectsMode) === NoMode ) { - mountResourceEffectImpl( - MountPassiveDevEffect | PassiveEffect | PassiveStaticEffect, - HookPassive, - create, - createDeps, - update, - updateDeps, - destroy, - ); } else { mountResourceEffectImpl( PassiveEffect | PassiveStaticEffect, @@ -4087,13 +4138,30 @@ if (__DEV__) { return readContext(context); }, useEffect( - create: () => (() => void) | void, - deps: Array | void | null, + create: (() => (() => void) | void) | (() => {...} | void | null), + createDeps: Array | void | null, + update?: ((resource: {...} | void | null) => void) | void, + updateDeps?: Array | void | null, + destroy?: ((resource: {...} | void | null) => void) | void, ): void { currentHookNameInDev = 'useEffect'; mountHookTypesDev(); - checkDepsAreArrayDev(deps); - return mountEffect(create, deps); + if ( + enableUseEffectCRUDOverload && + (typeof update === 'function' || typeof destroy === 'function') + ) { + checkDepsAreNonEmptyArrayDev(updateDeps); + return mountResourceEffect( + create, + createDeps, + update, + updateDeps, + destroy, + ); + } else { + checkDepsAreArrayDev(createDeps); + return mountEffect(create, createDeps); + } }, useImperativeHandle( ref: {current: T | null} | ((inst: T | null) => mixed) | null | void, @@ -4280,12 +4348,28 @@ if (__DEV__) { return readContext(context); }, useEffect( - create: () => (() => void) | void, - deps: Array | void | null, + create: (() => (() => void) | void) | (() => {...} | void | null), + createDeps: Array | void | null, + update?: ((resource: {...} | void | null) => void) | void, + updateDeps?: Array | void | null, + destroy?: ((resource: {...} | void | null) => void) | void, ): void { currentHookNameInDev = 'useEffect'; updateHookTypesDev(); - return mountEffect(create, deps); + if ( + enableUseEffectCRUDOverload && + (typeof update === 'function' || typeof destroy === 'function') + ) { + return mountResourceEffect( + create, + createDeps, + update, + updateDeps, + destroy, + ); + } else { + return mountEffect(create, createDeps); + } }, useImperativeHandle( ref: {current: T | null} | ((inst: T | null) => mixed) | null | void, @@ -4467,12 +4551,28 @@ if (__DEV__) { return readContext(context); }, useEffect( - create: () => (() => void) | void, - deps: Array | void | null, + create: (() => (() => void) | void) | (() => {...} | void | null), + createDeps: Array | void | null, + update?: ((resource: {...} | void | null) => void) | void, + updateDeps?: Array | void | null, + destroy?: ((resource: {...} | void | null) => void) | void, ): void { currentHookNameInDev = 'useEffect'; updateHookTypesDev(); - return updateEffect(create, deps); + if ( + enableUseEffectCRUDOverload && + (typeof update === 'function' || typeof destroy === 'function') + ) { + return updateResourceEffect( + create, + createDeps, + update, + updateDeps, + destroy, + ); + } else { + return updateEffect(create, createDeps); + } }, useImperativeHandle( ref: {current: T | null} | ((inst: T | null) => mixed) | null | void, @@ -4654,12 +4754,28 @@ if (__DEV__) { return readContext(context); }, useEffect( - create: () => (() => void) | void, - deps: Array | void | null, + create: (() => (() => void) | void) | (() => {...} | void | null), + createDeps: Array | void | null, + update?: ((resource: {...} | void | null) => void) | void, + updateDeps?: Array | void | null, + destroy?: ((resource: {...} | void | null) => void) | void, ): void { currentHookNameInDev = 'useEffect'; updateHookTypesDev(); - return updateEffect(create, deps); + if ( + enableUseEffectCRUDOverload && + (typeof update === 'function' || typeof destroy === 'function') + ) { + return updateResourceEffect( + create, + createDeps, + update, + updateDeps, + destroy, + ); + } else { + return updateEffect(create, createDeps); + } }, useImperativeHandle( ref: {current: T | null} | ((inst: T | null) => mixed) | null | void, @@ -4847,13 +4963,29 @@ if (__DEV__) { return readContext(context); }, useEffect( - create: () => (() => void) | void, - deps: Array | void | null, + create: (() => (() => void) | void) | (() => {...} | void | null), + createDeps: Array | void | null, + update?: ((resource: {...} | void | null) => void) | void, + updateDeps?: Array | void | null, + destroy?: ((resource: {...} | void | null) => void) | void, ): void { currentHookNameInDev = 'useEffect'; warnInvalidHookAccess(); mountHookTypesDev(); - return mountEffect(create, deps); + if ( + enableUseEffectCRUDOverload && + (typeof update === 'function' || typeof destroy === 'function') + ) { + return mountResourceEffect( + create, + createDeps, + update, + updateDeps, + destroy, + ); + } else { + return mountEffect(create, createDeps); + } }, useImperativeHandle( ref: {current: T | null} | ((inst: T | null) => mixed) | null | void, @@ -5060,13 +5192,29 @@ if (__DEV__) { return readContext(context); }, useEffect( - create: () => (() => void) | void, - deps: Array | void | null, + create: (() => (() => void) | void) | (() => {...} | void | null), + createDeps: Array | void | null, + update?: ((resource: {...} | void | null) => void) | void, + updateDeps?: Array | void | null, + destroy?: ((resource: {...} | void | null) => void) | void, ): void { currentHookNameInDev = 'useEffect'; warnInvalidHookAccess(); updateHookTypesDev(); - return updateEffect(create, deps); + if ( + enableUseEffectCRUDOverload && + (typeof update === 'function' || typeof destroy === 'function') + ) { + return updateResourceEffect( + create, + createDeps, + update, + updateDeps, + destroy, + ); + } else { + return updateEffect(create, createDeps); + } }, useImperativeHandle( ref: {current: T | null} | ((inst: T | null) => mixed) | null | void, @@ -5273,13 +5421,29 @@ if (__DEV__) { return readContext(context); }, useEffect( - create: () => (() => void) | void, - deps: Array | void | null, + create: (() => (() => void) | void) | (() => {...} | void | null), + createDeps: Array | void | null, + update?: ((resource: {...} | void | null) => void) | void, + updateDeps?: Array | void | null, + destroy?: ((resource: {...} | void | null) => void) | void, ): void { currentHookNameInDev = 'useEffect'; warnInvalidHookAccess(); updateHookTypesDev(); - return updateEffect(create, deps); + if ( + enableUseEffectCRUDOverload && + (typeof update === 'function' || typeof destroy === 'function') + ) { + return updateResourceEffect( + create, + createDeps, + update, + updateDeps, + destroy, + ); + } else { + return updateEffect(create, createDeps); + } }, useImperativeHandle( ref: {current: T | null} | ((inst: T | null) => mixed) | null | void, diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index 9ac0680fb6..756d88c4a5 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -391,8 +391,11 @@ export type Dispatcher = { useContext(context: ReactContext): T, useRef(initialValue: T): {current: T}, useEffect( - create: () => (() => void) | void, - deps: Array | void | null, + create: (() => (() => void) | void) | (() => {...} | void | null), + createDeps: Array | void | null, + update?: ((resource: {...} | void | null) => void) | void, + updateDeps?: Array | void | null, + destroy?: ((resource: {...} | void | null) => void) | void, ): void, // TODO: Non-nullable once `enableUseEffectEventHook` is on everywhere. useEffectEvent?: ) => mixed>(callback: F) => F, diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index 27bff1e080..47fd534e68 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -41,7 +41,6 @@ let waitFor; let waitForThrow; let waitForPaint; let assertLog; -let useResourceEffect; let assertConsoleErrorDev; describe('ReactHooksWithNoopRenderer', () => { @@ -70,7 +69,6 @@ describe('ReactHooksWithNoopRenderer', () => { useDeferredValue = React.useDeferredValue; Suspense = React.Suspense; Activity = React.unstable_Activity; - useResourceEffect = React.experimental_useResourceEffect; ContinuousEventPriority = require('react-reconciler/constants').ContinuousEventPriority; if (gate(flags => flags.enableSuspenseList)) { @@ -3312,7 +3310,7 @@ describe('ReactHooksWithNoopRenderer', () => { }); // @gate enableUseEffectCRUDOverload - describe('useResourceEffect', () => { + describe('useEffect CRUD overload', () => { class Resource { isDeleted: false; id: string; @@ -3335,34 +3333,34 @@ describe('ReactHooksWithNoopRenderer', () => { // @gate !enableUseEffectCRUDOverload it('is null when flag is disabled', async () => { - expect(useResourceEffect).toBeUndefined(); - }); - - // @gate enableUseEffectCRUDOverload - it('validates create return value', async () => { function App({id}) { - useResourceEffect(() => { - Scheduler.log(`create(${id})`); - }, [id]); + useEffect( + () => { + Scheduler.log(`create(${id})`); + return {}; + }, + [id], + () => { + Scheduler.log('update'); + }, + [], + ); return null; } - await act(() => { - ReactNoop.render(); - }); - assertConsoleErrorDev( - [ - 'useResourceEffect must provide a callback which returns a resource. ' + - 'If a managed resource is not needed here, use useEffect. Received undefined', - ], - {withoutStack: true}, + await expect(async () => { + await act(() => { + ReactNoop.render(); + }); + }).rejects.toThrow( + 'useEffect CRUD overload is not enabled in this build of React.', ); }); // @gate enableUseEffectCRUDOverload it('validates non-empty update deps', async () => { function App({id}) { - useResourceEffect( + useEffect( () => { Scheduler.log(`create(${id})`); return {}; @@ -3380,7 +3378,7 @@ describe('ReactHooksWithNoopRenderer', () => { ReactNoop.render(); }); assertConsoleErrorDev([ - 'useResourceEffect received a dependency array with no dependencies. ' + + 'useEffect received a dependency array with no dependencies. ' + 'When specified, the dependency array must have at least one dependency.\n' + ' in App (at **)', ]); @@ -3392,7 +3390,7 @@ describe('ReactHooksWithNoopRenderer', () => { const opts = useMemo(() => { return {username}; }, [username]); - useResourceEffect( + useEffect( () => { const resource = new Resource(id, opts); Scheduler.log(`create(${resource.id}, ${resource.opts.username})`); @@ -3449,7 +3447,7 @@ describe('ReactHooksWithNoopRenderer', () => { const opts = useMemo(() => { return {username}; }, [username]); - useResourceEffect( + useEffect( () => { const resource = new Resource(id, opts); Scheduler.log(`create(${resource.id}, ${resource.opts.username})`); @@ -3486,7 +3484,7 @@ describe('ReactHooksWithNoopRenderer', () => { const opts = useMemo(() => { return {username}; }, [username]); - useResourceEffect( + useEffect( () => { const resource = new Resource(id, opts); Scheduler.log(`create(${resource.id}, ${resource.opts.username})`); @@ -3524,9 +3522,9 @@ describe('ReactHooksWithNoopRenderer', () => { }); // @gate enableUseEffectCRUDOverload - it('does not unmount previous useResourceEffect between updates', async () => { + it('does not unmount previous useEffect between updates', async () => { function App({id}) { - useResourceEffect( + useEffect( () => { const resource = new Resource(id); Scheduler.log(`create(${resource.id})`); @@ -3565,7 +3563,7 @@ describe('ReactHooksWithNoopRenderer', () => { // @gate enableUseEffectCRUDOverload it('unmounts only on deletion', async () => { function App({id}) { - useResourceEffect( + useEffect( () => { const resource = new Resource(id); Scheduler.log(`create(${resource.id})`); @@ -3605,7 +3603,7 @@ describe('ReactHooksWithNoopRenderer', () => { const opts = useMemo(() => { return {username}; }, [username]); - useResourceEffect( + useEffect( () => { const resource = new Resource(id, opts); Scheduler.log(`create(${resource.id}, ${resource.opts.username})`); @@ -3653,7 +3651,7 @@ describe('ReactHooksWithNoopRenderer', () => { // @gate enableUseEffectCRUDOverload it('handles errors in create on mount', async () => { function App({id}) { - useResourceEffect( + useEffect( () => { Scheduler.log(`Mount A [${id}]`); return {}; @@ -3665,7 +3663,7 @@ describe('ReactHooksWithNoopRenderer', () => { Scheduler.log(`Unmount A [${id}]`); }, ); - useResourceEffect( + useEffect( () => { Scheduler.log('Oops!'); throw new Error('Oops!'); @@ -3703,7 +3701,7 @@ describe('ReactHooksWithNoopRenderer', () => { // @gate enableUseEffectCRUDOverload it('handles errors in create on update', async () => { function App({id}) { - useResourceEffect( + useEffect( () => { Scheduler.log(`Mount A [${id}]`); return {}; @@ -3750,7 +3748,7 @@ describe('ReactHooksWithNoopRenderer', () => { const opts = useMemo(() => { return {username}; }, [username]); - useResourceEffect( + useEffect( () => { const resource = new Resource(id, opts); Scheduler.log(`Mount A [${id}, ${resource.opts.username}]`); @@ -3806,7 +3804,7 @@ describe('ReactHooksWithNoopRenderer', () => { const opts = useMemo(() => { return {username}; }, [username]); - useResourceEffect( + useEffect( () => { const resource = new Resource(id, opts); Scheduler.log(`create(${resource.id}, ${resource.opts.username})`); @@ -3885,7 +3883,7 @@ describe('ReactHooksWithNoopRenderer', () => { const opts = useMemo(() => { return {username}; }, [username]); - useResourceEffect( + useEffect( () => { const resource = new Resource(id, opts); Scheduler.log(`create(${resource.id}, ${resource.opts.username})`); @@ -4003,7 +4001,7 @@ describe('ReactHooksWithNoopRenderer', () => { useEffect(() => { Scheduler.log(`useEffect(${count})`); }, [count]); - useResourceEffect( + useEffect( () => { const resource = new Resource(id, opts); Scheduler.log(`create(${resource.id}, ${resource.opts.username})`); diff --git a/packages/react/src/ReactHooks.js b/packages/react/src/ReactHooks.js index 677e0d705b..2fe6d2b85b 100644 --- a/packages/react/src/ReactHooks.js +++ b/packages/react/src/ReactHooks.js @@ -87,11 +87,31 @@ export function useRef(initialValue: T): {current: T} { } export function useEffect( - create: () => (() => void) | void, - deps: Array | void | null, + create: (() => (() => void) | void) | (() => {...} | void | null), + createDeps: Array | void | null, + update?: ((resource: {...} | void | null) => void) | void, + updateDeps?: Array | void | null, + destroy?: ((resource: {...} | void | null) => void) | void, ): void { const dispatcher = resolveDispatcher(); - return dispatcher.useEffect(create, deps); + if ( + enableUseEffectCRUDOverload && + (typeof update === 'function' || typeof destroy === 'function') + ) { + // $FlowFixMe[not-a-function] This is unstable, thus optional + return dispatcher.useEffect( + create, + createDeps, + update, + updateDeps, + destroy, + ); + } else if (typeof update === 'function') { + throw new Error( + 'useEffect CRUD overload is not enabled in this build of React.', + ); + } + return dispatcher.useEffect(create, createDeps); } export function useInsertionEffect( diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 8fa3d190ec..6ab654f1d3 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -530,5 +530,6 @@ "542": "Suspense Exception: This is not a real error! It's an implementation detail of `useActionState` to interrupt the current render. You must either rethrow it immediately, or move the `useActionState` call outside of the `try/catch` block. Capturing without rethrowing will lead to unexpected behavior.\n\nTo handle async errors, wrap your component in an error boundary.", "543": "Expected a ResourceEffectUpdate to be pushed together with ResourceEffectIdentity. This is a bug in React.", "544": "Found a pair with an auto name. This is a bug in React.", - "545": "The %s tag may only be rendered once." + "545": "The %s tag may only be rendered once.", + "546": "useEffect CRUD overload is not enabled in this build of React." } From 7ea1abc12f8eb2dc4f15788a1adced39d9321398 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Tue, 11 Feb 2025 13:53:00 -0500 Subject: [PATCH 4/4] [crud] Remove useResourceEffect Removes useResourceEffect. --- .../react-debug-tools/src/ReactDebugHooks.js | 22 --- .../react-reconciler/src/ReactFiberHooks.js | 158 ------------------ .../src/ReactInternalTypes.js | 9 - packages/react-server/src/ReactFizzHooks.js | 10 +- packages/react/index.development.js | 1 - .../react/index.experimental.development.js | 1 - packages/react/index.fb.js | 1 - packages/react/src/ReactClient.js | 5 - packages/react/src/ReactHooks.js | 21 --- 9 files changed, 1 insertion(+), 227 deletions(-) diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index f45ccfecdc..114080d03e 100644 --- a/packages/react-debug-tools/src/ReactDebugHooks.js +++ b/packages/react-debug-tools/src/ReactDebugHooks.js @@ -128,9 +128,6 @@ function getPrimitiveStackCache(): Map> { Dispatcher.useId(); - if (typeof Dispatcher.useResourceEffect === 'function') { - Dispatcher.useResourceEffect(() => ({}), []); - } if (typeof Dispatcher.useEffectEvent === 'function') { Dispatcher.useEffectEvent((args: empty) => {}); } @@ -741,24 +738,6 @@ function useHostTransitionStatus(): TransitionStatus { return status; } -function useResourceEffect( - create: () => {...} | void | null, - createDeps: Array | void | null, - update: ((resource: {...} | void | null) => void) | void, - updateDeps: Array | void | null, - destroy: ((resource: {...} | void | null) => void) | void, -) { - nextHook(); - hookLog.push({ - displayName: null, - primitive: 'ResourceEffect', - stackError: new Error(), - value: create, - debugInfo: null, - dispatcherHookName: 'ResourceEffect', - }); -} - function useEffectEvent) => mixed>(callback: F): F { nextHook(); hookLog.push({ @@ -798,7 +777,6 @@ const Dispatcher: DispatcherType = { useActionState, useHostTransitionStatus, useEffectEvent, - useResourceEffect, }; // create a proxy to throw a custom error diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 976657d4c6..2ab41770bd 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -3989,9 +3989,6 @@ export const ContextOnlyDispatcher: Dispatcher = { if (enableUseEffectEventHook) { (ContextOnlyDispatcher: Dispatcher).useEffectEvent = throwInvalidHookError; } -if (enableUseEffectCRUDOverload) { - (ContextOnlyDispatcher: Dispatcher).useResourceEffect = throwInvalidHookError; -} const HooksDispatcherOnMount: Dispatcher = { readContext, @@ -4022,9 +4019,6 @@ const HooksDispatcherOnMount: Dispatcher = { if (enableUseEffectEventHook) { (HooksDispatcherOnMount: Dispatcher).useEffectEvent = mountEvent; } -if (enableUseEffectCRUDOverload) { - (HooksDispatcherOnMount: Dispatcher).useResourceEffect = mountResourceEffect; -} const HooksDispatcherOnUpdate: Dispatcher = { readContext, @@ -4055,10 +4049,6 @@ const HooksDispatcherOnUpdate: Dispatcher = { if (enableUseEffectEventHook) { (HooksDispatcherOnUpdate: Dispatcher).useEffectEvent = updateEvent; } -if (enableUseEffectCRUDOverload) { - (HooksDispatcherOnUpdate: Dispatcher).useResourceEffect = - updateResourceEffect; -} const HooksDispatcherOnRerender: Dispatcher = { readContext, @@ -4089,10 +4079,6 @@ const HooksDispatcherOnRerender: Dispatcher = { if (enableUseEffectEventHook) { (HooksDispatcherOnRerender: Dispatcher).useEffectEvent = updateEvent; } -if (enableUseEffectCRUDOverload) { - (HooksDispatcherOnRerender: Dispatcher).useResourceEffect = - updateResourceEffect; -} let HooksDispatcherOnMountInDEV: Dispatcher | null = null; let HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher | null = null; @@ -4310,27 +4296,6 @@ if (__DEV__) { return mountEvent(callback); }; } - if (enableUseEffectCRUDOverload) { - (HooksDispatcherOnMountInDEV: Dispatcher).useResourceEffect = - function useResourceEffect( - create: () => {...} | void | null, - createDeps: Array | void | null, - update: ((resource: {...} | void | null) => void) | void, - updateDeps: Array | void | null, - destroy: ((resource: {...} | void | null) => void) | void, - ): void { - currentHookNameInDev = 'useResourceEffect'; - mountHookTypesDev(); - checkDepsAreNonEmptyArrayDev(updateDeps); - return mountResourceEffect( - create, - createDeps, - update, - updateDeps, - destroy, - ); - }; - } HooksDispatcherOnMountWithHookTypesInDEV = { readContext(context: ReactContext): T { @@ -4514,26 +4479,6 @@ if (__DEV__) { return mountEvent(callback); }; } - if (enableUseEffectCRUDOverload) { - (HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher).useResourceEffect = - function useResourceEffect( - create: () => {...} | void | null, - createDeps: Array | void | null, - update: ((resource: {...} | void | null) => void) | void, - updateDeps: Array | void | null, - destroy: ((resource: {...} | void | null) => void) | void, - ): void { - currentHookNameInDev = 'useResourceEffect'; - updateHookTypesDev(); - return mountResourceEffect( - create, - createDeps, - update, - updateDeps, - destroy, - ); - }; - } HooksDispatcherOnUpdateInDEV = { readContext(context: ReactContext): T { @@ -4717,26 +4662,6 @@ if (__DEV__) { return updateEvent(callback); }; } - if (enableUseEffectCRUDOverload) { - (HooksDispatcherOnUpdateInDEV: Dispatcher).useResourceEffect = - function useResourceEffect( - create: () => {...} | void | null, - createDeps: Array | void | null, - update: ((resource: {...} | void | null) => void) | void, - updateDeps: Array | void | null, - destroy: ((resource: {...} | void | null) => void) | void, - ) { - currentHookNameInDev = 'useResourceEffect'; - updateHookTypesDev(); - return updateResourceEffect( - create, - createDeps, - update, - updateDeps, - destroy, - ); - }; - } HooksDispatcherOnRerenderInDEV = { readContext(context: ReactContext): T { @@ -4920,26 +4845,6 @@ if (__DEV__) { return updateEvent(callback); }; } - if (enableUseEffectCRUDOverload) { - (HooksDispatcherOnRerenderInDEV: Dispatcher).useResourceEffect = - function useResourceEffect( - create: () => {...} | void | null, - createDeps: Array | void | null, - update: ((resource: {...} | void | null) => void) | void, - updateDeps: Array | void | null, - destroy: ((resource: {...} | void | null) => void) | void, - ) { - currentHookNameInDev = 'useResourceEffect'; - updateHookTypesDev(); - return updateResourceEffect( - create, - createDeps, - update, - updateDeps, - destroy, - ); - }; - } InvalidNestedHooksDispatcherOnMountInDEV = { readContext(context: ReactContext): T { @@ -5148,27 +5053,6 @@ if (__DEV__) { return mountEvent(callback); }; } - if (enableUseEffectCRUDOverload) { - (InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher).useResourceEffect = - function useResourceEffect( - create: () => {...} | void | null, - createDeps: Array | void | null, - update: ((resource: {...} | void | null) => void) | void, - updateDeps: Array | void | null, - destroy: ((resource: {...} | void | null) => void) | void, - ): void { - currentHookNameInDev = 'useResourceEffect'; - warnInvalidHookAccess(); - mountHookTypesDev(); - return mountResourceEffect( - create, - createDeps, - update, - updateDeps, - destroy, - ); - }; - } InvalidNestedHooksDispatcherOnUpdateInDEV = { readContext(context: ReactContext): T { @@ -5377,27 +5261,6 @@ if (__DEV__) { return updateEvent(callback); }; } - if (enableUseEffectCRUDOverload) { - (InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher).useResourceEffect = - function useResourceEffect( - create: () => {...} | void | null, - createDeps: Array | void | null, - update: ((resource: {...} | void | null) => void) | void, - updateDeps: Array | void | null, - destroy: ((resource: {...} | void | null) => void) | void, - ) { - currentHookNameInDev = 'useResourceEffect'; - warnInvalidHookAccess(); - updateHookTypesDev(); - return updateResourceEffect( - create, - createDeps, - update, - updateDeps, - destroy, - ); - }; - } InvalidNestedHooksDispatcherOnRerenderInDEV = { readContext(context: ReactContext): T { @@ -5606,25 +5469,4 @@ if (__DEV__) { return updateEvent(callback); }; } - if (enableUseEffectCRUDOverload) { - (InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher).useResourceEffect = - function useResourceEffect( - create: () => {...} | void | null, - createDeps: Array | void | null, - update: ((resource: {...} | void | null) => void) | void, - updateDeps: Array | void | null, - destroy: ((resource: {...} | void | null) => void) | void, - ) { - currentHookNameInDev = 'useResourceEffect'; - warnInvalidHookAccess(); - updateHookTypesDev(); - return updateResourceEffect( - create, - createDeps, - update, - updateDeps, - destroy, - ); - }; - } } diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index 756d88c4a5..98e7d4deef 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -47,7 +47,6 @@ export type HookType = | 'useRef' | 'useEffect' | 'useEffectEvent' - | 'useResourceEffect' | 'useInsertionEffect' | 'useLayoutEffect' | 'useCallback' @@ -399,14 +398,6 @@ export type Dispatcher = { ): void, // TODO: Non-nullable once `enableUseEffectEventHook` is on everywhere. useEffectEvent?: ) => mixed>(callback: F) => F, - // TODO: Non-nullable once `enableUseEffectCRUDOverload` is on everywhere. - useResourceEffect?: ( - create: () => {...} | void | null, - createDeps: Array | void | null, - update: ((resource: {...} | void | null) => void) | void, - updateDeps: Array | void | null, - destroy: ((resource: {...} | void | null) => void) | void, - ) => void, useInsertionEffect( create: () => (() => void) | void, deps: Array | void | null, diff --git a/packages/react-server/src/ReactFizzHooks.js b/packages/react-server/src/ReactFizzHooks.js index 63e8576ca7..a0ec1c7414 100644 --- a/packages/react-server/src/ReactFizzHooks.js +++ b/packages/react-server/src/ReactFizzHooks.js @@ -38,10 +38,7 @@ import { } from './ReactFizzConfig'; import {createFastHash} from './ReactServerStreamConfig'; -import { - enableUseEffectEventHook, - enableUseEffectCRUDOverload, -} from 'shared/ReactFeatureFlags'; +import {enableUseEffectEventHook} from 'shared/ReactFeatureFlags'; import is from 'shared/objectIs'; import { REACT_CONTEXT_TYPE, @@ -866,11 +863,6 @@ export const HooksDispatcher: Dispatcher = supportsClientAPIs if (enableUseEffectEventHook) { HooksDispatcher.useEffectEvent = useEffectEvent; } -if (enableUseEffectCRUDOverload) { - HooksDispatcher.useResourceEffect = supportsClientAPIs - ? noop - : clientHookNotSupported; -} export let currentResumableState: null | ResumableState = (null: any); export function setCurrentResumableState( diff --git a/packages/react/index.development.js b/packages/react/index.development.js index 809e940f07..fa79633001 100644 --- a/packages/react/index.development.js +++ b/packages/react/index.development.js @@ -59,7 +59,6 @@ export { useDeferredValue, useEffect, experimental_useEffectEvent, - experimental_useResourceEffect, useImperativeHandle, useInsertionEffect, useLayoutEffect, diff --git a/packages/react/index.experimental.development.js b/packages/react/index.experimental.development.js index 49c98bb208..6074b683b7 100644 --- a/packages/react/index.experimental.development.js +++ b/packages/react/index.experimental.development.js @@ -42,7 +42,6 @@ export { useDeferredValue, useEffect, experimental_useEffectEvent, - experimental_useResourceEffect, useImperativeHandle, useInsertionEffect, useLayoutEffect, diff --git a/packages/react/index.fb.js b/packages/react/index.fb.js index 8e11bade4d..29133df24f 100644 --- a/packages/react/index.fb.js +++ b/packages/react/index.fb.js @@ -21,7 +21,6 @@ export { createElement, createRef, experimental_useEffectEvent, - experimental_useResourceEffect, forwardRef, Fragment, isValidElement, diff --git a/packages/react/src/ReactClient.js b/packages/react/src/ReactClient.js index 2bacbc8567..715ea8ab47 100644 --- a/packages/react/src/ReactClient.js +++ b/packages/react/src/ReactClient.js @@ -41,7 +41,6 @@ import { useContext, useEffect, useEffectEvent, - useResourceEffect, useImperativeHandle, useDebugValue, useInsertionEffect, @@ -65,7 +64,6 @@ import {addTransitionType} from './ReactTransitionType'; import {act} from './ReactAct'; import {captureOwnerStack} from './ReactOwnerStack'; import * as ReactCompilerRuntime from './ReactCompilerRuntime'; -import {enableUseEffectCRUDOverload} from 'shared/ReactFeatureFlags'; const Children = { map, @@ -132,6 +130,3 @@ export { act, // DEV-only captureOwnerStack, // DEV-only }; - -export const experimental_useResourceEffect: typeof useResourceEffect | void = - enableUseEffectCRUDOverload ? useResourceEffect : undefined; diff --git a/packages/react/src/ReactHooks.js b/packages/react/src/ReactHooks.js index 2fe6d2b85b..06d61d2238 100644 --- a/packages/react/src/ReactHooks.js +++ b/packages/react/src/ReactHooks.js @@ -221,27 +221,6 @@ export function useEffectEvent) => mixed>( return dispatcher.useEffectEvent(callback); } -export function useResourceEffect( - create: () => {...} | void | null, - createDeps: Array | void | null, - update: ((resource: {...} | void | null) => void) | void, - updateDeps: Array | void | null, - destroy: ((resource: {...} | void | null) => void) | void, -): void { - if (!enableUseEffectCRUDOverload) { - throw new Error('Not implemented.'); - } - const dispatcher = resolveDispatcher(); - // $FlowFixMe[not-a-function] This is unstable, thus optional - return dispatcher.useResourceEffect( - create, - createDeps, - update, - updateDeps, - destroy, - ); -} - export function useOptimistic( passthrough: S, reducer: ?(S, A) => S,