From cede9cfe0b1f42fbdb0448c60c980730803e096b Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 5 Sep 2024 15:58:08 -0700 Subject: [PATCH 01/12] Fix useMemoCache with setState in render [ghstack-poisoned] --- .../react-reconciler/src/ReactFiberHooks.js | 18 ++++++++++++++- .../src/__tests__/useMemoCache-test.js | 22 +++++++++---------- 2 files changed, 27 insertions(+), 13 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 83b9d10fbf91d..b0af4b617de9b 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -828,7 +828,9 @@ function renderWithHooksAgain( currentHook = null; workInProgressHook = null; - workInProgress.updateQueue = null; + if (workInProgress.updateQueue != null) { + clearFunctionComponentUpdateQueue(workInProgress.updateQueue); + } if (__DEV__) { // Also validate hook order for cascading updates. @@ -1101,6 +1103,20 @@ if (enableUseMemoCacheHook) { }; } +// NOTE: this function intentionally does not reset memoCache. We reuse updateQueue for the memo +// cache to avoid increasing the size of fibers that don't need a cache, but we don't want to reset +// the cache when other properties are reset. +const clearFunctionComponentUpdateQueue = ( + updateQueue: FunctionComponentUpdateQueue, +) => { + updateQueue.lastEffect = null; + updateQueue.events = null; + updateQueue.stores = null; + if (updateQueue.memoCache != null) { + updateQueue.memoCache.index = 0; + } +}; + function useThenable(thenable: Thenable): T { // Track the position of the thenable within this fiber. const index = thenableIndexCounter; diff --git a/packages/react-reconciler/src/__tests__/useMemoCache-test.js b/packages/react-reconciler/src/__tests__/useMemoCache-test.js index 8a1bd5dcdf644..8e67e79a1a722 100644 --- a/packages/react-reconciler/src/__tests__/useMemoCache-test.js +++ b/packages/react-reconciler/src/__tests__/useMemoCache-test.js @@ -667,7 +667,7 @@ describe('useMemoCache()', () => { } // Baseline / source code - function useUserMemo(value) { + function useManualMemo(value) { return useMemo(() => [value], [value]); } @@ -683,24 +683,22 @@ describe('useMemoCache()', () => { } /** - * Test case: note that the initial render never completes + * Test with useMemoCache */ let root = ReactNoop.createRoot(); - const IncorrectInfiniteComponent = makeComponent(useCompilerMemo); - root.render(); - await waitForThrow( - 'Too many re-renders. React limits the number of renders to prevent ' + - 'an infinite loop.', - ); + const CompilerMemoComponent = makeComponent(useCompilerMemo); + await act(() => { + root.render(); + }); + expect(root).toMatchRenderedOutput(
2
); /** - * Baseline test: initial render is expected to complete after a retry - * (triggered by the setState) + * Test with useMemo */ root = ReactNoop.createRoot(); - const CorrectComponent = makeComponent(useUserMemo); + const HookMemoComponent = makeComponent(useManualMemo); await act(() => { - root.render(); + root.render(); }); expect(root).toMatchRenderedOutput(
2
); }); From 4986fe2075e3dcb1726bfde7ff4e0f6b9c509ed4 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 5 Sep 2024 16:01:44 -0700 Subject: [PATCH 02/12] Update on "Fix useMemoCache with setState in render" Fixes the bug that alexmckenley and mofeiZ found where setState-in-render can reset useMemoCache and cause an infinite loop. The bug was that renderWithHooksAgain() was not resetting hook state when rerendering (so useMemo values were preserved) but was resetting the updateQueue. This meant that the entire memo cache was cleared on a setState-in-render. The fix here is to call a new helper function to clear the update queue. It nulls out other properties, but for memoCache it just sets the index back to zero. [ghstack-poisoned] From 3bca87d99c53001e97c54f657cb3c465801fbbaa Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 5 Sep 2024 16:03:10 -0700 Subject: [PATCH 03/12] Update on "Fix useMemoCache with setState in render" Fixes the bug that alexmckenley and mofeiZ found where setState-in-render can reset useMemoCache and cause an infinite loop. The bug was that renderWithHooksAgain() was not resetting hook state when rerendering (so useMemo values were preserved) but was resetting the updateQueue. This meant that the entire memo cache was cleared on a setState-in-render. The fix here is to call a new helper function to clear the update queue. It nulls out other properties, but for memoCache it just sets the index back to zero. [ghstack-poisoned] --- packages/react-reconciler/src/ReactFiberHooks.js | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index b0af4b617de9b..13584c673eb91 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -1103,7 +1103,7 @@ if (enableUseMemoCacheHook) { }; } -// NOTE: this function intentionally does not reset memoCache. We reuse updateQueue for the memo +// NOTE: this function intentionally does not reset memoCache data. We reuse updateQueue for the memo // cache to avoid increasing the size of fibers that don't need a cache, but we don't want to reset // the cache when other properties are reset. const clearFunctionComponentUpdateQueue = ( @@ -1112,8 +1112,10 @@ const clearFunctionComponentUpdateQueue = ( updateQueue.lastEffect = null; updateQueue.events = null; updateQueue.stores = null; - if (updateQueue.memoCache != null) { - updateQueue.memoCache.index = 0; + if (enableUseMemoCacheHook) { + if (updateQueue.memoCache != null) { + updateQueue.memoCache.index = 0; + } } }; From 9267cb6f01e8565b09c32778582f648062c41c98 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 5 Sep 2024 16:04:56 -0700 Subject: [PATCH 04/12] Update on "Fix useMemoCache with setState in render" Fixes the bug that alexmckenley and mofeiZ found where setState-in-render can reset useMemoCache and cause an infinite loop. The bug was that renderWithHooksAgain() was not resetting hook state when rerendering (so useMemo values were preserved) but was resetting the updateQueue. This meant that the entire memo cache was cleared on a setState-in-render. The fix here is to call a new helper function to clear the update queue. It nulls out other properties, but for memoCache it just sets the index back to zero. [ghstack-poisoned] --- packages/react-reconciler/src/ReactFiberHooks.js | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 13584c673eb91..0ce14ebbc6963 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -829,7 +829,7 @@ function renderWithHooksAgain( workInProgressHook = null; if (workInProgress.updateQueue != null) { - clearFunctionComponentUpdateQueue(workInProgress.updateQueue); + resetFunctionComponentUpdateQueue(workInProgress.updateQueue); } if (__DEV__) { @@ -1103,10 +1103,7 @@ if (enableUseMemoCacheHook) { }; } -// NOTE: this function intentionally does not reset memoCache data. We reuse updateQueue for the memo -// cache to avoid increasing the size of fibers that don't need a cache, but we don't want to reset -// the cache when other properties are reset. -const clearFunctionComponentUpdateQueue = ( +const resetFunctionComponentUpdateQueue = ( updateQueue: FunctionComponentUpdateQueue, ) => { updateQueue.lastEffect = null; @@ -1114,6 +1111,9 @@ const clearFunctionComponentUpdateQueue = ( updateQueue.stores = null; if (enableUseMemoCacheHook) { if (updateQueue.memoCache != null) { + // NOTE: this function intentionally does not reset memoCache data. We reuse updateQueue for the memo + // cache to avoid increasing the size of fibers that don't need a cache, but we don't want to reset + // the cache when other properties are reset. updateQueue.memoCache.index = 0; } } From 87c4c0d7ed205523b1e7f4fa09622b17b209e58a Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 5 Sep 2024 16:06:04 -0700 Subject: [PATCH 05/12] Update on "Fix useMemoCache with setState in render" Fixes the bug that alexmckenley and mofeiZ found where setState-in-render can reset useMemoCache and cause an infinite loop. The bug was that renderWithHooksAgain() was not resetting hook state when rerendering (so useMemo values were preserved) but was resetting the updateQueue. This meant that the entire memo cache was cleared on a setState-in-render. The fix here is to call a new helper function to clear the update queue. It nulls out other properties, but for memoCache it just sets the index back to zero. [ghstack-poisoned] --- packages/react-reconciler/src/ReactFiberHooks.js | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 0ce14ebbc6963..04bcd2843ea99 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -1103,9 +1103,9 @@ if (enableUseMemoCacheHook) { }; } -const resetFunctionComponentUpdateQueue = ( +function resetFunctionComponentUpdateQueue( updateQueue: FunctionComponentUpdateQueue, -) => { +): void { updateQueue.lastEffect = null; updateQueue.events = null; updateQueue.stores = null; @@ -1117,7 +1117,7 @@ const resetFunctionComponentUpdateQueue = ( updateQueue.memoCache.index = 0; } } -}; +} function useThenable(thenable: Thenable): T { // Track the position of the thenable within this fiber. From a82868f0c4c6f7e19ebe705e870d2d05fc730537 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 5 Sep 2024 16:08:17 -0700 Subject: [PATCH 06/12] Update on "Fix useMemoCache with setState in render" Fixes the bug that alexmckenley and mofeiZ found where setState-in-render can reset useMemoCache and cause an infinite loop. The bug was that renderWithHooksAgain() was not resetting hook state when rerendering (so useMemo values were preserved) but was resetting the updateQueue. This meant that the entire memo cache was cleared on a setState-in-render. The fix here is to call a new helper function to clear the update queue. It nulls out other properties, but for memoCache it just sets the index back to zero. [ghstack-poisoned] --- packages/react-reconciler/src/__tests__/useMemoCache-test.js | 2 -- 1 file changed, 2 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/useMemoCache-test.js b/packages/react-reconciler/src/__tests__/useMemoCache-test.js index 8e67e79a1a722..d5008fbae6c06 100644 --- a/packages/react-reconciler/src/__tests__/useMemoCache-test.js +++ b/packages/react-reconciler/src/__tests__/useMemoCache-test.js @@ -16,7 +16,6 @@ let assertLog; let useMemo; let useState; let useMemoCache; -let waitForThrow; let MemoCacheSentinel; let ErrorBoundary; @@ -32,7 +31,6 @@ describe('useMemoCache()', () => { useMemo = React.useMemo; useMemoCache = require('react/compiler-runtime').c; useState = React.useState; - waitForThrow = require('internal-test-utils').waitForThrow; MemoCacheSentinel = Symbol.for('react.memo_cache_sentinel'); class _ErrorBoundary extends React.Component { From 837cd09e69d78b68bc027d82ef8ddbe36a0fe374 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Thu, 5 Sep 2024 16:46:54 -0700 Subject: [PATCH 07/12] Update enableNoCloningMemoCache test to reflect updated reset behavior on "Fix useMemoCache with setState in render" Fixes the bug that alexmckenley and mofeiZ found where setState-in-render can reset useMemoCache and cause an infinite loop. The bug was that renderWithHooksAgain() was not resetting hook state when rerendering (so useMemo values were preserved) but was resetting the updateQueue. This meant that the entire memo cache was cleared on a setState-in-render. The fix here is to call a new helper function to clear the update queue. It nulls out other properties, but for memoCache it just sets the index back to zero. [ghstack-poisoned] --- .../src/__tests__/useMemoCache-test.js | 15 +++------------ 1 file changed, 3 insertions(+), 12 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/useMemoCache-test.js b/packages/react-reconciler/src/__tests__/useMemoCache-test.js index d5008fbae6c06..e72106493cf13 100644 --- a/packages/react-reconciler/src/__tests__/useMemoCache-test.js +++ b/packages/react-reconciler/src/__tests__/useMemoCache-test.js @@ -628,18 +628,9 @@ describe('useMemoCache()', () => { // Finish loading the data. await act(() => updatedChunkB.resolve('B2')); - if (gate(flags => flags.enableNoCloningMemoCache)) { - // We did not have process the first chunk again. We reused the - // computation from the earlier attempt. - assertLog([]); - } else { - // Because we clone/reset the memo cache after every aborted attempt, we - // must process the first chunk again. - // - // That's three total times we've processed the first chunk, compared to - // just once when enableNoCloningMemoCache is on. - assertLog(['Some expensive processing... [A2]']); - } + // We did not have process the first chunk again. We reused the + // computation from the earlier attempt. + assertLog([]); expect(root).toMatchRenderedOutput( <>
Input: hi!
From 637b9331ffb69416f9852472993f9ad937354994 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 6 Sep 2024 08:46:04 -0700 Subject: [PATCH 08/12] Update on "Fix useMemoCache with setState in render" Fixes the bug that alexmckenley and mofeiZ found where setState-in-render can reset useMemoCache and cause an infinite loop. The bug was that renderWithHooksAgain() was not resetting hook state when rerendering (so useMemo values were preserved) but was resetting the updateQueue. This meant that the entire memo cache was cleared on a setState-in-render. The fix here is to call a new helper function to clear the update queue. It nulls out other properties, but for memoCache it just sets the index back to zero. [ghstack-poisoned] --- .../react-reconciler/src/ReactFiberHooks.js | 4 ++- .../src/__tests__/useMemoCache-test.js | 33 +++++-------------- 2 files changed, 11 insertions(+), 26 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 04bcd2843ea99..487bd07344484 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -522,7 +522,9 @@ export function renderWithHooks( } workInProgress.memoizedState = null; - workInProgress.updateQueue = null; + if (workInProgress.updateQueue != null) { + resetFunctionComponentUpdateQueue(workInProgress.updateQueue); + } workInProgress.lanes = NoLanes; // The following should have already been reset diff --git a/packages/react-reconciler/src/__tests__/useMemoCache-test.js b/packages/react-reconciler/src/__tests__/useMemoCache-test.js index e72106493cf13..c304c90b7ea3a 100644 --- a/packages/react-reconciler/src/__tests__/useMemoCache-test.js +++ b/packages/react-reconciler/src/__tests__/useMemoCache-test.js @@ -575,11 +575,7 @@ describe('useMemoCache()', () => { 'Some expensive processing... [A2]', 'Suspend! [chunkB]', - ...(gate('enableSiblingPrerendering') - ? gate('enableNoCloningMemoCache') - ? ['Suspend! [chunkB]'] - : ['Some expensive processing... [A2]', 'Suspend! [chunkB]'] - : []), + ...(gate('enableSiblingPrerendering') ? ['Suspend! [chunkB]'] : []), ]); // The second chunk hasn't loaded yet, so we're still showing the @@ -598,26 +594,13 @@ describe('useMemoCache()', () => { await act(() => setInput('hi!')); // Once the input has updated, we go back to rendering the transition. - if (gate(flags => flags.enableNoCloningMemoCache)) { - // We did not have process the first chunk again. We reused the - // computation from the earlier attempt. - assertLog([ - 'Suspend! [chunkB]', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [chunkB]'] : []), - ]); - } else { - // Because we clone/reset the memo cache after every aborted attempt, we - // must process the first chunk again. - assertLog([ - 'Some expensive processing... [A2]', - 'Suspend! [chunkB]', - - ...(gate('enableSiblingPrerendering') - ? ['Some expensive processing... [A2]', 'Suspend! [chunkB]'] - : []), - ]); - } + // We did not have process the first chunk again. We reused the + // computation from the earlier attempt. + assertLog([ + 'Suspend! [chunkB]', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [chunkB]'] : []), + ]); expect(root).toMatchRenderedOutput( <> From 539d4071f85828f8f34dcc4c9e6bc469ed59c77d Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 6 Sep 2024 14:04:57 -0700 Subject: [PATCH 09/12] Update on "Fix useMemoCache with setState in render" Fixes the bug that alexmckenley and mofeiZ found where setState-in-render can reset useMemoCache and cause an infinite loop. The bug was that renderWithHooksAgain() was not resetting hook state when rerendering (so useMemo values were preserved) but was resetting the updateQueue. This meant that the entire memo cache was cleared on a setState-in-render. The fix here is to call a new helper function to clear the update queue. It nulls out other properties, but for memoCache it just sets the index back to zero. [ghstack-poisoned] --- .../react-reconciler/src/ReactFiberHooks.js | 21 ++++----- .../src/__tests__/useMemoCache-test.js | 44 ++++++++++++++++--- 2 files changed, 46 insertions(+), 19 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 487bd07344484..757a0e086d90f 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -522,9 +522,7 @@ export function renderWithHooks( } workInProgress.memoizedState = null; - if (workInProgress.updateQueue != null) { - resetFunctionComponentUpdateQueue(workInProgress.updateQueue); - } + workInProgress.updateQueue = null; workInProgress.lanes = NoLanes; // The following should have already been reset @@ -772,6 +770,7 @@ export function replaySuspendedComponentWithHooks( ignorePreviousDependencies = current !== null && current.type !== workInProgress.type; } + workInProgress.updateQueue = null; const children = renderWithHooksAgain( workInProgress, Component, @@ -2516,17 +2515,15 @@ function pushEffect( if (componentUpdateQueue === null) { componentUpdateQueue = createFunctionComponentUpdateQueue(); currentlyRenderingFiber.updateQueue = (componentUpdateQueue: any); + } + const lastEffect = componentUpdateQueue.lastEffect; + if (lastEffect === null) { componentUpdateQueue.lastEffect = effect.next = effect; } else { - const lastEffect = componentUpdateQueue.lastEffect; - if (lastEffect === null) { - componentUpdateQueue.lastEffect = effect.next = effect; - } else { - const firstEffect = lastEffect.next; - lastEffect.next = effect; - effect.next = firstEffect; - componentUpdateQueue.lastEffect = effect; - } + const firstEffect = lastEffect.next; + lastEffect.next = effect; + effect.next = firstEffect; + componentUpdateQueue.lastEffect = effect; } return effect; } diff --git a/packages/react-reconciler/src/__tests__/useMemoCache-test.js b/packages/react-reconciler/src/__tests__/useMemoCache-test.js index c304c90b7ea3a..0ff935ae5b195 100644 --- a/packages/react-reconciler/src/__tests__/useMemoCache-test.js +++ b/packages/react-reconciler/src/__tests__/useMemoCache-test.js @@ -575,7 +575,11 @@ describe('useMemoCache()', () => { 'Some expensive processing... [A2]', 'Suspend! [chunkB]', - ...(gate('enableSiblingPrerendering') ? ['Suspend! [chunkB]'] : []), + ...(gate('enableSiblingPrerendering') + ? gate('enableNoCloningMemoCache') + ? ['Suspend! [chunkB]'] + : ['Some expensive processing... [A2]', 'Suspend! [chunkB]'] + : []), ]); // The second chunk hasn't loaded yet, so we're still showing the @@ -596,11 +600,26 @@ describe('useMemoCache()', () => { // Once the input has updated, we go back to rendering the transition. // We did not have process the first chunk again. We reused the // computation from the earlier attempt. - assertLog([ - 'Suspend! [chunkB]', - - ...(gate('enableSiblingPrerendering') ? ['Suspend! [chunkB]'] : []), - ]); + if (gate(flags => flags.enableNoCloningMemoCache)) { + // We did not have process the first chunk again. We reused the + // computation from the earlier attempt. + assertLog([ + 'Suspend! [chunkB]', + + ...(gate('enableSiblingPrerendering') ? ['Suspend! [chunkB]'] : []), + ]); + } else { + // Because we clone/reset the memo cache after every aborted attempt, we + // must process the first chunk again. + assertLog([ + 'Some expensive processing... [A2]', + 'Suspend! [chunkB]', + + ...(gate('enableSiblingPrerendering') + ? ['Some expensive processing... [A2]', 'Suspend! [chunkB]'] + : []), + ]); + } expect(root).toMatchRenderedOutput( <> @@ -613,7 +632,18 @@ describe('useMemoCache()', () => { await act(() => updatedChunkB.resolve('B2')); // We did not have process the first chunk again. We reused the // computation from the earlier attempt. - assertLog([]); + if (gate(flags => flags.enableNoCloningMemoCache)) { + // We did not have process the first chunk again. We reused the + // computation from the earlier attempt. + assertLog([]); + } else { + // Because we clone/reset the memo cache after every aborted attempt, we + // must process the first chunk again. + // + // That's three total times we've processed the first chunk, compared to + // just once when enableNoCloningMemoCache is on. + assertLog(['Some expensive processing... [A2]']); + } expect(root).toMatchRenderedOutput( <>
Input: hi!
From f6803f59f0b0490a75a8177059d71e59d07c73c0 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 6 Sep 2024 14:06:44 -0700 Subject: [PATCH 10/12] Update on "Fix useMemoCache with setState in render" Fixes the bug that alexmckenley and mofeiZ found where setState-in-render can reset useMemoCache and cause an infinite loop. The bug was that renderWithHooksAgain() was not resetting hook state when rerendering (so useMemo values were preserved) but was resetting the updateQueue. This meant that the entire memo cache was cleared on a setState-in-render. The fix here is to call a new helper function to clear the update queue. It nulls out other properties, but for memoCache it just sets the index back to zero. [ghstack-poisoned] --- packages/react-reconciler/src/ReactFiberHooks.js | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 757a0e086d90f..5fc5473a61cf2 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -770,6 +770,11 @@ export function replaySuspendedComponentWithHooks( ignorePreviousDependencies = current !== null && current.type !== workInProgress.type; } + // renderWithHooks only resets the updateQueue but does not clear it, since + // it needs to work for both this case (suspense replay) as well as for double + // renders in dev and setState-in-render. However, for the suspense replay case + // we need to reset the updateQueue to correctly handle unmount effects, so we + // clear the queue here workInProgress.updateQueue = null; const children = renderWithHooksAgain( workInProgress, From b8da0e6c791ca835b6ba4f24531ece54cb1e8571 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 6 Sep 2024 14:07:20 -0700 Subject: [PATCH 11/12] Update on "Fix useMemoCache with setState in render" Fixes the bug that alexmckenley and mofeiZ found where setState-in-render can reset useMemoCache and cause an infinite loop. The bug was that renderWithHooksAgain() was not resetting hook state when rerendering (so useMemo values were preserved) but was resetting the updateQueue. This meant that the entire memo cache was cleared on a setState-in-render. The fix here is to call a new helper function to clear the update queue. It nulls out other properties, but for memoCache it just sets the index back to zero. [ghstack-poisoned] --- packages/react-reconciler/src/__tests__/useMemoCache-test.js | 4 ---- 1 file changed, 4 deletions(-) diff --git a/packages/react-reconciler/src/__tests__/useMemoCache-test.js b/packages/react-reconciler/src/__tests__/useMemoCache-test.js index 0ff935ae5b195..d5008fbae6c06 100644 --- a/packages/react-reconciler/src/__tests__/useMemoCache-test.js +++ b/packages/react-reconciler/src/__tests__/useMemoCache-test.js @@ -598,8 +598,6 @@ describe('useMemoCache()', () => { await act(() => setInput('hi!')); // Once the input has updated, we go back to rendering the transition. - // We did not have process the first chunk again. We reused the - // computation from the earlier attempt. if (gate(flags => flags.enableNoCloningMemoCache)) { // We did not have process the first chunk again. We reused the // computation from the earlier attempt. @@ -630,8 +628,6 @@ describe('useMemoCache()', () => { // Finish loading the data. await act(() => updatedChunkB.resolve('B2')); - // We did not have process the first chunk again. We reused the - // computation from the earlier attempt. if (gate(flags => flags.enableNoCloningMemoCache)) { // We did not have process the first chunk again. We reused the // computation from the earlier attempt. From 03000dd721570e2462293f81c133c993968f40f2 Mon Sep 17 00:00:00 2001 From: Joe Savona Date: Fri, 6 Sep 2024 14:10:56 -0700 Subject: [PATCH 12/12] suppress error for untyped updateQueue on "Fix useMemoCache with setState in render" Fixes the bug that alexmckenley and mofeiZ found where setState-in-render can reset useMemoCache and cause an infinite loop. The bug was that renderWithHooksAgain() was not resetting hook state when rerendering (so useMemo values were preserved) but was resetting the updateQueue. This meant that the entire memo cache was cleared on a setState-in-render. The fix here is to call a new helper function to clear the update queue. It nulls out other properties, but for memoCache it just sets the index back to zero. [ghstack-poisoned] --- packages/react-reconciler/src/ReactFiberHooks.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/react-reconciler/src/ReactFiberHooks.js b/packages/react-reconciler/src/ReactFiberHooks.js index 5fc5473a61cf2..ca02af3393102 100644 --- a/packages/react-reconciler/src/ReactFiberHooks.js +++ b/packages/react-reconciler/src/ReactFiberHooks.js @@ -835,7 +835,7 @@ function renderWithHooksAgain( workInProgressHook = null; if (workInProgress.updateQueue != null) { - resetFunctionComponentUpdateQueue(workInProgress.updateQueue); + resetFunctionComponentUpdateQueue((workInProgress.updateQueue: any)); } if (__DEV__) {