diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index 9c310723b26..114080d03e9 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) => {}); } @@ -373,8 +370,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({ @@ -738,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({ @@ -795,7 +777,6 @@ const Dispatcher: DispatcherType = { useActionState, useHostTransitionStatus, useEffectEvent, - useResourceEffect, }; // create a proxy to throw a custom error diff --git a/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js b/packages/react-dom/src/__tests__/ReactDOMServerIntegrationHooks-test.js index bce830ddf06..840d6c5b15d 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 25ede645b5e..a9b5590f387 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 29e5de28172..05cf17a3427 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, -) { - 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: {...}, + destroy: (() => void) | (({...}) => void), + resource?: {...} | void | null, ) { - 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 87f601dae59..2ab41770bd0 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, @@ -3938,9 +3989,6 @@ export const ContextOnlyDispatcher: Dispatcher = { if (enableUseEffectEventHook) { (ContextOnlyDispatcher: Dispatcher).useEffectEvent = throwInvalidHookError; } -if (enableUseEffectCRUDOverload) { - (ContextOnlyDispatcher: Dispatcher).useResourceEffect = throwInvalidHookError; -} const HooksDispatcherOnMount: Dispatcher = { readContext, @@ -3971,9 +4019,6 @@ const HooksDispatcherOnMount: Dispatcher = { if (enableUseEffectEventHook) { (HooksDispatcherOnMount: Dispatcher).useEffectEvent = mountEvent; } -if (enableUseEffectCRUDOverload) { - (HooksDispatcherOnMount: Dispatcher).useResourceEffect = mountResourceEffect; -} const HooksDispatcherOnUpdate: Dispatcher = { readContext, @@ -4004,10 +4049,6 @@ const HooksDispatcherOnUpdate: Dispatcher = { if (enableUseEffectEventHook) { (HooksDispatcherOnUpdate: Dispatcher).useEffectEvent = updateEvent; } -if (enableUseEffectCRUDOverload) { - (HooksDispatcherOnUpdate: Dispatcher).useResourceEffect = - updateResourceEffect; -} const HooksDispatcherOnRerender: Dispatcher = { readContext, @@ -4038,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; @@ -4087,13 +4124,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, @@ -4242,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 { @@ -4280,12 +4313,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, @@ -4430,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 { @@ -4467,12 +4496,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, @@ -4617,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 { @@ -4654,12 +4679,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, @@ -4804,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 { @@ -4847,13 +4868,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, @@ -5016,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 { @@ -5060,13 +5076,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, @@ -5229,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 { @@ -5273,13 +5284,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, @@ -5442,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 9ac0680fb62..98e7d4deefe 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' @@ -391,19 +390,14 @@ 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, - // 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-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index 27bff1e0806..47fd534e683 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-server/src/ReactFizzHooks.js b/packages/react-server/src/ReactFizzHooks.js index 63e8576ca78..a0ec1c74149 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 809e940f070..fa79633001a 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 49c98bb208a..6074b683b78 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 8e11bade4df..29133df24f3 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 2bacbc85676..715ea8ab47c 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 677e0d705b8..06d61d22387 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( @@ -201,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, diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 8fa3d190ecb..6ab654f1d35 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." }