diff --git a/packages/react-devtools-shared/src/backend/renderer.js b/packages/react-devtools-shared/src/backend/renderer.js index cc82fda74cdb1..a9e3b2aa0b415 100644 --- a/packages/react-devtools-shared/src/backend/renderer.js +++ b/packages/react-devtools-shared/src/backend/renderer.js @@ -103,6 +103,7 @@ import type { ComponentFilter, ElementType, } from 'react-devtools-shared/src/types'; +import is from 'shared/objectIs'; type getDisplayNameForFiberType = (fiber: Fiber) => string | null; type getTypeSymbolType = (type: any) => Symbol | number; @@ -1073,6 +1074,49 @@ export function attach( return null; } + function areHookInputsEqual( + nextDeps: Array, + prevDeps: Array | null, + ) { + if (prevDeps === null) { + return false; + } + + for (let i = 0; i < prevDeps.length && i < nextDeps.length; i++) { + if (is(nextDeps[i], prevDeps[i])) { + continue; + } + return false; + } + return true; + } + + function isEffect(memoizedState) { + return ( + memoizedState !== null && + typeof memoizedState === 'object' && + memoizedState.hasOwnProperty('tag') && + memoizedState.hasOwnProperty('create') && + memoizedState.hasOwnProperty('destroy') && + memoizedState.hasOwnProperty('deps') && + (memoizedState.deps === null || Array.isArray(memoizedState.deps)) && + memoizedState.hasOwnProperty('next') + ); + } + + function didHookChange(prev: any, next: any): boolean { + const prevMemoizedState = prev.memoizedState; + const nextMemoizedState = next.memoizedState; + + if (isEffect(prevMemoizedState) && isEffect(nextMemoizedState)) { + return ( + prevMemoizedState !== nextMemoizedState && + !areHookInputsEqual(nextMemoizedState.deps, prevMemoizedState.deps) + ); + } + return nextMemoizedState !== prevMemoizedState; + } + function didHooksChange(prev: any, next: any): boolean { if (prev == null || next == null) { return false; @@ -1086,7 +1130,7 @@ export function attach( next.hasOwnProperty('queue') ) { while (next !== null) { - if (next.memoizedState !== prev.memoizedState) { + if (didHookChange(prev, next)) { return true; } else { next = next.next; diff --git a/packages/react-reconciler/src/ReactFiberHooks.new.js b/packages/react-reconciler/src/ReactFiberHooks.new.js index bac6749f64465..fe49e725fd7fc 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.new.js +++ b/packages/react-reconciler/src/ReactFiberHooks.new.js @@ -1365,7 +1365,7 @@ function updateEffectImpl(fiberFlags, hookFlags, create, deps): void { if (nextDeps !== null) { const prevDeps = prevEffect.deps; if (areHookInputsEqual(nextDeps, prevDeps)) { - pushEffect(hookFlags, create, destroy, nextDeps); + hook.memoizedState = pushEffect(hookFlags, create, destroy, nextDeps); return; } } diff --git a/packages/react-reconciler/src/ReactFiberHooks.old.js b/packages/react-reconciler/src/ReactFiberHooks.old.js index 6a42f73527f93..ccb719dc3d8a1 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.old.js +++ b/packages/react-reconciler/src/ReactFiberHooks.old.js @@ -1344,7 +1344,7 @@ function updateEffectImpl(fiberFlags, hookFlags, create, deps): void { if (nextDeps !== null) { const prevDeps = prevEffect.deps; if (areHookInputsEqual(nextDeps, prevDeps)) { - pushEffect(hookFlags, create, destroy, nextDeps); + hook.memoizedState = pushEffect(hookFlags, create, destroy, nextDeps); return; } } diff --git a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js index a792f8609be44..cc86edb132209 100644 --- a/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js +++ b/packages/react-reconciler/src/__tests__/ReactHooksWithNoopRenderer-test.js @@ -3760,4 +3760,47 @@ describe('ReactHooksWithNoopRenderer', () => { // effects would not fire. expect(Scheduler).toHaveYielded(['Unmount A', 'Unmount B']); }); + + it('effect dependencies are persisted after a render phase update', () => { + let handleClick; + function Test() { + const [count, setCount] = useState(0); + + useEffect(() => { + Scheduler.unstable_yieldValue(`Effect: ${count}`); + }, [count]); + + if (count > 0) { + setCount(0); + } + + handleClick = () => setCount(2); + + return ; + } + + ReactNoop.act(() => { + ReactNoop.render(); + }); + + expect(Scheduler).toHaveYielded(['Render: 0', 'Effect: 0']); + + ReactNoop.act(() => { + handleClick(); + }); + + expect(Scheduler).toHaveYielded(['Render: 0']); + + ReactNoop.act(() => { + handleClick(); + }); + + expect(Scheduler).toHaveYielded(['Render: 0']); + + ReactNoop.act(() => { + handleClick(); + }); + + expect(Scheduler).toHaveYielded(['Render: 0']); + }); });