From d693ca3ea38098f02175eb375eea0db03bfa3037 Mon Sep 17 00:00:00 2001 From: lauren Date: Tue, 5 Nov 2024 21:24:36 -0500 Subject: [PATCH] [crud] Basic implementation wip: - [ ] more tests - [ ] error handling - [ ] flow shenanigans --- .../src/ReactFiberCallUserSpace.js | 22 +- .../src/ReactFiberCommitEffects.js | 58 ++- .../react-reconciler/src/ReactFiberHooks.js | 392 +++++++++++++++- .../src/ReactInternalTypes.js | 8 + .../ReactHooksWithNoopRenderer-test.js | 427 ++++++++++++++++++ packages/react/index.development.js | 1 + .../react/index.experimental.development.js | 1 + packages/react/index.experimental.js | 1 + packages/react/index.fb.js | 1 + packages/react/index.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 + .../shared/forks/ReactFeatureFlags.www.js | 2 + 19 files changed, 918 insertions(+), 28 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberCallUserSpace.js b/packages/react-reconciler/src/ReactFiberCallUserSpace.js index ada092438a..ec9585bd6f 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 {SimpleEffectKind} from './ReactFiberHooks'; // 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,22 @@ 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 (effect.kind === SimpleEffectKind) { + const create = effect.create; + const inst = effect.inst; + const destroy = create(); + inst.destroy = destroy; + return destroy; + } else if (typeof effect.destroy === 'function') { + const inst = effect.inst; + const _destroy = effect.destroy; + const destroy = () => { + _destroy(effect.resource); + effect.resource = null; + }; + inst.destroy = destroy; + return destroy; + } }, }; diff --git a/packages/react-reconciler/src/ReactFiberCommitEffects.js b/packages/react-reconciler/src/ReactFiberCommitEffects.js index 4d10c9ee6f..32a1cdea95 100644 --- a/packages/react-reconciler/src/ReactFiberCommitEffects.js +++ b/packages/react-reconciler/src/ReactFiberCommitEffects.js @@ -71,6 +71,7 @@ import { } from './ReactFiberCallUserSpace'; import {runWithFiberInDEV} from './ReactCurrentFiber'; +import {ResourceEffectKind, SimpleEffectKind} from './ReactFiberHooks'; function shouldProfile(current: Fiber): boolean { return ( @@ -147,19 +148,45 @@ export function commitHookEffectListMount( // Mount let destroy; + if (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') { + // TODO: 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) { @@ -177,6 +204,8 @@ export function commitHookEffectListMount( hookName = 'useLayoutEffect'; } else if ((effect.tag & HookInsertion) !== NoFlags) { hookName = 'useInsertionEffect'; + } else if (effect.kind === ResourceEffectKind) { + hookName = 'useResourceEffect'; } else { hookName = 'useEffect'; } @@ -245,6 +274,9 @@ export function commitHookEffectListUnmount( if ((effect.tag & flags) === flags) { // Unmount const inst = effect.inst; + if (effect.kind === ResourceEffectKind && effect.resource != null) { + inst.destroy = effect.destroy; + } const destroy = inst.destroy; if (destroy !== undefined) { inst.destroy = undefined; @@ -261,7 +293,13 @@ export function commitHookEffectListUnmount( setIsRunningInsertionEffect(true); } } - safelyCallDestroy(finishedWork, nearestMountedAncestor, destroy); + safelyCallDestroy( + finishedWork, + nearestMountedAncestor, + destroy, + effect.resource, + ); + effect.resource = null; if (__DEV__) { if ((flags & HookInsertion) !== NoHookEffect) { setIsRunningInsertionEffect(false); @@ -877,19 +915,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..bfaadeffe4 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), createEffectInstance(), + SimpleEffectKind, + 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), createEffectInstance(), - null, + SimpleEffectKind, + 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), createEffectInstance(), + SimpleEffectKind, + actionStateActionEffect.bind(null, actionQueue, action), null, ); } @@ -2510,15 +2552,26 @@ function rerenderActionState( function pushEffect( tag: HookFlags, - create: () => (() => void) | void, inst: EffectInstance, - deps: Array | null, + kind: EffectKind, + 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 lauren 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, createEffectInstance(), + SimpleEffectKind, + 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 lauren if (areHookInputsEqual(nextDeps, prevDeps)) { - hook.memoizedState = pushEffect(hookFlags, create, inst, nextDeps); + hook.memoizedState = pushEffect( + hookFlags, + inst, + SimpleEffectKind, + create, + nextDeps, + ); return; } } @@ -2601,8 +2662,9 @@ function updateEffectImpl( hook.memoizedState = pushEffect( HookHasEffect | hookFlags, - create, inst, + SimpleEffectKind, + create, nextDeps, ); } @@ -2639,6 +2701,150 @@ 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, + createEffectInstance(), + ResourceEffectKind, + 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: Effect = 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: Effect = 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, + inst, + ResourceEffectKind, + create, + nextUpdateDeps, + update, + updateDeps, + destroy, + ); + hook.memoizedState.resource = prevEffect.resource; + return; + } + } + } + + currentlyRenderingFiber.flags |= fiberFlags; + + hook.memoizedState = pushEffect( + HookHasEffect | hookFlags, + inst, + ResourceEffectKind, + // $FlowFixMe lauren + isCreateDepsSame ? undefined : create, + nextCreateDepsArray, + update, + nextUpdateDeps, + // TODO: this causes a memory leak where the effect only ever cleans up if the createDeps have + // explicitly changed, even on an unmount. + isCreateDepsSame ? undefined : destroy, + ); + if (currentHook != null) { + const currentHookState: Effect = currentHook.memoizedState; + hook.memoizedState.resource = currentHookState.resource; + } +} + function useEffectEventImpl) => Return>( payload: EventFunctionPayload, ) { @@ -3789,6 +3995,9 @@ if (enableUseMemoCacheHook) { if (enableUseEffectEventHook) { (ContextOnlyDispatcher: Dispatcher).useEffectEvent = throwInvalidHookError; } +if (enableUseResourceEffectHook) { + (ContextOnlyDispatcher: Dispatcher).useResourceEffect = throwInvalidHookError; +} if (enableAsyncActions) { (ContextOnlyDispatcher: Dispatcher).useHostTransitionStatus = throwInvalidHookError; @@ -3832,6 +4041,9 @@ if (enableUseMemoCacheHook) { if (enableUseEffectEventHook) { (HooksDispatcherOnMount: Dispatcher).useEffectEvent = mountEvent; } +if (enableUseResourceEffectHook) { + (HooksDispatcherOnMount: Dispatcher).useResourceEffect = mountResourceEffect; +} if (enableAsyncActions) { (HooksDispatcherOnMount: Dispatcher).useHostTransitionStatus = useHostTransitionStatus; @@ -3875,6 +4087,10 @@ if (enableUseMemoCacheHook) { if (enableUseEffectEventHook) { (HooksDispatcherOnUpdate: Dispatcher).useEffectEvent = updateEvent; } +if (enableUseResourceEffectHook) { + (HooksDispatcherOnUpdate: Dispatcher).useResourceEffect = + updateResourceEffect; +} if (enableAsyncActions) { (HooksDispatcherOnUpdate: Dispatcher).useHostTransitionStatus = useHostTransitionStatus; @@ -3918,6 +4134,10 @@ if (enableUseMemoCacheHook) { if (enableUseEffectEventHook) { (HooksDispatcherOnRerender: Dispatcher).useEffectEvent = updateEvent; } +if (enableUseResourceEffectHook) { + (HooksDispatcherOnRerender: Dispatcher).useResourceEffect = + updateResourceEffect; +} if (enableAsyncActions) { (HooksDispatcherOnRerender: Dispatcher).useHostTransitionStatus = useHostTransitionStatus; @@ -4108,6 +4328,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 +4541,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 +4752,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 +4963,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 +5198,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 +5437,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 +5676,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 1c98a9e9c7..bfe12a0017 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..da864eb3e5 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,431 @@ 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('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(['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.skip('unmounts on deletion after skipped effect', 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 ; + } + + 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); + // TODO(lauren) This is a bug + 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], + resource => { + 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!']); + expect(ReactNoop).toMatchRenderedOutput(null); + }); + }).rejects.toThrow('Oops error!'); + }); + + // @gate enableUseResourceEffectHook + it.skip('handles errors in create on destroy', async () => { + function App({id, username}) { + const opts = useMemo(() => { + return {username}; + }, username); + useResourceEffect( + () => { + const resource = new Resource(id, opts); + Scheduler.log(`Mount A [${id}]`); + return resource; + }, + [id], + resource => { + resource.update(opts); + Scheduler.log(`Update A [${id}]`); + }, + [opts], + resource => { + Scheduler.log('Oops!'); + if (id === 1) { + throw new Error('Oops error!'); + } + 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 during passive effect unmount + ReactNoop.render(, () => Scheduler.log('Sync effect')); + await waitFor(['Id: 1', 'Sync effect']); + expect(ReactNoop).toMatchRenderedOutput(); + ReactNoop.flushPassiveEffects(); + // TODO(lauren): another bug here + assertLog(['Oops!', 'Mount A [1]', 'Oops!']); + }); + }).rejects.toThrow('Oops error!'); + + 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.experimental.js b/packages/react/index.experimental.js index ae98e3b91f..b43d25ca41 100644 --- a/packages/react/index.experimental.js +++ b/packages/react/index.experimental.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/index.js b/packages/react/index.js index 2edb0a2c1d..5d9593767e 100644 --- a/packages/react/index.js +++ b/packages/react/index.js @@ -61,6 +61,7 @@ export { useDeferredValue, useEffect, experimental_useEffectEvent, + experimental_useResourceEffect, useImperativeHandle, useInsertionEffect, useLayoutEffect, 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 a0722c3138..920f179735 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 = __EXPERIMENTAL__; + // ----------------------------------------------------------------------------- // Ready for next major. // diff --git a/packages/shared/forks/ReactFeatureFlags.native-fb.js b/packages/shared/forks/ReactFeatureFlags.native-fb.js index b4339c50a5..f238662b4c 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.native-fb.js @@ -94,6 +94,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 ea670690a5..6e687ea097 100644 --- a/packages/shared/forks/ReactFeatureFlags.native-oss.js +++ b/packages/shared/forks/ReactFeatureFlags.native-oss.js @@ -86,6 +86,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 2eb241af6c..ed8de8d651 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.js @@ -84,6 +84,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 1d505aaf56..43d18db940 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.native-fb.js @@ -81,6 +81,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 5c836b48fe..186c2a43e3 100644 --- a/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js +++ b/packages/shared/forks/ReactFeatureFlags.test-renderer.www.js @@ -97,5 +97,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.js b/packages/shared/forks/ReactFeatureFlags.www.js index b86394ecf7..44d1bc9d87 100644 --- a/packages/shared/forks/ReactFeatureFlags.www.js +++ b/packages/shared/forks/ReactFeatureFlags.www.js @@ -125,5 +125,7 @@ export const disableLegacyMode: boolean = export const enableOwnerStacks = false; export const enableShallowPropDiffing = false; +export const enableUseResourceEffectHook = true; + // Flow magic to verify the exports of this file match the original version. ((((null: any): ExportsType): FeatureFlagsType): ExportsType);