From 601b4ea9328f81867026e93b1d505a4f8a130c04 Mon Sep 17 00:00:00 2001 From: Lauren Tan Date: Thu, 30 Jan 2025 16:27:02 -0500 Subject: [PATCH] [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 | 62 ++-- packages/react/src/ReactHooks.js | 26 +- scripts/error-codes/codes.json | 3 +- 9 files changed, 279 insertions(+), 143 deletions(-) diff --git a/packages/react-debug-tools/src/ReactDebugHooks.js b/packages/react-debug-tools/src/ReactDebugHooks.js index 9c310723b2605..f45ccfecdcaec 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 bce830ddf0647..840d6c5b15d5e 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 25ede645b5e7f..a9b5590f387b7 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 29e5de2817201..05cf17a34273e 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 87f601dae597d..976657d4c68f7 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 9ac0680fb6279..756d88c4a5d15 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 27bff1e0806c4..4fd0cba0b31df 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; @@ -3333,36 +3331,10 @@ 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]); - 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}, - ); - }); - // @gate enableUseEffectCRUDOverload it('validates non-empty update deps', async () => { function App({id}) { - useResourceEffect( + useEffect( () => { Scheduler.log(`create(${id})`); return {}; @@ -3380,7 +3352,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 +3364,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 +3421,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 +3458,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 +3496,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 +3537,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 +3577,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 +3625,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 +3637,7 @@ describe('ReactHooksWithNoopRenderer', () => { Scheduler.log(`Unmount A [${id}]`); }, ); - useResourceEffect( + useEffect( () => { Scheduler.log('Oops!'); throw new Error('Oops!'); @@ -3703,7 +3675,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 +3722,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 +3778,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 +3857,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 +3975,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 677e0d705b8aa..2fe6d2b85bcf1 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 8fa3d190ecb70..6ab654f1d35e5 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." }