diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 3e344d33261e8..688b67059de51 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,17 @@ function updateDehydratedSuspenseComponent( } } -function scheduleWorkOnFiber(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); } - scheduleWorkOnParentPath(fiber.return, renderLanes); + scheduleContextWorkOnParentPath(fiber.return, renderLanes, propagationRoot); } function propagateSuspenseContextChange( @@ -2776,7 +2780,7 @@ function propagateSuspenseContextChange( if (node.tag === SuspenseComponent) { const state: SuspenseState | null = node.memoizedState; if (state !== null) { - scheduleWorkOnFiber(node, renderLanes); + scheduleSuspenseWorkOnFiber(node, renderLanes, workInProgress); } } else if (node.tag === SuspenseListComponent) { // If the tail is hidden there might not be an Suspense boundaries @@ -2784,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. - scheduleWorkOnFiber(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 5192d6b2fc99d..827e512c2c9f0 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,17 @@ function updateDehydratedSuspenseComponent( } } -function scheduleWorkOnFiber(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); } - scheduleWorkOnParentPath(fiber.return, renderLanes); + scheduleContextWorkOnParentPath(fiber.return, renderLanes, propagationRoot); } function propagateSuspenseContextChange( @@ -2776,7 +2780,7 @@ function propagateSuspenseContextChange( if (node.tag === SuspenseComponent) { const state: SuspenseState | null = node.memoizedState; if (state !== null) { - scheduleWorkOnFiber(node, renderLanes); + scheduleSuspenseWorkOnFiber(node, renderLanes, workInProgress); } } else if (node.tag === SuspenseListComponent) { // If the tail is hidden there might not be an Suspense boundaries @@ -2784,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. - scheduleWorkOnFiber(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/ReactFiberNewContext.new.js b/packages/react-reconciler/src/ReactFiberNewContext.new.js index 2f59cbec27f29..8ff30c810f03f 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.new.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.new.js @@ -138,9 +138,10 @@ export function popProvider( } } -export function scheduleWorkOnParentPath( +export function scheduleContextWorkOnParentPath( parent: Fiber | null, renderLanes: Lanes, + propagationRoot: Fiber, ) { // Update the child lanes of all the ancestors, including the alternates. let node = parent; @@ -157,12 +158,26 @@ 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 === propagationRoot) { break; } 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( @@ -246,7 +261,11 @@ function propagateContextChange_eager( if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleWorkOnParentPath(fiber.return, renderLanes); + scheduleContextWorkOnParentPath( + fiber.return, + renderLanes, + workInProgress, + ); // Mark the updated lanes on the list, too. list.lanes = mergeLanes(list.lanes, renderLanes); @@ -284,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); + scheduleContextWorkOnParentPath( + parentSuspense, + renderLanes, + workInProgress, + ); nextFiber = fiber.sibling; } else { // Traverse down. @@ -365,7 +388,11 @@ function propagateContextChanges( if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleWorkOnParentPath(consumer.return, renderLanes); + scheduleContextWorkOnParentPath( + consumer.return, + renderLanes, + workInProgress, + ); if (!forcePropagateEntireTree) { // During lazy propagation, when we find a match, we can defer @@ -406,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); + 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 0d492397afd8e..93fe3bc8395c7 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.old.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.old.js @@ -138,9 +138,10 @@ export function popProvider( } } -export function scheduleWorkOnParentPath( +export function scheduleContextWorkOnParentPath( parent: Fiber | null, renderLanes: Lanes, + propagationRoot: Fiber, ) { // Update the child lanes of all the ancestors, including the alternates. let node = parent; @@ -157,12 +158,26 @@ 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 === propagationRoot) { break; } 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( @@ -246,7 +261,11 @@ function propagateContextChange_eager( if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleWorkOnParentPath(fiber.return, renderLanes); + scheduleContextWorkOnParentPath( + fiber.return, + renderLanes, + workInProgress, + ); // Mark the updated lanes on the list, too. list.lanes = mergeLanes(list.lanes, renderLanes); @@ -284,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); + scheduleContextWorkOnParentPath( + parentSuspense, + renderLanes, + workInProgress, + ); nextFiber = fiber.sibling; } else { // Traverse down. @@ -365,7 +388,11 @@ function propagateContextChanges( if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleWorkOnParentPath(consumer.return, renderLanes); + scheduleContextWorkOnParentPath( + consumer.return, + renderLanes, + workInProgress, + ); if (!forcePropagateEntireTree) { // During lazy propagation, when we find a match, we can defer @@ -406,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); + scheduleContextWorkOnParentPath( + 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 c11dcfe692922..f6c1258728de8 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -1532,5 +1532,68 @@ 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'); + }); }); }); 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 + , + ); + }); });