diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index d7f9d00e79..514f937609 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -253,6 +253,7 @@ export type ResourceEffectUpdate = { update: ((resource: mixed) => void) | void, deps: Array | void | null, next: Effect, + identity: ResourceEffectIdentity, }; type StoreInstance = { @@ -2585,40 +2586,37 @@ function pushSimpleEffect( return pushEffectImpl(effect); } -function pushResourceEffectIdentity( - tag: HookFlags, +function pushResourceEffect( + identityTag: HookFlags, + updateTag: HookFlags, inst: EffectInstance, create: () => mixed, - deps: Array | void | null, -): Effect { - const effect: ResourceEffectIdentity = { - resourceKind: ResourceEffectIdentityKind, - tag, - create, - deps, - inst, - // Circular - next: (null: any), - }; - return pushEffectImpl(effect); -} - -function pushResourceEffectUpdate( - tag: HookFlags, - inst: EffectInstance, + createDeps: Array | void | null, update: ((resource: mixed) => void) | void, - deps: Array | void | null, + updateDeps: Array | void | null, ): Effect { - const effect: ResourceEffectUpdate = { - resourceKind: ResourceEffectUpdateKind, - tag, - update, - deps, + const effectIdentity: ResourceEffectIdentity = { + resourceKind: ResourceEffectIdentityKind, + tag: identityTag, + create, + deps: createDeps, inst, // Circular next: (null: any), }; - return pushEffectImpl(effect); + pushEffectImpl(effectIdentity); + + const effectUpdate: ResourceEffectUpdate = { + resourceKind: ResourceEffectUpdateKind, + tag: updateTag, + update, + deps: updateDeps, + inst, + identity: effectIdentity, + // Circular + next: (null: any), + }; + return pushEffectImpl(effectUpdate); } function pushEffectImpl(effect: Effect): Effect { @@ -2792,15 +2790,12 @@ function mountResourceEffectImpl( currentlyRenderingFiber.flags |= fiberFlags; const inst = createEffectInstance(); inst.destroy = destroy; - hook.memoizedState = pushResourceEffectIdentity( + hook.memoizedState = pushResourceEffect( HookHasEffect | hookFlags, + hookFlags, inst, create, createDeps, - ); - hook.memoizedState = pushResourceEffectUpdate( - hookFlags, - inst, update, updateDeps, ); @@ -2847,25 +2842,31 @@ function updateResourceEffectImpl( const prevEffect: Effect = currentHook.memoizedState; if (nextCreateDeps !== null) { let prevCreateDeps; - // Seems sketchy but in practice we always push an Identity and an Update together. For safety - // we error in DEV if this does not hold true. - if (prevEffect.resourceKind === ResourceEffectUpdateKind) { + if ( + prevEffect.resourceKind != null && + prevEffect.resourceKind === ResourceEffectUpdateKind + ) { prevCreateDeps = - prevEffect.next.deps != null ? prevEffect.next.deps : null; + prevEffect.identity.deps != null ? prevEffect.identity.deps : null; } else { - if (__DEV__) { - console.error( - 'Expected a ResourceEffectUpdateKind to be pushed together with ' + - 'ResourceEffectIdentityKind, got %s. This is a bug in React.', - prevEffect.resourceKind, - ); - } - prevCreateDeps = prevEffect.deps != null ? prevEffect.deps : null; + throw new Error( + `Expected a ResourceEffectUpdate to be pushed together with ResourceEffectIdentity. This is a bug in React.`, + ); } isCreateDepsSame = areHookInputsEqual(nextCreateDeps, prevCreateDeps); } if (nextUpdateDeps !== null) { - const prevUpdateDeps = prevEffect.deps != null ? prevEffect.deps : null; + let prevUpdateDeps; + if ( + prevEffect.resourceKind != null && + prevEffect.resourceKind === ResourceEffectUpdateKind + ) { + prevUpdateDeps = prevEffect.deps != null ? prevEffect.deps : null; + } else { + throw new Error( + `Expected a ResourceEffectUpdate to be pushed together with ResourceEffectIdentity. This is a bug in React.`, + ); + } isUpdateDepsSame = areHookInputsEqual(nextUpdateDeps, prevUpdateDeps); } } @@ -2874,15 +2875,12 @@ function updateResourceEffectImpl( currentlyRenderingFiber.flags |= fiberFlags; } - hook.memoizedState = pushResourceEffectIdentity( + hook.memoizedState = pushResourceEffect( isCreateDepsSame ? hookFlags : HookHasEffect | hookFlags, + isUpdateDepsSame ? hookFlags : HookHasEffect | hookFlags, inst, create, nextCreateDeps, - ); - hook.memoizedState = pushResourceEffectUpdate( - isUpdateDepsSame ? hookFlags : HookHasEffect | hookFlags, - inst, update, nextUpdateDeps, ); diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index f9b382647d..a0e84b81d0 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -3927,6 +3927,79 @@ describe('ReactHooksWithNoopRenderer', () => { , ); }); + + // @gate enableUseResourceEffectHook + it('composes with other kinds of effects', async () => { + let rerender; + function App({id, username}) { + const [count, rerender_] = useState(0); + rerender = rerender_; + const opts = useMemo(() => { + return {username}; + }, [username]); + useEffect(() => { + Scheduler.log(`useEffect(${count})`); + }, [count]); + 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(['useEffect(0)', '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(() => { + rerender(n => n + 1); + }); + assertLog(['useEffect(1)']); + + await act(() => { + ReactNoop.render(); + }); + assertLog(['update(1, Mofei)']); + + await act(() => { + ReactNoop.render(); + }); + assertLog(['destroy(1, Mofei)', 'create(2, Jack)']); + + await act(() => { + ReactNoop.render(null); + }); + assertLog(['destroy(2, Jack)']); + }); }); describe('useCallback', () => { diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json index 48b6ae91a1..4c99d330f0 100644 --- a/scripts/error-codes/codes.json +++ b/scripts/error-codes/codes.json @@ -527,5 +527,7 @@ "539": "Binary RSC chunks cannot be encoded as strings. This is a bug in the wiring of the React streams.", "540": "String chunks need to be passed in their original shape. Not split into smaller string chunks. This is a bug in the wiring of the React streams.", "541": "Compared context values must be arrays", - "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." + "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." } +