From f55e80e2971aabd40d4d45aed68f79023accd5ea Mon Sep 17 00:00:00 2001 From: okmttdhr Date: Sat, 8 May 2021 22:30:29 +0900 Subject: [PATCH 1/4] Failing test for Context.Consumer in suspended Suspense --- .../__tests__/ReactSuspense-test.internal.js | 59 +++++++++++++++++++ 1 file changed, 59 insertions(+) diff --git a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js index e0fcab7a77cc0..29fc2bea7c6b4 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -1523,4 +1523,63 @@ describe('ReactSuspense', () => { 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 ed589927ba84cb36342918b26f7062be3d927ef9 Mon Sep 17 00:00:00 2001 From: okmttdhr Date: Sun, 9 May 2021 14:28:56 +0900 Subject: [PATCH 2/4] Schedule work to update Context.Consumer inside Suspense --- .../src/ReactFiberNewContext.new.js | 21 ++++++++++++++++++- .../src/ReactFiberNewContext.old.js | 21 ++++++++++++++++++- 2 files changed, 40 insertions(+), 2 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberNewContext.new.js b/packages/react-reconciler/src/ReactFiberNewContext.new.js index 9d2484a783971..f622b36de2cf5 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.new.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.new.js @@ -23,6 +23,7 @@ import { ContextProvider, ClassComponent, DehydratedFragment, + SuspenseComponent, } from './ReactWorkTags'; import { NoLanes, @@ -284,6 +285,14 @@ function propagateContextChange_eager( // this fiber to indicate that a context has changed. scheduleWorkOnParentPath(parentSuspense, renderLanes); nextFiber = fiber.sibling; + } else if ( + fiber.tag === SuspenseComponent && + workInProgress.tag === ContextProvider + ) { + // We don't know if it will have any context consumers in it. + // Schedule this fiber as having work on its children. + scheduleWorkOnParentPath(fiber.child, renderLanes); + nextFiber = fiber.child; } else { // Traverse down. nextFiber = fiber.child; @@ -363,7 +372,17 @@ function propagateContextChanges( if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleWorkOnParentPath(consumer.return, renderLanes); + + if (workInProgress.tag === SuspenseComponent) { + // This is intentionally passing this fiber as the parent + // 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. + const primaryChildFragment = workInProgress.child; + scheduleWorkOnParentPath(primaryChildFragment, renderLanes); + } else { + scheduleWorkOnParentPath(consumer.return, renderLanes); + } if (!forcePropagateEntireTree) { // During lazy propagation, when we find a match, we can defer diff --git a/packages/react-reconciler/src/ReactFiberNewContext.old.js b/packages/react-reconciler/src/ReactFiberNewContext.old.js index c2a1600650119..57108bf8ecc9b 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.old.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.old.js @@ -23,6 +23,7 @@ import { ContextProvider, ClassComponent, DehydratedFragment, + SuspenseComponent, } from './ReactWorkTags'; import { NoLanes, @@ -284,6 +285,14 @@ function propagateContextChange_eager( // this fiber to indicate that a context has changed. scheduleWorkOnParentPath(parentSuspense, renderLanes); nextFiber = fiber.sibling; + } else if ( + fiber.tag === SuspenseComponent && + workInProgress.tag === ContextProvider + ) { + // We don't know if it will have any context consumers in it. + // Schedule this fiber as having work on its children. + scheduleWorkOnParentPath(fiber.child, renderLanes); + nextFiber = fiber.child; } else { // Traverse down. nextFiber = fiber.child; @@ -363,7 +372,17 @@ function propagateContextChanges( if (alternate !== null) { alternate.lanes = mergeLanes(alternate.lanes, renderLanes); } - scheduleWorkOnParentPath(consumer.return, renderLanes); + + if (workInProgress.tag === SuspenseComponent) { + // This is intentionally passing this fiber as the parent + // 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. + const primaryChildFragment = workInProgress.child; + scheduleWorkOnParentPath(primaryChildFragment, renderLanes); + } else { + scheduleWorkOnParentPath(consumer.return, renderLanes); + } if (!forcePropagateEntireTree) { // During lazy propagation, when we find a match, we can defer From 3a1aa3eb323cd4d62420123c87e449562be62cdb Mon Sep 17 00:00:00 2001 From: okmttdhr Date: Mon, 17 Jan 2022 11:41:55 +0900 Subject: [PATCH 3/4] Update only parents that may be inconsistent --- .../src/ReactFiberNewContext.new.js | 14 ++++++++++++-- .../src/ReactFiberNewContext.old.js | 14 ++++++++++++-- 2 files changed, 24 insertions(+), 4 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberNewContext.new.js b/packages/react-reconciler/src/ReactFiberNewContext.new.js index d478369e64135..e5aee6675db99 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.new.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.new.js @@ -142,6 +142,7 @@ export function popProvider( export function scheduleWorkOnParentPath( parent: Fiber | null, renderLanes: Lanes, + stopAt?: Fiber | null = null, ) { // Update the child lanes of all the ancestors, including the alternates. let node = parent; @@ -160,6 +161,11 @@ export function scheduleWorkOnParentPath( } else { // Neither alternate was updated, which means the rest of the // ancestor path already has sufficient priority. + if (stopAt === null) { + break; + } + } + if (stopAt && node === stopAt) { break; } node = node.return; @@ -293,7 +299,7 @@ function propagateContextChange_eager( ) { // We don't know if it will have any context consumers in it. // Schedule this fiber as having work on its children. - scheduleWorkOnParentPath(fiber.child, renderLanes); + scheduleWorkOnParentPath(fiber.child, renderLanes, workInProgress); nextFiber = fiber.child; } else { // Traverse down. @@ -381,7 +387,11 @@ function propagateContextChanges( // on its children. We'll use the childLanes on // this fiber to indicate that a context has changed. const primaryChildFragment = workInProgress.child; - scheduleWorkOnParentPath(primaryChildFragment, renderLanes); + scheduleWorkOnParentPath( + primaryChildFragment, + renderLanes, + workInProgress, + ); } else { scheduleWorkOnParentPath(consumer.return, renderLanes); } diff --git a/packages/react-reconciler/src/ReactFiberNewContext.old.js b/packages/react-reconciler/src/ReactFiberNewContext.old.js index e55a73ef42b2a..a2587966c5e4d 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.old.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.old.js @@ -142,6 +142,7 @@ export function popProvider( export function scheduleWorkOnParentPath( parent: Fiber | null, renderLanes: Lanes, + stopAt?: Fiber | null = null, ) { // Update the child lanes of all the ancestors, including the alternates. let node = parent; @@ -160,6 +161,11 @@ export function scheduleWorkOnParentPath( } else { // Neither alternate was updated, which means the rest of the // ancestor path already has sufficient priority. + if (stopAt === null) { + break; + } + } + if (stopAt && node === stopAt) { break; } node = node.return; @@ -293,7 +299,7 @@ function propagateContextChange_eager( ) { // We don't know if it will have any context consumers in it. // Schedule this fiber as having work on its children. - scheduleWorkOnParentPath(fiber.child, renderLanes); + scheduleWorkOnParentPath(fiber.child, renderLanes, workInProgress); nextFiber = fiber.child; } else { // Traverse down. @@ -381,7 +387,11 @@ function propagateContextChanges( // on its children. We'll use the childLanes on // this fiber to indicate that a context has changed. const primaryChildFragment = workInProgress.child; - scheduleWorkOnParentPath(primaryChildFragment, renderLanes); + scheduleWorkOnParentPath( + primaryChildFragment, + renderLanes, + workInProgress, + ); } else { scheduleWorkOnParentPath(consumer.return, renderLanes); } From df5a90bf547c5dceef499799b582bdcb1a397f9c Mon Sep 17 00:00:00 2001 From: okmttdhr Date: Mon, 17 Jan 2022 12:07:34 +0900 Subject: [PATCH 4/4] No need to traverse to `stopAt` because it will `break;` anyway if it's consistent Revert "Update only parents that may be inconsistent" This reverts commit 3a1aa3eb323cd4d62420123c87e449562be62cdb. --- .../src/ReactFiberNewContext.new.js | 14 ++------------ .../src/ReactFiberNewContext.old.js | 14 ++------------ 2 files changed, 4 insertions(+), 24 deletions(-) diff --git a/packages/react-reconciler/src/ReactFiberNewContext.new.js b/packages/react-reconciler/src/ReactFiberNewContext.new.js index e5aee6675db99..d478369e64135 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.new.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.new.js @@ -142,7 +142,6 @@ export function popProvider( export function scheduleWorkOnParentPath( parent: Fiber | null, renderLanes: Lanes, - stopAt?: Fiber | null = null, ) { // Update the child lanes of all the ancestors, including the alternates. let node = parent; @@ -161,11 +160,6 @@ export function scheduleWorkOnParentPath( } else { // Neither alternate was updated, which means the rest of the // ancestor path already has sufficient priority. - if (stopAt === null) { - break; - } - } - if (stopAt && node === stopAt) { break; } node = node.return; @@ -299,7 +293,7 @@ function propagateContextChange_eager( ) { // We don't know if it will have any context consumers in it. // Schedule this fiber as having work on its children. - scheduleWorkOnParentPath(fiber.child, renderLanes, workInProgress); + scheduleWorkOnParentPath(fiber.child, renderLanes); nextFiber = fiber.child; } else { // Traverse down. @@ -387,11 +381,7 @@ function propagateContextChanges( // on its children. We'll use the childLanes on // this fiber to indicate that a context has changed. const primaryChildFragment = workInProgress.child; - scheduleWorkOnParentPath( - primaryChildFragment, - renderLanes, - workInProgress, - ); + scheduleWorkOnParentPath(primaryChildFragment, renderLanes); } else { scheduleWorkOnParentPath(consumer.return, renderLanes); } diff --git a/packages/react-reconciler/src/ReactFiberNewContext.old.js b/packages/react-reconciler/src/ReactFiberNewContext.old.js index a2587966c5e4d..e55a73ef42b2a 100644 --- a/packages/react-reconciler/src/ReactFiberNewContext.old.js +++ b/packages/react-reconciler/src/ReactFiberNewContext.old.js @@ -142,7 +142,6 @@ export function popProvider( export function scheduleWorkOnParentPath( parent: Fiber | null, renderLanes: Lanes, - stopAt?: Fiber | null = null, ) { // Update the child lanes of all the ancestors, including the alternates. let node = parent; @@ -161,11 +160,6 @@ export function scheduleWorkOnParentPath( } else { // Neither alternate was updated, which means the rest of the // ancestor path already has sufficient priority. - if (stopAt === null) { - break; - } - } - if (stopAt && node === stopAt) { break; } node = node.return; @@ -299,7 +293,7 @@ function propagateContextChange_eager( ) { // We don't know if it will have any context consumers in it. // Schedule this fiber as having work on its children. - scheduleWorkOnParentPath(fiber.child, renderLanes, workInProgress); + scheduleWorkOnParentPath(fiber.child, renderLanes); nextFiber = fiber.child; } else { // Traverse down. @@ -387,11 +381,7 @@ function propagateContextChanges( // on its children. We'll use the childLanes on // this fiber to indicate that a context has changed. const primaryChildFragment = workInProgress.child; - scheduleWorkOnParentPath( - primaryChildFragment, - renderLanes, - workInProgress, - ); + scheduleWorkOnParentPath(primaryChildFragment, renderLanes); } else { scheduleWorkOnParentPath(consumer.return, renderLanes); }