From 460793740c47a16f46f875cf0c89b6de85cfb14c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Fri, 15 Apr 2022 00:04:39 +0200 Subject: [PATCH 1/2] Fixed an issue with simple memo components being updated with new props during context change --- .../src/ReactFiberBeginWork.new.js | 3 +- .../src/ReactFiberBeginWork.old.js | 3 +- .../src/__tests__/ReactMemo-test.js | 45 +++++++++++++++++++ 3 files changed, 49 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 88876f05f7d8d..ace4f1b4c708f 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -592,8 +592,9 @@ function updateSimpleMemoComponent( } if (current !== null) { const prevProps = current.memoizedProps; + nextProps = shallowEqual(prevProps, nextProps) ? prevProps : nextProps; if ( - shallowEqual(prevProps, nextProps) && + prevProps === nextProps && current.ref === workInProgress.ref && // Prevent bailout if the implementation changed due to hot reload. (__DEV__ ? workInProgress.type === current.type : true) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index 418b714117e81..a22e3b377ced6 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -592,8 +592,9 @@ function updateSimpleMemoComponent( } if (current !== null) { const prevProps = current.memoizedProps; + nextProps = shallowEqual(prevProps, nextProps) ? prevProps : nextProps; if ( - shallowEqual(prevProps, nextProps) && + prevProps === nextProps && current.ref === workInProgress.ref && // Prevent bailout if the implementation changed due to hot reload. (__DEV__ ? workInProgress.type === current.type : true) diff --git a/packages/react-reconciler/src/__tests__/ReactMemo-test.js b/packages/react-reconciler/src/__tests__/ReactMemo-test.js index 0bbab43d33e0d..7dd0b84609d97 100644 --- a/packages/react-reconciler/src/__tests__/ReactMemo-test.js +++ b/packages/react-reconciler/src/__tests__/ReactMemo-test.js @@ -179,6 +179,51 @@ describe('memo', () => { expect(ReactNoop.getChildren()).toEqual([span('Count: 1')]); }); + it('bails out on props referential equality during context change', async () => { + const CountContext = React.createContext(0); + let renderCount = 0; + + function Inner(props) { + React.useContext(CountContext); + return React.useMemo(() => { + const text = `Inner render count: ${++renderCount}`; + return ; + }, [props]); + } + + Inner = memo(Inner); + + function Outer(props) { + React.useContext(CountContext); + return ; + } + + function Parent({value}) { + return ( + }> + + + + + ); + } + + let ctxValue = 0; + ReactNoop.render(); + expect(Scheduler).toFlushAndYield(['Loading...']); + await Promise.resolve(); + expect(Scheduler).toFlushAndYield(['Inner render count: 1']); + expect(ReactNoop.getChildren()).toEqual([ + span('Inner render count: 1'), + ]); + + ReactNoop.render(); + expect(Scheduler).toFlushAndYield([]); + expect(ReactNoop.getChildren()).toEqual([ + span('Inner render count: 1'), + ]); + }); + it('accepts custom comparison function', async () => { function Counter({count}) { return ; From bd13426453866c50b27a813d26e9396aa34dbf8b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Mateusz=20Burzy=C5=84ski?= Date: Fri, 15 Apr 2022 11:23:27 +0200 Subject: [PATCH 2/2] Recycle prevProps in simple memo based on the `shallowEqual` check --- packages/react-reconciler/src/ReactFiberBeginWork.new.js | 6 +++++- packages/react-reconciler/src/ReactFiberBeginWork.old.js | 6 +++++- packages/react-reconciler/src/__tests__/ReactMemo-test.js | 6 ++++++ 3 files changed, 16 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index ace4f1b4c708f..096e3d1cfc3dd 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -592,7 +592,11 @@ function updateSimpleMemoComponent( } if (current !== null) { const prevProps = current.memoizedProps; - nextProps = shallowEqual(prevProps, nextProps) ? prevProps : nextProps; + // potentially recycle `prevProps` if they are the same + // this allows hooks depending on the `props` to be reused + workInProgress.pendingProps = nextProps = shallowEqual(prevProps, nextProps) + ? prevProps + : nextProps; if ( prevProps === nextProps && current.ref === workInProgress.ref && diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index a22e3b377ced6..1377ed13de317 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -592,7 +592,11 @@ function updateSimpleMemoComponent( } if (current !== null) { const prevProps = current.memoizedProps; - nextProps = shallowEqual(prevProps, nextProps) ? prevProps : nextProps; + // potentially recycle `prevProps` if they are the same + // this allows hooks depending on the `props` to be reused + workInProgress.pendingProps = nextProps = shallowEqual(prevProps, nextProps) + ? prevProps + : nextProps; if ( prevProps === nextProps && current.ref === workInProgress.ref && diff --git a/packages/react-reconciler/src/__tests__/ReactMemo-test.js b/packages/react-reconciler/src/__tests__/ReactMemo-test.js index 7dd0b84609d97..5169767f8227f 100644 --- a/packages/react-reconciler/src/__tests__/ReactMemo-test.js +++ b/packages/react-reconciler/src/__tests__/ReactMemo-test.js @@ -222,6 +222,12 @@ describe('memo', () => { expect(ReactNoop.getChildren()).toEqual([ span('Inner render count: 1'), ]); + + ReactNoop.render(); + expect(Scheduler).toFlushAndYield([]); + expect(ReactNoop.getChildren()).toEqual([ + span('Inner render count: 1'), + ]); }); it('accepts custom comparison function', async () => {