From d6dc2029a99384cc89ca319fb39d3e19a3a3160f Mon Sep 17 00:00:00 2001 From: overlookmotel Date: Wed, 26 Aug 2020 20:10:03 +0100 Subject: [PATCH 1/7] Failing test for Context.Consumer in suspended Suspense See issue #19701. --- .../__tests__/ReactSuspense-test.internal.js | 52 +++++++++++++++++++ 1 file changed, 52 insertions(+) diff --git a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js index c11dcfe692922..45640af935916 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -1532,5 +1532,57 @@ describe('ReactSuspense', () => { expect(Scheduler).toFlushUntilNextPaint(['new value']); expect(root).toMatchRenderedOutput('new value'); }); + + it('updates context consumer within child of suspended suspense component when context updates', () => { + const {createContext, useState} = React; + + const ValueContext = createContext(null); + + const promiseThatNeverResolves = new Promise(() => {}); + function Child() { + return ( + + {value => { + Scheduler.unstable_yieldValue(`Received context value [${value}]`); + if (value === 'default') return ; + throw promiseThatNeverResolves; + }} + + ); + } + + let setValue; + function Wrapper({children}) { + const [value, _setValue] = useState('default'); + setValue = _setValue; + return ( + + {children} + + ); + } + + function App() { + return ( + + }> + + + + ); + } + + const root = ReactTestRenderer.create(); + expect(Scheduler).toHaveYielded(['Received context value [default]', 'default']); + expect(root).toMatchRenderedOutput('default'); + + act(() => setValue('new value')); + expect(Scheduler).toHaveYielded(['Received context value [new value]', 'Loading...']); + expect(root).toMatchRenderedOutput('Loading...'); + + act(() => setValue('default')); + expect(Scheduler).toHaveYielded(['Received context value [default]', 'default']); + expect(root).toMatchRenderedOutput('default'); + }); }); }); From 0ce097b413bfae2c9eb22742939d24be732c9dae Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 11 Jan 2022 19:07:42 +0000 Subject: [PATCH 2/7] Fix context propagation for offscreen trees --- .../src/ReactFiberBeginWork.new.js | 2 +- .../src/ReactFiberBeginWork.old.js | 2 +- .../src/ReactFiberNewContext.new.js | 22 ++++++++++++++----- .../src/ReactFiberNewContext.old.js | 22 ++++++++++++++----- .../__tests__/ReactSuspense-test.internal.js | 19 ++++++++++++---- 5 files changed, 51 insertions(+), 16 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 3e344d33261e8..d42fab5c03208 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -2760,7 +2760,7 @@ function scheduleWorkOnFiber(fiber: Fiber, renderLanes: Lanes) { if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleWorkOnParentPath(fiber.return, renderLanes); + scheduleWorkOnParentPath(fiber.return, renderLanes, null); } function propagateSuspenseContextChange( diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index 5192d6b2fc99d..e367704f86fff 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -2760,7 +2760,7 @@ function scheduleWorkOnFiber(fiber: Fiber, renderLanes: Lanes) { if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleWorkOnParentPath(fiber.return, renderLanes); + scheduleWorkOnParentPath(fiber.return, renderLanes, null); } function propagateSuspenseContextChange( diff --git a/packages/react-reconciler/src/ReactFiberNewContext.new.js b/packages/react-reconciler/src/ReactFiberNewContext.new.js index 2f59cbec27f29..05d4de5917a33 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.new.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.new.js @@ -141,9 +141,11 @@ export function popProvider( export function scheduleWorkOnParentPath( parent: Fiber | null, renderLanes: Lanes, + stopAt: Fiber | null, ) { // Update the child lanes of all the ancestors, including the alternates. let node = parent; + const stopAtAlternate = stopAt !== null ? stopAt.alternate : null; while (node !== null) { const alternate = node.alternate; if (!isSubsetOfLanes(node.childLanes, renderLanes)) { @@ -157,8 +159,14 @@ export function scheduleWorkOnParentPath( ) { alternate.childLanes = mergeLanes(alternate.childLanes, renderLanes); } else { - // Neither alternate was updated, which means the rest of the + // Neither alternate was updated. + // Normally, this would mean that the rest of the // ancestor path already has sufficient priority. + // However, this is not necessarily true inside offscreen + // or fallback trees because childLanes may be inconsistent + // with the surroundings. This is why we continue the loop. + } + if (node === stopAt || node === stopAtAlternate) { break; } node = node.return; @@ -246,7 +254,7 @@ function propagateContextChange_eager( if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleWorkOnParentPath(fiber.return, renderLanes); + scheduleWorkOnParentPath(fiber.return, renderLanes, workInProgress); // Mark the updated lanes on the list, too. list.lanes = mergeLanes(list.lanes, renderLanes); @@ -284,7 +292,7 @@ function propagateContextChange_eager( // because we want to schedule this fiber as having work // on its children. We'll use the childLanes on // this fiber to indicate that a context has changed. - scheduleWorkOnParentPath(parentSuspense, renderLanes); + scheduleWorkOnParentPath(parentSuspense, renderLanes, workInProgress); nextFiber = fiber.sibling; } else { // Traverse down. @@ -365,7 +373,11 @@ function propagateContextChanges( if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleWorkOnParentPath(consumer.return, renderLanes); + scheduleWorkOnParentPath( + consumer.return, + renderLanes, + workInProgress, + ); if (!forcePropagateEntireTree) { // During lazy propagation, when we find a match, we can defer @@ -406,7 +418,7 @@ function propagateContextChanges( // because we want to schedule this fiber as having work // on its children. We'll use the childLanes on // this fiber to indicate that a context has changed. - scheduleWorkOnParentPath(parentSuspense, renderLanes); + scheduleWorkOnParentPath(parentSuspense, renderLanes, workInProgress); nextFiber = null; } else { // Traverse down. diff --git a/packages/react-reconciler/src/ReactFiberNewContext.old.js b/packages/react-reconciler/src/ReactFiberNewContext.old.js index 0d492397afd8e..ff4f3ece45175 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.old.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.old.js @@ -141,9 +141,11 @@ export function popProvider( export function scheduleWorkOnParentPath( parent: Fiber | null, renderLanes: Lanes, + stopAt: Fiber | null, ) { // Update the child lanes of all the ancestors, including the alternates. let node = parent; + const stopAtAlternate = stopAt !== null ? stopAt.alternate : null; while (node !== null) { const alternate = node.alternate; if (!isSubsetOfLanes(node.childLanes, renderLanes)) { @@ -157,8 +159,14 @@ export function scheduleWorkOnParentPath( ) { alternate.childLanes = mergeLanes(alternate.childLanes, renderLanes); } else { - // Neither alternate was updated, which means the rest of the + // Neither alternate was updated. + // Normally, this would mean that the rest of the // ancestor path already has sufficient priority. + // However, this is not necessarily true inside offscreen + // or fallback trees because childLanes may be inconsistent + // with the surroundings. This is why we continue the loop. + } + if (node === stopAt || node === stopAtAlternate) { break; } node = node.return; @@ -246,7 +254,7 @@ function propagateContextChange_eager( if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleWorkOnParentPath(fiber.return, renderLanes); + scheduleWorkOnParentPath(fiber.return, renderLanes, workInProgress); // Mark the updated lanes on the list, too. list.lanes = mergeLanes(list.lanes, renderLanes); @@ -284,7 +292,7 @@ function propagateContextChange_eager( // because we want to schedule this fiber as having work // on its children. We'll use the childLanes on // this fiber to indicate that a context has changed. - scheduleWorkOnParentPath(parentSuspense, renderLanes); + scheduleWorkOnParentPath(parentSuspense, renderLanes, workInProgress); nextFiber = fiber.sibling; } else { // Traverse down. @@ -365,7 +373,11 @@ function propagateContextChanges( if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleWorkOnParentPath(consumer.return, renderLanes); + scheduleWorkOnParentPath( + consumer.return, + renderLanes, + workInProgress, + ); if (!forcePropagateEntireTree) { // During lazy propagation, when we find a match, we can defer @@ -406,7 +418,7 @@ function propagateContextChanges( // because we want to schedule this fiber as having work // on its children. We'll use the childLanes on // this fiber to indicate that a context has changed. - scheduleWorkOnParentPath(parentSuspense, renderLanes); + scheduleWorkOnParentPath(parentSuspense, renderLanes, workInProgress); nextFiber = null; } else { // Traverse down. diff --git a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js index 45640af935916..f6c1258728de8 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -1543,7 +1543,9 @@ describe('ReactSuspense', () => { return ( {value => { - Scheduler.unstable_yieldValue(`Received context value [${value}]`); + Scheduler.unstable_yieldValue( + `Received context value [${value}]`, + ); if (value === 'default') return ; throw promiseThatNeverResolves; }} @@ -1573,15 +1575,24 @@ describe('ReactSuspense', () => { } const root = ReactTestRenderer.create(); - expect(Scheduler).toHaveYielded(['Received context value [default]', 'default']); + expect(Scheduler).toHaveYielded([ + 'Received context value [default]', + 'default', + ]); expect(root).toMatchRenderedOutput('default'); act(() => setValue('new value')); - expect(Scheduler).toHaveYielded(['Received context value [new value]', 'Loading...']); + expect(Scheduler).toHaveYielded([ + 'Received context value [new value]', + 'Loading...', + ]); expect(root).toMatchRenderedOutput('Loading...'); act(() => setValue('default')); - expect(Scheduler).toHaveYielded(['Received context value [default]', 'default']); + expect(Scheduler).toHaveYielded([ + 'Received context value [default]', + 'default', + ]); expect(root).toMatchRenderedOutput('default'); }); }); From 92c5c04472be41603624b2cc74ec9be961e48305 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 18 Jan 2022 18:53:49 +0000 Subject: [PATCH 3/7] Address nits --- .../src/ReactFiberBeginWork.new.js | 10 +++--- .../src/ReactFiberBeginWork.old.js | 10 +++--- .../src/ReactFiberNewContext.new.js | 35 ++++++++++++++----- .../src/ReactFiberNewContext.old.js | 35 ++++++++++++++----- 4 files changed, 64 insertions(+), 26 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index d42fab5c03208..cdbb5352eaa6b 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -173,7 +173,7 @@ import { checkIfContextChanged, readContext, prepareToReadContext, - scheduleWorkOnParentPath, + scheduleContextWorkOnParentPath, } from './ReactFiberNewContext.new'; import { renderWithHooks, @@ -2754,13 +2754,13 @@ function updateDehydratedSuspenseComponent( } } -function scheduleWorkOnFiber(fiber: Fiber, renderLanes: Lanes) { +function scheduleContextWorkOnFiber(fiber: Fiber, renderLanes: Lanes) { fiber.lanes = mergeLanes(fiber.lanes, renderLanes); const alternate = fiber.alternate; if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleWorkOnParentPath(fiber.return, renderLanes, null); + scheduleContextWorkOnParentPath(fiber.return, renderLanes, null); } function propagateSuspenseContextChange( @@ -2776,7 +2776,7 @@ function propagateSuspenseContextChange( if (node.tag === SuspenseComponent) { const state: SuspenseState | null = node.memoizedState; if (state !== null) { - scheduleWorkOnFiber(node, renderLanes); + scheduleContextWorkOnFiber(node, renderLanes); } } else if (node.tag === SuspenseListComponent) { // If the tail is hidden there might not be an Suspense boundaries @@ -2784,7 +2784,7 @@ function propagateSuspenseContextChange( // list itself. // We don't have to traverse to the children of the list since // the list will propagate the change when it rerenders. - scheduleWorkOnFiber(node, renderLanes); + scheduleContextWorkOnFiber(node, renderLanes); } else if (node.child !== null) { node.child.return = node; node = node.child; diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index e367704f86fff..379245ebdfbc7 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -173,7 +173,7 @@ import { checkIfContextChanged, readContext, prepareToReadContext, - scheduleWorkOnParentPath, + scheduleContextWorkOnParentPath, } from './ReactFiberNewContext.old'; import { renderWithHooks, @@ -2754,13 +2754,13 @@ function updateDehydratedSuspenseComponent( } } -function scheduleWorkOnFiber(fiber: Fiber, renderLanes: Lanes) { +function scheduleContextWorkOnFiber(fiber: Fiber, renderLanes: Lanes) { fiber.lanes = mergeLanes(fiber.lanes, renderLanes); const alternate = fiber.alternate; if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleWorkOnParentPath(fiber.return, renderLanes, null); + scheduleContextWorkOnParentPath(fiber.return, renderLanes, null); } function propagateSuspenseContextChange( @@ -2776,7 +2776,7 @@ function propagateSuspenseContextChange( if (node.tag === SuspenseComponent) { const state: SuspenseState | null = node.memoizedState; if (state !== null) { - scheduleWorkOnFiber(node, renderLanes); + scheduleContextWorkOnFiber(node, renderLanes); } } else if (node.tag === SuspenseListComponent) { // If the tail is hidden there might not be an Suspense boundaries @@ -2784,7 +2784,7 @@ function propagateSuspenseContextChange( // list itself. // We don't have to traverse to the children of the list since // the list will propagate the change when it rerenders. - scheduleWorkOnFiber(node, renderLanes); + scheduleContextWorkOnFiber(node, renderLanes); } else if (node.child !== null) { node.child.return = node; node = node.child; diff --git a/packages/react-reconciler/src/ReactFiberNewContext.new.js b/packages/react-reconciler/src/ReactFiberNewContext.new.js index 05d4de5917a33..67eeac88a3cd6 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.new.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.new.js @@ -138,14 +138,13 @@ export function popProvider( } } -export function scheduleWorkOnParentPath( +export function scheduleContextWorkOnParentPath( parent: Fiber | null, renderLanes: Lanes, - stopAt: Fiber | null, + propagationRoot: Fiber | null, ) { // Update the child lanes of all the ancestors, including the alternates. let node = parent; - const stopAtAlternate = stopAt !== null ? stopAt.alternate : null; while (node !== null) { const alternate = node.alternate; if (!isSubsetOfLanes(node.childLanes, renderLanes)) { @@ -166,9 +165,17 @@ export function scheduleWorkOnParentPath( // or fallback trees because childLanes may be inconsistent // with the surroundings. This is why we continue the loop. } - if (node === stopAt || node === stopAtAlternate) { + if (node === propagationRoot) { break; } + if (__DEV__) { + if (propagationRoot !== null && node === propagationRoot.alternate) { + console.error( + 'Did not expect to encounter a propagation root alternate when scheduling context work. ' + + 'This error is likely caused by a bug in React. Please file an issue.', + ); + } + } node = node.return; } } @@ -254,7 +261,11 @@ function propagateContextChange_eager( if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleWorkOnParentPath(fiber.return, renderLanes, workInProgress); + scheduleContextWorkOnParentPath( + fiber.return, + renderLanes, + workInProgress, + ); // Mark the updated lanes on the list, too. list.lanes = mergeLanes(list.lanes, renderLanes); @@ -292,7 +303,11 @@ function propagateContextChange_eager( // because we want to schedule this fiber as having work // on its children. We'll use the childLanes on // this fiber to indicate that a context has changed. - scheduleWorkOnParentPath(parentSuspense, renderLanes, workInProgress); + scheduleContextWorkOnParentPath( + parentSuspense, + renderLanes, + workInProgress, + ); nextFiber = fiber.sibling; } else { // Traverse down. @@ -373,7 +388,7 @@ function propagateContextChanges( if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleWorkOnParentPath( + scheduleContextWorkOnParentPath( consumer.return, renderLanes, workInProgress, @@ -418,7 +433,11 @@ function propagateContextChanges( // because we want to schedule this fiber as having work // on its children. We'll use the childLanes on // this fiber to indicate that a context has changed. - scheduleWorkOnParentPath(parentSuspense, renderLanes, workInProgress); + scheduleContextWorkOnParentPath( + parentSuspense, + renderLanes, + workInProgress, + ); nextFiber = null; } else { // Traverse down. diff --git a/packages/react-reconciler/src/ReactFiberNewContext.old.js b/packages/react-reconciler/src/ReactFiberNewContext.old.js index ff4f3ece45175..aac18822c28bb 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.old.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.old.js @@ -138,14 +138,13 @@ export function popProvider( } } -export function scheduleWorkOnParentPath( +export function scheduleContextWorkOnParentPath( parent: Fiber | null, renderLanes: Lanes, - stopAt: Fiber | null, + propagationRoot: Fiber | null, ) { // Update the child lanes of all the ancestors, including the alternates. let node = parent; - const stopAtAlternate = stopAt !== null ? stopAt.alternate : null; while (node !== null) { const alternate = node.alternate; if (!isSubsetOfLanes(node.childLanes, renderLanes)) { @@ -166,9 +165,17 @@ export function scheduleWorkOnParentPath( // or fallback trees because childLanes may be inconsistent // with the surroundings. This is why we continue the loop. } - if (node === stopAt || node === stopAtAlternate) { + if (node === propagationRoot) { break; } + if (__DEV__) { + if (propagationRoot !== null && node === propagationRoot.alternate) { + console.error( + 'Did not expect to encounter a propagation root alternate when scheduling context work. ' + + 'This error is likely caused by a bug in React. Please file an issue.', + ); + } + } node = node.return; } } @@ -254,7 +261,11 @@ function propagateContextChange_eager( if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleWorkOnParentPath(fiber.return, renderLanes, workInProgress); + scheduleContextWorkOnParentPath( + fiber.return, + renderLanes, + workInProgress, + ); // Mark the updated lanes on the list, too. list.lanes = mergeLanes(list.lanes, renderLanes); @@ -292,7 +303,11 @@ function propagateContextChange_eager( // because we want to schedule this fiber as having work // on its children. We'll use the childLanes on // this fiber to indicate that a context has changed. - scheduleWorkOnParentPath(parentSuspense, renderLanes, workInProgress); + scheduleContextWorkOnParentPath( + parentSuspense, + renderLanes, + workInProgress, + ); nextFiber = fiber.sibling; } else { // Traverse down. @@ -373,7 +388,7 @@ function propagateContextChanges( if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleWorkOnParentPath( + scheduleContextWorkOnParentPath( consumer.return, renderLanes, workInProgress, @@ -418,7 +433,11 @@ function propagateContextChanges( // because we want to schedule this fiber as having work // on its children. We'll use the childLanes on // this fiber to indicate that a context has changed. - scheduleWorkOnParentPath(parentSuspense, renderLanes, workInProgress); + scheduleContextWorkOnParentPath( + parentSuspense, + renderLanes, + workInProgress, + ); nextFiber = null; } else { // Traverse down. From 7d93066ce1dd16092ee70035abc67612dec0ba46 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Tue, 18 Jan 2022 19:05:50 +0000 Subject: [PATCH 4/7] Specify propagation root for Suspense too --- .../react-reconciler/src/ReactFiberBeginWork.new.js | 10 ++++++---- .../react-reconciler/src/ReactFiberBeginWork.old.js | 10 ++++++---- .../react-reconciler/src/ReactFiberNewContext.new.js | 4 ++-- .../react-reconciler/src/ReactFiberNewContext.old.js | 4 ++-- 4 files changed, 16 insertions(+), 12 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index cdbb5352eaa6b..fdd2398ebc955 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -2754,13 +2754,15 @@ function updateDehydratedSuspenseComponent( } } -function scheduleContextWorkOnFiber(fiber: Fiber, renderLanes: Lanes) { +function scheduleSuspenseWorkOnFiber(fiber: Fiber, renderLanes: Lanes) { fiber.lanes = mergeLanes(fiber.lanes, renderLanes); const alternate = fiber.alternate; if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleContextWorkOnParentPath(fiber.return, renderLanes, null); + // Guaranteed to be non-empty since the fiber is not a root. + const parentFiber: Fiber = (fiber.return: any); + scheduleContextWorkOnParentPath(parentFiber, renderLanes, parentFiber); } function propagateSuspenseContextChange( @@ -2776,7 +2778,7 @@ function propagateSuspenseContextChange( if (node.tag === SuspenseComponent) { const state: SuspenseState | null = node.memoizedState; if (state !== null) { - scheduleContextWorkOnFiber(node, renderLanes); + scheduleSuspenseWorkOnFiber(node, renderLanes); } } else if (node.tag === SuspenseListComponent) { // If the tail is hidden there might not be an Suspense boundaries @@ -2784,7 +2786,7 @@ function propagateSuspenseContextChange( // list itself. // We don't have to traverse to the children of the list since // the list will propagate the change when it rerenders. - scheduleContextWorkOnFiber(node, renderLanes); + scheduleSuspenseWorkOnFiber(node, renderLanes); } else if (node.child !== null) { node.child.return = node; node = node.child; diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index 379245ebdfbc7..1073ecd7d5f26 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -2754,13 +2754,15 @@ function updateDehydratedSuspenseComponent( } } -function scheduleContextWorkOnFiber(fiber: Fiber, renderLanes: Lanes) { +function scheduleSuspenseWorkOnFiber(fiber: Fiber, renderLanes: Lanes) { fiber.lanes = mergeLanes(fiber.lanes, renderLanes); const alternate = fiber.alternate; if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleContextWorkOnParentPath(fiber.return, renderLanes, null); + // Guaranteed to be non-empty since the fiber is not a root. + const parentFiber: Fiber = (fiber.return: any); + scheduleContextWorkOnParentPath(parentFiber, renderLanes, parentFiber); } function propagateSuspenseContextChange( @@ -2776,7 +2778,7 @@ function propagateSuspenseContextChange( if (node.tag === SuspenseComponent) { const state: SuspenseState | null = node.memoizedState; if (state !== null) { - scheduleContextWorkOnFiber(node, renderLanes); + scheduleSuspenseWorkOnFiber(node, renderLanes); } } else if (node.tag === SuspenseListComponent) { // If the tail is hidden there might not be an Suspense boundaries @@ -2784,7 +2786,7 @@ function propagateSuspenseContextChange( // list itself. // We don't have to traverse to the children of the list since // the list will propagate the change when it rerenders. - scheduleContextWorkOnFiber(node, renderLanes); + scheduleSuspenseWorkOnFiber(node, renderLanes); } else if (node.child !== null) { node.child.return = node; node = node.child; diff --git a/packages/react-reconciler/src/ReactFiberNewContext.new.js b/packages/react-reconciler/src/ReactFiberNewContext.new.js index 67eeac88a3cd6..996e463cd7480 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.new.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.new.js @@ -141,7 +141,7 @@ export function popProvider( export function scheduleContextWorkOnParentPath( parent: Fiber | null, renderLanes: Lanes, - propagationRoot: Fiber | null, + propagationRoot: Fiber, ) { // Update the child lanes of all the ancestors, including the alternates. let node = parent; @@ -169,7 +169,7 @@ export function scheduleContextWorkOnParentPath( break; } if (__DEV__) { - if (propagationRoot !== null && node === propagationRoot.alternate) { + if (node === propagationRoot.alternate) { console.error( 'Did not expect to encounter a propagation root alternate when scheduling context work. ' + 'This error is likely caused by a bug in React. Please file an issue.', diff --git a/packages/react-reconciler/src/ReactFiberNewContext.old.js b/packages/react-reconciler/src/ReactFiberNewContext.old.js index aac18822c28bb..b8f6611d8d4af 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.old.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.old.js @@ -141,7 +141,7 @@ export function popProvider( export function scheduleContextWorkOnParentPath( parent: Fiber | null, renderLanes: Lanes, - propagationRoot: Fiber | null, + propagationRoot: Fiber, ) { // Update the child lanes of all the ancestors, including the alternates. let node = parent; @@ -169,7 +169,7 @@ export function scheduleContextWorkOnParentPath( break; } if (__DEV__) { - if (propagationRoot !== null && node === propagationRoot.alternate) { + if (node === propagationRoot.alternate) { console.error( 'Did not expect to encounter a propagation root alternate when scheduling context work. ' + 'This error is likely caused by a bug in React. Please file an issue.', From 09a1975ae4986c394b800f6b85e4908762c781a4 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 19 Jan 2022 01:43:42 +0000 Subject: [PATCH 5/7] Pass correct propagation root --- .../src/ReactFiberBeginWork.new.js | 14 ++++++++------ .../src/ReactFiberBeginWork.old.js | 14 ++++++++------ 2 files changed, 16 insertions(+), 12 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index fdd2398ebc955..688b67059de51 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -2754,15 +2754,17 @@ function updateDehydratedSuspenseComponent( } } -function scheduleSuspenseWorkOnFiber(fiber: Fiber, renderLanes: Lanes) { +function scheduleSuspenseWorkOnFiber( + fiber: Fiber, + renderLanes: Lanes, + propagationRoot: Fiber, +) { fiber.lanes = mergeLanes(fiber.lanes, renderLanes); const alternate = fiber.alternate; if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - // Guaranteed to be non-empty since the fiber is not a root. - const parentFiber: Fiber = (fiber.return: any); - scheduleContextWorkOnParentPath(parentFiber, renderLanes, parentFiber); + scheduleContextWorkOnParentPath(fiber.return, renderLanes, propagationRoot); } function propagateSuspenseContextChange( @@ -2778,7 +2780,7 @@ function propagateSuspenseContextChange( if (node.tag === SuspenseComponent) { const state: SuspenseState | null = node.memoizedState; if (state !== null) { - scheduleSuspenseWorkOnFiber(node, renderLanes); + scheduleSuspenseWorkOnFiber(node, renderLanes, workInProgress); } } else if (node.tag === SuspenseListComponent) { // If the tail is hidden there might not be an Suspense boundaries @@ -2786,7 +2788,7 @@ function propagateSuspenseContextChange( // list itself. // We don't have to traverse to the children of the list since // the list will propagate the change when it rerenders. - scheduleSuspenseWorkOnFiber(node, renderLanes); + scheduleSuspenseWorkOnFiber(node, renderLanes, workInProgress); } else if (node.child !== null) { node.child.return = node; node = node.child; diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index 1073ecd7d5f26..827e512c2c9f0 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -2754,15 +2754,17 @@ function updateDehydratedSuspenseComponent( } } -function scheduleSuspenseWorkOnFiber(fiber: Fiber, renderLanes: Lanes) { +function scheduleSuspenseWorkOnFiber( + fiber: Fiber, + renderLanes: Lanes, + propagationRoot: Fiber, +) { fiber.lanes = mergeLanes(fiber.lanes, renderLanes); const alternate = fiber.alternate; if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - // Guaranteed to be non-empty since the fiber is not a root. - const parentFiber: Fiber = (fiber.return: any); - scheduleContextWorkOnParentPath(parentFiber, renderLanes, parentFiber); + scheduleContextWorkOnParentPath(fiber.return, renderLanes, propagationRoot); } function propagateSuspenseContextChange( @@ -2778,7 +2780,7 @@ function propagateSuspenseContextChange( if (node.tag === SuspenseComponent) { const state: SuspenseState | null = node.memoizedState; if (state !== null) { - scheduleSuspenseWorkOnFiber(node, renderLanes); + scheduleSuspenseWorkOnFiber(node, renderLanes, workInProgress); } } else if (node.tag === SuspenseListComponent) { // If the tail is hidden there might not be an Suspense boundaries @@ -2786,7 +2788,7 @@ function propagateSuspenseContextChange( // list itself. // We don't have to traverse to the children of the list since // the list will propagate the change when it rerenders. - scheduleSuspenseWorkOnFiber(node, renderLanes); + scheduleSuspenseWorkOnFiber(node, renderLanes, workInProgress); } else if (node.child !== null) { node.child.return = node; node = node.child; From 1fed15d4fe84994f7fd2f1a7b66ba674819ce791 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 19 Jan 2022 02:09:11 +0000 Subject: [PATCH 6/7] Harden test coverage This test will fail if we remove propagation, or if we propagate with a root node like fiber.return or fiber.return.return. The additional DEV-only error helps detect a different kind of mistake, like if the thing being passed hasn't actually been encountered on the way up. However, we still leave the actual production loop to check against null so that there is no way we loop forever if the propagation root is wrong. --- .../src/ReactFiberNewContext.new.js | 8 ++ .../src/ReactFiberNewContext.old.js | 8 ++ .../src/__tests__/ReactSuspenseList-test.js | 98 +++++++++++++++++++ 3 files changed, 114 insertions(+) diff --git a/packages/react-reconciler/src/ReactFiberNewContext.new.js b/packages/react-reconciler/src/ReactFiberNewContext.new.js index 996e463cd7480..0ccf580079196 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.new.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.new.js @@ -178,6 +178,14 @@ export function scheduleContextWorkOnParentPath( } node = node.return; } + if (__DEV__) { + if (node !== propagationRoot) { + console.error( + 'Expected to find the propagation root when scheduling context work. ' + + 'This error is likely caused by a bug in React. Please file an issue.', + ); + } + } } export function propagateContextChange( diff --git a/packages/react-reconciler/src/ReactFiberNewContext.old.js b/packages/react-reconciler/src/ReactFiberNewContext.old.js index b8f6611d8d4af..258463c4d89a4 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.old.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.old.js @@ -178,6 +178,14 @@ export function scheduleContextWorkOnParentPath( } node = node.return; } + if (__DEV__) { + if (node !== propagationRoot) { + console.error( + 'Expected to find the propagation root when scheduling context work. ' + + 'This error is likely caused by a bug in React. Please file an issue.', + ); + } + } } export function propagateContextChange( diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js index 8aff360fe661a..b7cdc19d63dff 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseList-test.js @@ -2968,4 +2968,102 @@ describe('ReactSuspenseList', () => { // treeBaseDuration expect(onRender.mock.calls[3][3]).toBe(1 + 4 + 5 + 3); }); + + // @gate enableSuspenseList + it('propagates despite a memo bailout', async () => { + const A = createAsyncText('A'); + const B = createAsyncText('B'); + const C = createAsyncText('C'); + + const Bailout = React.memo(({children}) => { + return children; + }); + + function Foo() { + // To test the part that relies on context propagation, + // we need to bailout *above* the Suspense's parent. + // Several layers of Bailout wrappers help verify we're + // marking updates all the way to the propagation root. + return ( + + + + + + }> + + + + + + + + + + + }> + + + + + + + + + + + }> + + + + + + + + ); + } + + await C.resolve(); + + ReactNoop.render(); + + expect(Scheduler).toFlushAndYield([ + 'Suspend! [A]', + 'Loading A', + 'Loading B', + 'Loading C', + ]); + + expect(ReactNoop).toMatchRenderedOutput( + <> + Loading A + Loading B + Loading C + , + ); + + await A.resolve(); + + expect(Scheduler).toFlushAndYield(['A', 'Suspend! [B]']); + + expect(ReactNoop).toMatchRenderedOutput( + <> + A + Loading B + Loading C + , + ); + + await B.resolve(); + + expect(Scheduler).toFlushAndYield(['B', 'C']); + + expect(ReactNoop).toMatchRenderedOutput( + <> + A + B + C + , + ); + }); }); From d465a97289328755ed072e372f2be6c190512ae5 Mon Sep 17 00:00:00 2001 From: Dan Abramov Date: Wed, 19 Jan 2022 16:28:43 +0000 Subject: [PATCH 7/7] Remove superfluous warning --- packages/react-reconciler/src/ReactFiberNewContext.new.js | 8 -------- packages/react-reconciler/src/ReactFiberNewContext.old.js | 8 -------- 2 files changed, 16 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberNewContext.new.js b/packages/react-reconciler/src/ReactFiberNewContext.new.js index 0ccf580079196..8ff30c810f03f 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.new.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.new.js @@ -168,14 +168,6 @@ export function scheduleContextWorkOnParentPath( if (node === propagationRoot) { break; } - if (__DEV__) { - if (node === propagationRoot.alternate) { - console.error( - 'Did not expect to encounter a propagation root alternate when scheduling context work. ' + - 'This error is likely caused by a bug in React. Please file an issue.', - ); - } - } node = node.return; } if (__DEV__) { diff --git a/packages/react-reconciler/src/ReactFiberNewContext.old.js b/packages/react-reconciler/src/ReactFiberNewContext.old.js index 258463c4d89a4..93fe3bc8395c7 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.old.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.old.js @@ -168,14 +168,6 @@ export function scheduleContextWorkOnParentPath( if (node === propagationRoot) { break; } - if (__DEV__) { - if (node === propagationRoot.alternate) { - console.error( - 'Did not expect to encounter a propagation root alternate when scheduling context work. ' + - 'This error is likely caused by a bug in React. Please file an issue.', - ); - } - } node = node.return; } if (__DEV__) {