From 0e9d7631d65ef3afa776f7e2ff630d349bcd8ea4 Mon Sep 17 00:00:00 2001 From: lauren Date: Mon, 11 Nov 2024 10:54:28 -0500 Subject: [PATCH] [crud] Basic implementation This PR introduces a new experimental hook `useResourceEffect`, which is something that we're doing some very early initial tests on. This may likely not pan out and will be removed or modified if so. Please do not rely on it as it will break. --- .../src/ReactFiberCallUserSpace.js | 15 +- .../src/ReactFiberCommitEffects.js | 76 ++- .../react-reconciler/src/ReactFiberHooks.js | 391 +++++++++++++- .../src/ReactInternalTypes.js | 8 + .../ReactHooksWithNoopRenderer-test.js | 486 ++++++++++++++++++ packages/react/index.development.js | 1 + .../react/index.experimental.development.js | 1 + packages/react/index.fb.js | 1 + packages/react/src/ReactClient.js | 2 + packages/react/src/ReactHooks.js | 18 + packages/shared/ReactFeatureFlags.js | 5 + .../forks/ReactFeatureFlags.native-fb.js | 1 + .../forks/ReactFeatureFlags.native-oss.js | 1 + .../forks/ReactFeatureFlags.test-renderer.js | 2 + ...actFeatureFlags.test-renderer.native-fb.js | 1 + .../ReactFeatureFlags.test-renderer.www.js | 2 + .../forks/ReactFeatureFlags.www-dynamic.js | 2 + .../shared/forks/ReactFeatureFlags.www.js | 1 + 18 files changed, 986 insertions(+), 28 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCallUserSpace.js b/packages/react-reconciler/src/ReactFiberCallUserSpace.js index ada092438a..7f599dfd1e 100644 --- a/packages/react-reconciler/src/ReactFiberCallUserSpace.js +++ b/packages/react-reconciler/src/ReactFiberCallUserSpace.js @@ -14,6 +14,7 @@ import type {CapturedValue} from './ReactCapturedValue'; import {isRendering, setIsRendering} from './ReactCurrentFiber'; import {captureCommitPhaseError} from './ReactFiberWorkLoop'; +import {enableUseResourceEffectHook} 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. @@ -177,11 +178,15 @@ export const callComponentWillUnmountInDEV: ( const callCreate = { 'react-stack-bottom-frame': function (effect: Effect): (() => void) | void { - const create = effect.create; - const inst = effect.inst; - const destroy = create(); - inst.destroy = destroy; - return destroy; + if (!enableUseResourceEffectHook) { + const create = effect.create; + const inst = effect.inst; + const destroy = create(); + // $FlowFixMe[incompatible-type] (@poteto) + inst.destroy = destroy; + // $FlowFixMe[incompatible-return] (@poteto) + return destroy; + } }, }; diff --git a/packages/react-reconciler/src/ReactFiberCommitEffects.js b/packages/react-reconciler/src/ReactFiberCommitEffects.js index 70c49bc62f..023dae18a3 100644 --- a/packages/react-reconciler/src/ReactFiberCommitEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitEffects.js @@ -18,6 +18,7 @@ import { enableProfilerNestedUpdatePhase, enableSchedulingProfiler, enableScopeAPI, + enableUseResourceEffectHook, } from 'shared/ReactFeatureFlags'; import { ClassComponent, @@ -70,6 +71,7 @@ import { } from './ReactFiberCallUserSpace'; import {runWithFiberInDEV} from './ReactCurrentFiber'; +import {ResourceEffectKind, SimpleEffectKind} from './ReactFiberHooks'; function shouldProfile(current: Fiber): boolean { return ( @@ -146,19 +148,51 @@ export function commitHookEffectListMount( // Mount let destroy; + if ( + enableUseResourceEffectHook && + effect.kind === ResourceEffectKind + ) { + if (typeof effect.create === 'function') { + effect.resource = effect.create(); + if (__DEV__) { + if (effect.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', + effect.resource, + ); + } + } + } else if ( + typeof effect.update === 'function' && + effect.resource != null + ) { + // TODO(@poteto) what about multiple updates? + effect.update(effect.resource); + } + destroy = effect.destroy; + } if (__DEV__) { if ((flags & HookInsertion) !== NoHookEffect) { setIsRunningInsertionEffect(true); } - destroy = runWithFiberInDEV(finishedWork, callCreateInDEV, effect); + if (effect.kind === SimpleEffectKind) { + destroy = runWithFiberInDEV( + finishedWork, + callCreateInDEV, + effect, + ); + } if ((flags & HookInsertion) !== NoHookEffect) { setIsRunningInsertionEffect(false); } } else { - const create = effect.create; - const inst = effect.inst; - destroy = create(); - inst.destroy = destroy; + if (effect.kind === SimpleEffectKind) { + const create = effect.create; + const inst = effect.inst; + destroy = create(); + inst.destroy = destroy; + } } if (enableSchedulingProfiler) { @@ -176,6 +210,11 @@ export function commitHookEffectListMount( hookName = 'useLayoutEffect'; } else if ((effect.tag & HookInsertion) !== NoFlags) { hookName = 'useInsertionEffect'; + } else if ( + enableUseResourceEffectHook && + effect.kind === ResourceEffectKind + ) { + hookName = 'useResourceEffect'; } else { hookName = 'useEffect'; } @@ -244,9 +283,21 @@ export function commitHookEffectListUnmount( if ((effect.tag & flags) === flags) { // Unmount const inst = effect.inst; + if ( + enableUseResourceEffectHook && + effect.kind === ResourceEffectKind && + effect.resource != null && + (effect.create != null || + // TODO(@poteto) this feels gross + finishedWork.return == null) + ) { + inst.destroy = effect.destroy; + } const destroy = inst.destroy; if (destroy !== undefined) { inst.destroy = undefined; + const resource = effect.resource; + effect.resource = null; if (enableSchedulingProfiler) { if ((flags & HookPassive) !== NoHookEffect) { markComponentPassiveEffectUnmountStarted(finishedWork); @@ -260,7 +311,12 @@ export function commitHookEffectListUnmount( setIsRunningInsertionEffect(true); } } - safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); + safelyCallDestroy( + finishedWork, + nearestMountedAncestor, + destroy, + resource, + ); if (__DEV__) { if ((flags & HookInsertion) !== NoHookEffect) { setIsRunningInsertionEffect(false); @@ -876,19 +932,21 @@ export function safelyDetachRef( function safelyCallDestroy( current: Fiber, nearestMountedAncestor: Fiber | null, - destroy: () => void, + destroy: mixed => void, + resource: mixed, ) { + const destroy_ = resource == null ? destroy : destroy.bind(null, resource); if (__DEV__) { runWithFiberInDEV( current, callDestroyInDEV, current, nearestMountedAncestor, - destroy, + destroy_, ); } else { try { - destroy(); + destroy_(); } catch (error) { captureCommitPhaseError(current, nearestMountedAncestor, error); } diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 01f3b400ad..e26bb9912b 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -48,6 +48,7 @@ import { disableLegacyMode, enableNoCloningMemoCache, enableContextProfiling, + enableUseResourceEffectHook, } from 'shared/ReactFeatureFlags'; import { REACT_CONTEXT_TYPE, @@ -218,12 +219,33 @@ type EffectInstance = { destroy: void | (() => void), }; -export type Effect = { +export const SimpleEffectKind: 0 = 0; +export const ResourceEffectKind: 1 = 1; +export type EffectKind = typeof SimpleEffectKind | typeof ResourceEffectKind; +export type Effect = SimpleEffect | ResourceEffect; +export type SimpleEffect = { + kind: typeof SimpleEffectKind, tag: HookFlags, - create: () => (() => void) | void, inst: EffectInstance, - deps: Array | null, + create: () => (() => void) | void, + createDeps: Array | null, + update: void | null, + updateDeps: void | null, + destroy: void | null, next: Effect, + resource: mixed, +}; +export type ResourceEffect = { + kind: typeof ResourceEffectKind, + tag: HookFlags, + create: () => mixed, + inst: EffectInstance, + createDeps: Array | void | null, + update: ((resource: mixed) => void) | void, + updateDeps: Array | void | null, + destroy: ((resource: mixed) => void) | void, + next: Effect, + resource: mixed, }; type StoreInstance = { @@ -347,6 +369,23 @@ function checkDepsAreArrayDev(deps: mixed): void { } } +function checkDepsAreNonEmptyArrayDev(deps: mixed): void { + if (__DEV__) { + if ( + deps !== undefined && + deps !== null && + isArray(deps) && + deps.length === 0 + ) { + console.error( + '%s received a dependency array with no dependencies. When ' + + 'specified, the dependency array must have at least one dependency.', + currentHookNameInDev, + ); + } + } +} + function warnOnHookMismatchInDev(currentHookName: HookType): void { if (__DEV__) { const componentName = getComponentNameFromFiber(currentlyRenderingFiber); @@ -1720,8 +1759,9 @@ function mountSyncExternalStore( fiber.flags |= PassiveEffect; pushEffect( HookHasEffect | HookPassive, - updateStoreInstance.bind(null, fiber, inst, nextSnapshot, getSnapshot), + SimpleEffectKind, createEffectInstance(), + updateStoreInstance.bind(null, fiber, inst, nextSnapshot, getSnapshot), null, ); @@ -1790,9 +1830,10 @@ function updateSyncExternalStore( fiber.flags |= PassiveEffect; pushEffect( HookHasEffect | HookPassive, - updateStoreInstance.bind(null, fiber, inst, nextSnapshot, getSnapshot), + SimpleEffectKind, createEffectInstance(), - null, + updateStoreInstance.bind(null, fiber, inst, nextSnapshot, getSnapshot), + undefined, ); // Unless we're rendering a blocking lane, schedule a consistency check. @@ -2450,8 +2491,9 @@ function updateActionStateImpl( currentlyRenderingFiber.flags |= PassiveEffect; pushEffect( HookHasEffect | HookPassive, - actionStateActionEffect.bind(null, actionQueue, action), + SimpleEffectKind, createEffectInstance(), + actionStateActionEffect.bind(null, actionQueue, action), null, ); } @@ -2510,15 +2552,26 @@ function rerenderActionState( function pushEffect( tag: HookFlags, - create: () => (() => void) | void, + kind: EffectKind, inst: EffectInstance, - deps: Array | null, + create: (() => (() => void) | void) | (() => mixed), + createDeps: Array | void | null, + update: ((resource: mixed) => void) | void, + updateDeps: Array | void | null, + destroy: ((resource: mixed) => void) | void, + resource: mixed, ): Effect { + // $FlowFixMe[incompatible-type] (@poteto) could not figure out how to appease Flow const effect: Effect = { + kind, tag, create, + createDeps, + update, + updateDeps, + destroy, inst, - deps, + resource, // Circular next: (null: any), }; @@ -2567,8 +2620,9 @@ function mountEffectImpl( currentlyRenderingFiber.flags |= fiberFlags; hook.memoizedState = pushEffect( HookHasEffect | hookFlags, - create, + SimpleEffectKind, createEffectInstance(), + create, nextDeps, ); } @@ -2589,9 +2643,16 @@ function updateEffectImpl( if (currentHook !== null) { if (nextDeps !== null) { const prevEffect: Effect = currentHook.memoizedState; - const prevDeps = prevEffect.deps; + const prevDeps = prevEffect.createDeps; + // $FlowFixMe[incompatible-call] (@poteto) if (areHookInputsEqual(nextDeps, prevDeps)) { - hook.memoizedState = pushEffect(hookFlags, create, inst, nextDeps); + hook.memoizedState = pushEffect( + hookFlags, + SimpleEffectKind, + inst, + create, + nextDeps, + ); return; } } @@ -2601,8 +2662,9 @@ function updateEffectImpl( hook.memoizedState = pushEffect( HookHasEffect | hookFlags, - create, + SimpleEffectKind, inst, + create, nextDeps, ); } @@ -2639,6 +2701,149 @@ function updateEffect( updateEffectImpl(PassiveEffect, HookPassive, create, deps); } +function mountResourceEffect( + create: () => mixed, + createDeps: Array | void | null, + update: ((resource: mixed) => void) | void, + updateDeps: Array | void | null, + destroy: ((resource: mixed) => void) | void, +) { + if ( + __DEV__ && + (currentlyRenderingFiber.mode & StrictEffectsMode) !== NoMode && + (currentlyRenderingFiber.mode & NoStrictPassiveEffectsMode) === NoMode + ) { + mountResourceEffectImpl( + MountPassiveDevEffect | PassiveEffect | PassiveStaticEffect, + HookPassive, + create, + createDeps, + update, + updateDeps, + destroy, + ); + } else { + mountResourceEffectImpl( + PassiveEffect | PassiveStaticEffect, + HookPassive, + create, + createDeps, + update, + updateDeps, + destroy, + ); + } +} + +function mountResourceEffectImpl( + fiberFlags: Flags, + hookFlags: HookFlags, + create: () => mixed, + createDeps: Array | void | null, + update: ((resource: mixed) => void) | void, + updateDeps: Array | void | null, + destroy: ((resource: mixed) => void) | void, +) { + const hook = mountWorkInProgressHook(); + currentlyRenderingFiber.flags |= fiberFlags; + hook.memoizedState = pushEffect( + HookHasEffect | hookFlags, + ResourceEffectKind, + createEffectInstance(), + create, + createDeps, + update, + updateDeps, + destroy, + ); +} + +function updateResourceEffect( + create: () => mixed, + createDeps: Array | void | null, + update: ((resource: mixed) => void) | void, + updateDeps: Array | void | null, + destroy: ((resource: mixed) => void) | void, +) { + updateResourceEffectImpl( + PassiveEffect, + HookPassive, + create, + createDeps, + update, + updateDeps, + destroy, + ); +} + +function updateResourceEffectImpl( + fiberFlags: Flags, + hookFlags: HookFlags, + create: () => mixed, + createDeps: Array | void | null, + update: ((resource: mixed) => void) | void, + updateDeps: Array | void | null, + destroy: ((resource: mixed) => void) | void, +) { + const hook = updateWorkInProgressHook(); + const effect: ResourceEffect = hook.memoizedState; + const inst = effect.inst; + + const nextCreateDepsArray = createDeps != null ? createDeps : []; + const nextUpdateDeps = updateDeps !== undefined ? updateDeps : null; + let isCreateDepsSame: boolean; + if (currentHook !== null) { + const prevEffect: ResourceEffect = currentHook.memoizedState; + const prevCreateDepsArray = + prevEffect.createDeps != null ? prevEffect.createDeps : []; + isCreateDepsSame = areHookInputsEqual( + nextCreateDepsArray, + prevCreateDepsArray, + ); + + if (nextUpdateDeps !== null) { + const prevUpdateDeps = + prevEffect.updateDeps != null ? prevEffect.updateDeps : null; + if ( + isCreateDepsSame && + areHookInputsEqual(nextUpdateDeps, prevUpdateDeps) + ) { + hook.memoizedState = pushEffect( + hookFlags, + ResourceEffectKind, + inst, + create, + createDeps, + update, + updateDeps, + destroy, + prevEffect.resource, + ); + return; + } + } + } + + currentlyRenderingFiber.flags |= fiberFlags; + + const resource = + currentHook !== null + ? (currentHook.memoizedState as ResourceEffect).resource + : undefined; + hook.memoizedState = pushEffect( + HookHasEffect | hookFlags, + ResourceEffectKind, + inst, + // $FlowFixMe[incompatible-call] (@poteto) + isCreateDepsSame ? undefined : create, + nextCreateDepsArray, + update, + nextUpdateDeps, + destroy, + resource, + ); +} + function useEffectEventImpl) => Return>( payload: EventFunctionPayload, ) { @@ -3789,6 +3994,9 @@ if (enableUseMemoCacheHook) { if (enableUseEffectEventHook) { (ContextOnlyDispatcher: Dispatcher).useEffectEvent = throwInvalidHookError; } +if (enableUseResourceEffectHook) { + (ContextOnlyDispatcher: Dispatcher).useResourceEffect = throwInvalidHookError; +} if (enableAsyncActions) { (ContextOnlyDispatcher: Dispatcher).useHostTransitionStatus = throwInvalidHookError; @@ -3832,6 +4040,9 @@ if (enableUseMemoCacheHook) { if (enableUseEffectEventHook) { (HooksDispatcherOnMount: Dispatcher).useEffectEvent = mountEvent; } +if (enableUseResourceEffectHook) { + (HooksDispatcherOnMount: Dispatcher).useResourceEffect = mountResourceEffect; +} if (enableAsyncActions) { (HooksDispatcherOnMount: Dispatcher).useHostTransitionStatus = useHostTransitionStatus; @@ -3875,6 +4086,10 @@ if (enableUseMemoCacheHook) { if (enableUseEffectEventHook) { (HooksDispatcherOnUpdate: Dispatcher).useEffectEvent = updateEvent; } +if (enableUseResourceEffectHook) { + (HooksDispatcherOnUpdate: Dispatcher).useResourceEffect = + updateResourceEffect; +} if (enableAsyncActions) { (HooksDispatcherOnUpdate: Dispatcher).useHostTransitionStatus = useHostTransitionStatus; @@ -3918,6 +4133,10 @@ if (enableUseMemoCacheHook) { if (enableUseEffectEventHook) { (HooksDispatcherOnRerender: Dispatcher).useEffectEvent = updateEvent; } +if (enableUseResourceEffectHook) { + (HooksDispatcherOnRerender: Dispatcher).useResourceEffect = + updateResourceEffect; +} if (enableAsyncActions) { (HooksDispatcherOnRerender: Dispatcher).useHostTransitionStatus = useHostTransitionStatus; @@ -4108,6 +4327,27 @@ if (__DEV__) { return mountEvent(callback); }; } + if (enableUseResourceEffectHook) { + (HooksDispatcherOnMountInDEV: Dispatcher).useResourceEffect = + function useResourceEffect( + create: () => mixed, + createDeps: Array | void | null, + update: ((resource: mixed) => void) | void, + updateDeps: Array | void | null, + destroy: ((resource: mixed) => void) | void, + ): void { + currentHookNameInDev = 'useResourceEffect'; + mountHookTypesDev(); + checkDepsAreNonEmptyArrayDev(updateDeps); + return mountResourceEffect( + create, + createDeps, + update, + updateDeps, + destroy, + ); + }; + } if (enableAsyncActions) { (HooksDispatcherOnMountInDEV: Dispatcher).useHostTransitionStatus = useHostTransitionStatus; @@ -4300,6 +4540,26 @@ if (__DEV__) { return mountEvent(callback); }; } + if (enableUseResourceEffectHook) { + (HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher).useResourceEffect = + function useResourceEffect( + create: () => mixed, + createDeps: Array | void | null, + update: ((resource: mixed) => void) | void, + updateDeps: Array | void | null, + destroy: ((resource: mixed) => void) | void, + ): void { + currentHookNameInDev = 'useResourceEffect'; + updateHookTypesDev(); + return mountResourceEffect( + create, + createDeps, + update, + updateDeps, + destroy, + ); + }; + } if (enableAsyncActions) { (HooksDispatcherOnMountWithHookTypesInDEV: Dispatcher).useHostTransitionStatus = useHostTransitionStatus; @@ -4491,6 +4751,26 @@ if (__DEV__) { return updateEvent(callback); }; } + if (enableUseResourceEffectHook) { + (HooksDispatcherOnUpdateInDEV: Dispatcher).useResourceEffect = + function useResourceEffect( + create: () => mixed, + createDeps: Array | void | null, + update: ((resource: mixed) => void) | void, + updateDeps: Array | void | null, + destroy: ((resource: mixed) => void) | void, + ) { + currentHookNameInDev = 'useResourceEffect'; + updateHookTypesDev(); + return updateResourceEffect( + create, + createDeps, + update, + updateDeps, + destroy, + ); + }; + } if (enableAsyncActions) { (HooksDispatcherOnUpdateInDEV: Dispatcher).useHostTransitionStatus = useHostTransitionStatus; @@ -4682,6 +4962,26 @@ if (__DEV__) { return updateEvent(callback); }; } + if (enableUseResourceEffectHook) { + (HooksDispatcherOnRerenderInDEV: Dispatcher).useResourceEffect = + function useResourceEffect( + create: () => mixed, + createDeps: Array | void | null, + update: ((resource: mixed) => void) | void, + updateDeps: Array | void | null, + destroy: ((resource: mixed) => void) | void, + ) { + currentHookNameInDev = 'useResourceEffect'; + updateHookTypesDev(); + return updateResourceEffect( + create, + createDeps, + update, + updateDeps, + destroy, + ); + }; + } if (enableAsyncActions) { (HooksDispatcherOnRerenderInDEV: Dispatcher).useHostTransitionStatus = useHostTransitionStatus; @@ -4897,6 +5197,27 @@ if (__DEV__) { return mountEvent(callback); }; } + if (InvalidNestedHooksDispatcherOnMountInDEV) { + (HooksDispatcherOnRerenderInDEV: Dispatcher).useResourceEffect = + function useResourceEffect( + create: () => mixed, + createDeps: Array | void | null, + update: ((resource: mixed) => void) | void, + updateDeps: Array | void | null, + destroy: ((resource: mixed) => void) | void, + ): void { + currentHookNameInDev = 'useResourceEffect'; + warnInvalidHookAccess(); + mountHookTypesDev(); + return mountResourceEffect( + create, + createDeps, + update, + updateDeps, + destroy, + ); + }; + } if (enableAsyncActions) { (InvalidNestedHooksDispatcherOnMountInDEV: Dispatcher).useHostTransitionStatus = useHostTransitionStatus; @@ -5115,6 +5436,27 @@ if (__DEV__) { return updateEvent(callback); }; } + if (enableUseResourceEffectHook) { + (InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher).useResourceEffect = + function useResourceEffect( + create: () => mixed, + createDeps: Array | void | null, + update: ((resource: mixed) => void) | void, + updateDeps: Array | void | null, + destroy: ((resource: mixed) => void) | void, + ) { + currentHookNameInDev = 'useResourceEffect'; + warnInvalidHookAccess(); + updateHookTypesDev(); + return updateResourceEffect( + create, + createDeps, + update, + updateDeps, + destroy, + ); + }; + } if (enableAsyncActions) { (InvalidNestedHooksDispatcherOnUpdateInDEV: Dispatcher).useHostTransitionStatus = useHostTransitionStatus; @@ -5333,6 +5675,27 @@ if (__DEV__) { return updateEvent(callback); }; } + if (enableUseResourceEffectHook) { + (InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher).useResourceEffect = + function useResourceEffect( + create: () => mixed, + createDeps: Array | void | null, + update: ((resource: mixed) => void) | void, + updateDeps: Array | void | null, + destroy: ((resource: mixed) => void) | void, + ) { + currentHookNameInDev = 'useResourceEffect'; + warnInvalidHookAccess(); + updateHookTypesDev(); + return updateResourceEffect( + create, + createDeps, + update, + updateDeps, + destroy, + ); + }; + } if (enableAsyncActions) { (InvalidNestedHooksDispatcherOnRerenderInDEV: Dispatcher).useHostTransitionStatus = useHostTransitionStatus; diff --git a/packages/react-reconciler/src/ReactInternalTypes.js b/packages/react-reconciler/src/ReactInternalTypes.js index d91727525c..bd40c74785 100644 --- a/packages/react-reconciler/src/ReactInternalTypes.js +++ b/packages/react-reconciler/src/ReactInternalTypes.js @@ -47,6 +47,7 @@ export type HookType = | 'useRef' | 'useEffect' | 'useEffectEvent' + | 'useResourceEffect' | 'useInsertionEffect' | 'useLayoutEffect' | 'useCallback' @@ -412,6 +413,13 @@ export type Dispatcher = { deps: Array | void | null, ): void, useEffectEvent?: ) => mixed>(callback: F) => F, + useResourceEffect?: ( + create: () => mixed, + createDeps: Array | void | null, + update: ((resource: mixed) => void) | void, + updateDeps: Array | void | null, + destroy: ((resource: mixed) => 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 bc87e47083..bf1822b065 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -41,6 +41,7 @@ let waitFor; let waitForThrow; let waitForPaint; let assertLog; +let useResourceEffect; describe('ReactHooksWithNoopRenderer', () => { beforeEach(() => { @@ -66,6 +67,7 @@ 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)) { @@ -3252,6 +3254,490 @@ describe('ReactHooksWithNoopRenderer', () => { }); }); + // @gate enableUseResourceEffectHook + describe('useResourceEffect', () => { + class Resource { + isDeleted: false; + id: string; + opts: mixed; + constructor(id, opts) { + this.id = id; + this.opts = opts; + } + update(opts) { + if (this.isDeleted) { + console.error('Cannot update deleted resource'); + return; + } + this.opts = opts; + } + destroy() { + this.isDeleted = true; + } + } + + // @gate enableUseResourceEffectHook + it('validates create return value', async () => { + function App({id}) { + useResourceEffect(() => { + Scheduler.log(`create(${id})`); + }, [id]); + return null; + } + + await expect(async () => { + await act(() => { + ReactNoop.render(); + }); + }).toErrorDev( + 'useResourceEffect must provide a callback which returns a resource. ' + + 'If a managed resource is not needed here, use useEffect. Received undefined', + {withoutStack: true}, + ); + }); + + // @gate enableUseResourceEffectHook + it('validates non-empty update deps', async () => { + function App({id}) { + useResourceEffect( + () => { + Scheduler.log(`create(${id})`); + return {}; + }, + [id], + () => { + Scheduler.log('update'); + }, + [], + ); + return null; + } + + await expect(async () => { + await act(() => { + ReactNoop.render(); + }); + }).toErrorDev( + 'useResourceEffect received a dependency array with no dependencies. ' + + 'When specified, the dependency array must have at least one dependency.', + ); + }); + + // @gate enableUseResourceEffectHook + it('simple mount and update', async () => { + function App({id, username}) { + const opts = useMemo(() => { + return {username}; + }, [username]); + useResourceEffect( + () => { + const resource = new Resource(id, opts); + Scheduler.log(`create(${resource.id}, ${resource.opts.username})`); + return resource; + }, + [id], + resource => { + resource.update(opts); + Scheduler.log(`update(${resource.id}, ${resource.opts.username})`); + }, + [opts], + resource => { + resource.destroy(); + Scheduler.log(`destroy(${resource.id}, ${resource.opts.username})`); + }, + ); + return null; + } + + await act(() => { + ReactNoop.render(); + }); + assertLog(['create(1, Jack)']); + + await act(() => { + ReactNoop.render(); + }); + assertLog(['update(1, Lauren)']); + + await act(() => { + ReactNoop.render(); + }); + assertLog([]); + + await act(() => { + ReactNoop.render(); + }); + assertLog(['update(1, Jordan)']); + + await act(() => { + ReactNoop.render(); + }); + assertLog(['destroy(1, Jordan)', 'create(2, Jack)']); + + await act(() => { + ReactNoop.render(null); + }); + assertLog(['destroy(2, Jack)']); + }); + + // @gate enableUseResourceEffectHook + it('simple mount with no update', async () => { + function App({id, username}) { + const opts = useMemo(() => { + return {username}; + }, [username]); + useResourceEffect( + () => { + const resource = new Resource(id, opts); + Scheduler.log(`create(${resource.id}, ${resource.opts.username})`); + return resource; + }, + [id], + resource => { + resource.update(opts); + Scheduler.log(`update(${resource.id}, ${resource.opts.username})`); + }, + [opts], + resource => { + resource.destroy(); + Scheduler.log(`destroy(${resource.id}, ${resource.opts.username})`); + }, + ); + return null; + } + + await act(() => { + ReactNoop.render(); + }); + assertLog(['create(1, Jack)']); + + await act(() => { + ReactNoop.render(null); + }); + assertLog(['destroy(1, Jack)']); + }); + + // @gate enableUseResourceEffectHook + it('calls update on every render if no deps are specified', async () => { + function App({id, username}) { + const opts = useMemo(() => { + return {username}; + }, [username]); + useResourceEffect( + () => { + const resource = new Resource(id, opts); + Scheduler.log(`create(${resource.id}, ${resource.opts.username})`); + return resource; + }, + [id], + resource => { + resource.update(opts); + Scheduler.log(`update(${resource.id}, ${resource.opts.username})`); + }, + ); + return null; + } + + await act(() => { + ReactNoop.render(); + }); + assertLog(['create(1, Jack)']); + + await act(() => { + ReactNoop.render(); + }); + assertLog(['update(1, Jack)']); + + await act(() => { + ReactNoop.render(); + }); + assertLog(['create(2, Jack)']); + + await act(() => { + ReactNoop.render(); + }); + + assertLog(['update(2, Lauren)']); + }); + + // @gate enableUseResourceEffectHook + it('does not unmount previous useResourceEffect between updates', async () => { + function App({id}) { + useResourceEffect( + () => { + const resource = new Resource(id); + Scheduler.log(`create(${resource.id})`); + return resource; + }, + undefined, + resource => { + Scheduler.log(`update(${resource.id})`); + }, + undefined, + resource => { + Scheduler.log(`destroy(${resource.id})`); + resource.destroy(); + }, + ); + return ; + } + + await act(async () => { + ReactNoop.render(, () => Scheduler.log('Sync effect')); + await waitFor(['Id: 0', 'Sync effect']); + expect(ReactNoop).toMatchRenderedOutput(); + }); + + assertLog(['create(0)']); + + await act(async () => { + ReactNoop.render(, () => Scheduler.log('Sync effect')); + await waitFor(['Id: 1', 'Sync effect']); + expect(ReactNoop).toMatchRenderedOutput(); + }); + + assertLog(['update(0)']); + }); + + // @gate enableUseResourceEffectHook + it('unmounts only on deletion', async () => { + function App({id}) { + useResourceEffect( + () => { + const resource = new Resource(id); + Scheduler.log(`create(${resource.id})`); + return resource; + }, + undefined, + resource => { + Scheduler.log(`update(${resource.id})`); + }, + undefined, + resource => { + Scheduler.log(`destroy(${resource.id})`); + resource.destroy(); + }, + ); + return ; + } + await act(async () => { + ReactNoop.render(, () => Scheduler.log('Sync effect')); + await waitFor(['Id: 0', 'Sync effect']); + expect(ReactNoop).toMatchRenderedOutput(); + }); + + assertLog(['create(0)']); + + ReactNoop.render(null); + await waitForAll(['destroy(0)']); + expect(ReactNoop).toMatchRenderedOutput(null); + }); + + // @gate enableUseResourceEffectHook + it('unmounts on deletion after skipped effect', async () => { + function Wrapper(props) { + return ; + } + function App({id, username}) { + const opts = useMemo(() => { + return {username}; + }, [username]); + useResourceEffect( + () => { + const resource = new Resource(id, opts); + Scheduler.log(`create(${resource.id}, ${resource.opts.username})`); + return resource; + }, + [id], + resource => { + resource.update(opts); + Scheduler.log(`update(${resource.id}, ${resource.opts.username})`); + }, + [opts], + resource => { + resource.destroy(); + Scheduler.log(`destroy(${resource.id}, ${resource.opts.username})`); + }, + ); + return ; + } + + await act(async () => { + ReactNoop.render(, () => + Scheduler.log('Sync effect'), + ); + await waitFor(['Id: 0', 'Sync effect']); + expect(ReactNoop).toMatchRenderedOutput(); + }); + + assertLog(['create(0, Sathya)']); + + await act(async () => { + ReactNoop.render(, () => + Scheduler.log('Sync effect'), + ); + await waitFor(['Id: 0', 'Sync effect']); + expect(ReactNoop).toMatchRenderedOutput(); + }); + + assertLog(['update(0, Lauren)']); + + ReactNoop.render(null); + await waitForAll(['destroy(0, Lauren)']); + expect(ReactNoop).toMatchRenderedOutput(null); + }); + + // @gate enableUseResourceEffectHook + it('handles errors in create on mount', async () => { + function App({id}) { + useResourceEffect( + () => { + Scheduler.log(`Mount A [${id}]`); + return {}; + }, + undefined, + undefined, + undefined, + resource => { + Scheduler.log(`Unmount A [${id}]`); + }, + ); + useResourceEffect( + () => { + Scheduler.log('Oops!'); + throw new Error('Oops!'); + // eslint-disable-next-line no-unreachable + Scheduler.log(`Mount B [${id}]`); + return {}; + }, + undefined, + undefined, + undefined, + resource => { + Scheduler.log(`Unmount B [${id}]`); + }, + ); + return ; + } + await expect(async () => { + await act(async () => { + ReactNoop.render(, () => Scheduler.log('Sync effect')); + await waitFor(['Id: 0', 'Sync effect']); + expect(ReactNoop).toMatchRenderedOutput(); + }); + }).rejects.toThrow('Oops'); + + assertLog([ + 'Mount A [0]', + 'Oops!', + // Clean up effect A. There's no effect B to clean-up, because it + // never mounted. + 'Unmount A [0]', + ]); + expect(ReactNoop).toMatchRenderedOutput(null); + }); + + // @gate enableUseResourceEffectHook + it('handles errors in create on update', async () => { + function App({id}) { + useResourceEffect( + () => { + Scheduler.log(`Mount A [${id}]`); + return {}; + }, + [], + () => { + if (id === 1) { + Scheduler.log('Oops!'); + throw new Error('Oops error!'); + } + Scheduler.log(`Update A [${id}]`); + }, + [id], + () => { + Scheduler.log(`Unmount A [${id}]`); + }, + ); + return ; + } + await act(async () => { + ReactNoop.render(, () => Scheduler.log('Sync effect')); + await waitFor(['Id: 0', 'Sync effect']); + expect(ReactNoop).toMatchRenderedOutput(); + ReactNoop.flushPassiveEffects(); + assertLog(['Mount A [0]']); + }); + + await expect(async () => { + await act(async () => { + // This update will trigger an error + ReactNoop.render(, () => Scheduler.log('Sync effect')); + await waitFor(['Id: 1', 'Sync effect']); + expect(ReactNoop).toMatchRenderedOutput(); + ReactNoop.flushPassiveEffects(); + assertLog(['Oops!', 'Unmount A [1]']); + expect(ReactNoop).toMatchRenderedOutput(null); + }); + }).rejects.toThrow('Oops error!'); + }); + + // @gate enableUseResourceEffectHook + it('handles errors in destroy on update', async () => { + function App({id, username}) { + const opts = useMemo(() => { + return {username}; + }, [username]); + useResourceEffect( + () => { + const resource = new Resource(id, opts); + Scheduler.log(`Mount A [${id}, ${resource.opts.username}]`); + return resource; + }, + [id], + resource => { + resource.update(opts); + Scheduler.log(`Update A [${id}, ${resource.opts.username}]`); + }, + [opts], + resource => { + Scheduler.log(`Oops, ${resource.opts.username}!`); + if (id === 1) { + throw new Error(`Oops ${resource.opts.username} error!`); + } + Scheduler.log(`Unmount A [${id}, ${resource.opts.username}]`); + }, + ); + return ; + } + await act(async () => { + ReactNoop.render(, () => + Scheduler.log('Sync effect'), + ); + await waitFor(['Id: 0', 'Sync effect']); + expect(ReactNoop).toMatchRenderedOutput(); + ReactNoop.flushPassiveEffects(); + assertLog(['Mount A [0, Lauren]']); + }); + + await expect(async () => { + await act(async () => { + // This update will trigger an error during passive effect unmount + ReactNoop.render(, () => + Scheduler.log('Sync effect'), + ); + await waitFor(['Id: 1', 'Sync effect']); + expect(ReactNoop).toMatchRenderedOutput(); + ReactNoop.flushPassiveEffects(); + assertLog(['Oops, Lauren!', 'Mount A [1, Sathya]', 'Oops, Sathya!']); + }); + // TODO(lauren) more explicit assertions. this is weird because we + // destroy both the first and second resource + }).rejects.toThrow(); + + expect(ReactNoop).toMatchRenderedOutput(null); + }); + }); + describe('useCallback', () => { it('memoizes callback by comparing inputs', async () => { class IncrementButton extends React.PureComponent { diff --git a/packages/react/index.development.js b/packages/react/index.development.js index 398e672404..8472789ba3 100644 --- a/packages/react/index.development.js +++ b/packages/react/index.development.js @@ -60,6 +60,7 @@ 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 676b9eea4d..d41774e1fe 100644 --- a/packages/react/index.experimental.development.js +++ b/packages/react/index.experimental.development.js @@ -41,6 +41,7 @@ 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 7999655f30..7892f31dd4 100644 --- a/packages/react/index.fb.js +++ b/packages/react/index.fb.js @@ -19,6 +19,7 @@ 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 ed064fe803..15f70bf70a 100644 --- a/packages/react/src/ReactClient.js +++ b/packages/react/src/ReactClient.js @@ -42,6 +42,7 @@ import { useContext, useEffect, useEffectEvent, + useResourceEffect, useImperativeHandle, useDebugValue, useInsertionEffect, @@ -89,6 +90,7 @@ export { useContext, useEffect, useEffectEvent as experimental_useEffectEvent, + useResourceEffect as experimental_useResourceEffect, useImperativeHandle, useDebugValue, useInsertionEffect, diff --git a/packages/react/src/ReactHooks.js b/packages/react/src/ReactHooks.js index 956a2a96b4..13266b998a 100644 --- a/packages/react/src/ReactHooks.js +++ b/packages/react/src/ReactHooks.js @@ -226,6 +226,24 @@ export function useEffectEvent) => mixed>( return dispatcher.useEffectEvent(callback); } +export function useResourceEffect( + create: () => mixed, + createDeps: Array | void | null, + update: ((resource: mixed) => void) | void, + updateDeps: Array | void | null, + destroy: ((resource: mixed) => void) | void, +): void { + 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/packages/shared/ReactFeatureFlags.js b/packages/shared/ReactFeatureFlags.js index 44d6e8629d..5c98562ca7 100644 --- a/packages/shared/ReactFeatureFlags.js +++ b/packages/shared/ReactFeatureFlags.js @@ -161,6 +161,11 @@ export const transitionLaneExpirationMs = 5000; */ export const enableInfiniteRenderLoopDetection = false; +/** + * Experimental new hook for better managing resources in effects. + */ +export const enableUseResourceEffectHook = false; + // ----------------------------------------------------------------------------- // Ready for next major. // diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index a553c275fe..f0a6d2ca68 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -92,6 +92,7 @@ export const retryLaneExpirationMs = 5000; export const syncLaneExpirationMs = 250; export const transitionLaneExpirationMs = 5000; export const useModernStrictMode = true; +export const enableUseResourceEffectHook = false; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.native-oss.js b/packages/shared/forks/ReactFeatureFlags.native-oss.js index d59e3f9698..c4bebc184d 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -84,6 +84,7 @@ export const syncLaneExpirationMs = 250; export const transitionLaneExpirationMs = 5000; export const useModernStrictMode = true; export const enableSiblingPrerendering = false; +export const enableUseResourceEffectHook = false; // Profiling Only export const enableProfilerTimer = __PROFILE__; diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.js index c27b2d6913..88e3855fe7 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -83,6 +83,8 @@ export const renameElementSymbol = true; export const enableShallowPropDiffing = false; export const enableSiblingPrerendering = false; +export const enableUseResourceEffectHook = false; + // TODO: This must be in sync with the main ReactFeatureFlags file because // the Test Renderer's value must be the same as the one used by the // react package. diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js index 170145c93d..55e72ab391 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -80,6 +80,7 @@ export const transitionLaneExpirationMs = 5000; export const useModernStrictMode = true; export const enableFabricCompleteRootInCommitPhase = false; export const enableSiblingPrerendering = false; +export const enableUseResourceEffectHook = false; // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js index 46e939fd73..af945fddaa 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -94,5 +94,7 @@ export const enableOwnerStacks = false; export const enableShallowPropDiffing = false; export const enableSiblingPrerendering = false; +export const enableUseResourceEffectHook = false; + // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType); diff --git a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js index 482ebb03c6..db38e0e1fd 100644 --- a/packages/shared/forks/ReactFeatureFlags.www-dynamic.js +++ b/packages/shared/forks/ReactFeatureFlags.www-dynamic.js @@ -42,6 +42,8 @@ export const enableSchedulingProfiler = __VARIANT__; export const enableInfiniteRenderLoopDetection = __VARIANT__; export const enableSiblingPrerendering = __VARIANT__; +export const enableUseResourceEffectHook = __VARIANT__; + // TODO: These flags are hard-coded to the default values used in open source. // Update the tests so that they pass in either mode, then set these // to __VARIANT__. diff --git a/packages/shared/forks/ReactFeatureFlags.www.js b/packages/shared/forks/ReactFeatureFlags.www.js index 013092c1c4..67d1ad74c0 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -31,6 +31,7 @@ export const { enableSiblingPrerendering, enableTransitionTracing, enableTrustedTypesIntegration, + enableUseResourceEffectHook, favorSafetyOverHydrationPerf, renameElementSymbol, retryLaneExpirationMs,