diff --git a/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.internal.js b/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.internal.js index 5c35263789743..fbed0d4b82a75 100644 --- a/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.internal.js +++ b/packages/react-dom/src/__tests__/ReactDOMSuspensePlaceholder-test.internal.js @@ -9,6 +9,7 @@ 'use strict'; +let ReactFeatureFlags; let React; let ReactDOM; let Suspense; @@ -20,6 +21,8 @@ describe('ReactDOMSuspensePlaceholder', () => { beforeEach(() => { jest.resetModules(); + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.enableHooks = true; React = require('react'); ReactDOM = require('react-dom'); ReactCache = require('react-cache'); @@ -108,4 +111,50 @@ describe('ReactDOMSuspensePlaceholder', () => { expect(container.textContent).toEqual('ABC'); }); + + it( + 'outside concurrent mode, re-hides children if their display is updated ' + + 'but the boundary is still showing the fallback', + async () => { + const {useState} = React; + + let setIsVisible; + function Sibling({children}) { + const [isVisible, _setIsVisible] = useState(false); + setIsVisible = _setIsVisible; + return ( + + {children} + + ); + } + + function App() { + return ( + }> + Sibling + + + + + ); + } + + ReactDOM.render(, container); + expect(container.innerHTML).toEqual( + 'SiblingLoading...', + ); + + setIsVisible(true); + expect(container.innerHTML).toEqual( + 'SiblingLoading...', + ); + + await advanceTimers(1000); + + expect(container.innerHTML).toEqual( + 'SiblingAsync', + ); + }, + ); }); diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.js b/packages/react-reconciler/src/ReactFiberBeginWork.js index 92f4ccef7954e..67808381bf06a 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.js @@ -66,7 +66,7 @@ import { } from './ReactChildFiber'; import {processUpdateQueue} from './ReactUpdateQueue'; import {NoWork, Never} from './ReactFiberExpirationTime'; -import {ConcurrentMode, StrictMode} from './ReactTypeOfMode'; +import {ConcurrentMode, StrictMode, NoContext} from './ReactTypeOfMode'; import { shouldSetTextContent, shouldDeprioritizeSubtree, @@ -1071,31 +1071,21 @@ function updateSuspenseComponent( // We should attempt to render the primary children unless this boundary // already suspended during this render (`alreadyCaptured` is true). let nextState: SuspenseState | null = workInProgress.memoizedState; - if (nextState === null) { - // An empty suspense state means this boundary has not yet timed out. + + let nextDidTimeout; + if ((workInProgress.effectTag & DidCapture) === NoEffect) { + // This is the first attempt. + nextState = null; + nextDidTimeout = false; } else { - if (!nextState.alreadyCaptured) { - // Since we haven't already suspended during this commit, clear the - // existing suspense state. We'll try rendering again. - nextState = null; - } else { - // Something in this boundary's subtree already suspended. Switch to - // rendering the fallback children. Set `alreadyCaptured` to true. - if (current !== null && nextState === current.memoizedState) { - // Create a new suspense state to avoid mutating the current tree's. - nextState = { - alreadyCaptured: true, - didTimeout: true, - timedOutAt: nextState.timedOutAt, - }; - } else { - // Already have a clone, so it's safe to mutate. - nextState.alreadyCaptured = true; - nextState.didTimeout = true; - } - } + // Something in this boundary's subtree already suspended. Switch to + // rendering the fallback children. + nextState = { + timedOutAt: nextState !== null ? nextState.timedOutAt : NoWork, + }; + nextDidTimeout = true; + workInProgress.effectTag &= ~DidCapture; } - const nextDidTimeout = nextState !== null && nextState.didTimeout; // This next part is a bit confusing. If the children timeout, we switch to // showing the fallback children in place of the "primary" children. @@ -1140,6 +1130,22 @@ function updateSuspenseComponent( NoWork, null, ); + + if ((workInProgress.mode & ConcurrentMode) === NoContext) { + // Outside of concurrent mode, we commit the effects from the + // partially completed, timed-out tree, too. + const progressedState: SuspenseState = workInProgress.memoizedState; + const progressedPrimaryChild: Fiber | null = + progressedState !== null + ? (workInProgress.child: any).child + : (workInProgress.child: any); + reuseProgressedPrimaryChild( + workInProgress, + primaryChildFragment, + progressedPrimaryChild, + ); + } + const fallbackChildFragment = createFiberFromFragment( nextFallbackChildren, mode, @@ -1166,7 +1172,7 @@ function updateSuspenseComponent( // This is an update. This branch is more complicated because we need to // ensure the state of the primary children is preserved. const prevState = current.memoizedState; - const prevDidTimeout = prevState !== null && prevState.didTimeout; + const prevDidTimeout = prevState !== null; if (prevDidTimeout) { // The current tree already timed out. That means each child set is // wrapped in a fragment fiber. @@ -1182,6 +1188,24 @@ function updateSuspenseComponent( NoWork, ); primaryChildFragment.effectTag |= Placement; + + if ((workInProgress.mode & ConcurrentMode) === NoContext) { + // Outside of concurrent mode, we commit the effects from the + // partially completed, timed-out tree, too. + const progressedState: SuspenseState = workInProgress.memoizedState; + const progressedPrimaryChild: Fiber | null = + progressedState !== null + ? (workInProgress.child: any).child + : (workInProgress.child: any); + if (progressedPrimaryChild !== currentPrimaryChildFragment.child) { + reuseProgressedPrimaryChild( + workInProgress, + primaryChildFragment, + progressedPrimaryChild, + ); + } + } + // Clone the fallback child fragment, too. These we'll continue // working on. const fallbackChildFragment = (primaryChildFragment.sibling = createWorkInProgress( @@ -1237,6 +1261,22 @@ function updateSuspenseComponent( primaryChildFragment.effectTag |= Placement; primaryChildFragment.child = currentPrimaryChild; currentPrimaryChild.return = primaryChildFragment; + + if ((workInProgress.mode & ConcurrentMode) === NoContext) { + // Outside of concurrent mode, we commit the effects from the + // partially completed, timed-out tree, too. + const progressedState: SuspenseState = workInProgress.memoizedState; + const progressedPrimaryChild: Fiber | null = + progressedState !== null + ? (workInProgress.child: any).child + : (workInProgress.child: any); + reuseProgressedPrimaryChild( + workInProgress, + primaryChildFragment, + progressedPrimaryChild, + ); + } + // Create a fragment from the fallback children, too. const fallbackChildFragment = (primaryChildFragment.sibling = createFiberFromFragment( nextFallbackChildren, @@ -1270,6 +1310,49 @@ function updateSuspenseComponent( return next; } +function reuseProgressedPrimaryChild( + workInProgress: Fiber, + primaryChildFragment: Fiber, + progressedChild: Fiber | null, +) { + // This function is only called outside concurrent mode. Usually, if a work- + // in-progress primary tree suspends, we throw it out and revert back to + // current. Outside concurrent mode, though, we commit the suspended work-in- + // progress, even though it didn't complete. This function reuses the children + // and transfers the effects. + let child = (primaryChildFragment.child = progressedChild); + while (child !== null) { + // Ensure that the first and last effect of the parent corresponds + // to the children's first and last effect. + if (primaryChildFragment.firstEffect === null) { + primaryChildFragment.firstEffect = child.firstEffect; + } + if (child.lastEffect !== null) { + if (primaryChildFragment.lastEffect !== null) { + primaryChildFragment.lastEffect.nextEffect = child.firstEffect; + } + primaryChildFragment.lastEffect = child.lastEffect; + } + + // Append all the effects of the subtree and this fiber onto the effect + // list of the parent. The completion order of the children affects the + // side-effect order. + if (child.effectTag > PerformedWork) { + if (primaryChildFragment.lastEffect !== null) { + primaryChildFragment.lastEffect.nextEffect = child; + } else { + primaryChildFragment.firstEffect = child; + } + primaryChildFragment.lastEffect = child; + } + child.return = primaryChildFragment; + child = child.sibling; + } + + workInProgress.firstEffect = primaryChildFragment.firstEffect; + workInProgress.lastEffect = primaryChildFragment.lastEffect; +} + function updatePortalComponent( current: Fiber | null, workInProgress: Fiber, @@ -1426,25 +1509,6 @@ function updateContextConsumer( return workInProgress.child; } -/* - function reuseChildrenEffects(returnFiber : Fiber, firstChild : Fiber) { - let child = firstChild; - do { - // Ensure that the first and last effect of the parent corresponds - // to the children's first and last effect. - if (!returnFiber.firstEffect) { - returnFiber.firstEffect = child.firstEffect; - } - if (child.lastEffect) { - if (returnFiber.lastEffect) { - returnFiber.lastEffect.nextEffect = child.firstEffect; - } - returnFiber.lastEffect = child.lastEffect; - } - } while (child = child.sibling); - } - */ - function bailoutOnAlreadyFinishedWork( current: Fiber | null, workInProgress: Fiber, @@ -1528,7 +1592,7 @@ function beginWork( break; case SuspenseComponent: { const state: SuspenseState | null = workInProgress.memoizedState; - const didTimeout = state !== null && state.didTimeout; + const didTimeout = state !== null; if (didTimeout) { // If this boundary is currently timed out, we need to decide // whether to retry the primary children, or to skip over it and diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index b8e4ac203b66b..359956a341d80 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -50,13 +50,12 @@ import { Placement, Snapshot, Update, - Callback, } from 'shared/ReactSideEffectTags'; import getComponentName from 'shared/getComponentName'; import invariant from 'shared/invariant'; import warningWithoutStack from 'shared/warningWithoutStack'; -import {NoWork, Sync} from './ReactFiberExpirationTime'; +import {NoWork} from './ReactFiberExpirationTime'; import {onCommitUnmount} from './ReactFiberDevToolsHook'; import {startPhaseTimer, stopPhaseTimer} from './ReactDebugFiberPerf'; import {getStackByFiberInDevAndProd} from './ReactCurrentFiber'; @@ -86,9 +85,7 @@ import { } from './ReactFiberHostConfig'; import { captureCommitPhaseError, - flushPassiveEffects, requestCurrentTime, - scheduleWork, } from './ReactFiberScheduler'; import { NoEffect as NoHookEffect, @@ -453,47 +450,27 @@ function commitLifeCycles( return; } case SuspenseComponent: { - if (finishedWork.effectTag & Callback) { - // In non-strict mode, a suspense boundary times out by commiting - // twice: first, by committing the children in an inconsistent state, - // then hiding them and showing the fallback children in a subsequent - // commit. - const newState: SuspenseState = { - alreadyCaptured: true, - didTimeout: false, - timedOutAt: NoWork, - }; - finishedWork.memoizedState = newState; - flushPassiveEffects(); - scheduleWork(finishedWork, Sync); - return; - } - let oldState: SuspenseState | null = - current !== null ? current.memoizedState : null; let newState: SuspenseState | null = finishedWork.memoizedState; - let oldDidTimeout = oldState !== null ? oldState.didTimeout : false; let newDidTimeout; let primaryChildParent = finishedWork; if (newState === null) { newDidTimeout = false; } else { - newDidTimeout = newState.didTimeout; - if (newDidTimeout) { - primaryChildParent = finishedWork.child; - newState.alreadyCaptured = false; - if (newState.timedOutAt === NoWork) { - // If the children had not already timed out, record the time. - // This is used to compute the elapsed time during subsequent - // attempts to render the children. - newState.timedOutAt = requestCurrentTime(); - } + newDidTimeout = true; + primaryChildParent = finishedWork.child; + if (newState.timedOutAt === NoWork) { + // If the children had not already timed out, record the time. + // This is used to compute the elapsed time during subsequent + // attempts to render the children. + newState.timedOutAt = requestCurrentTime(); } } - if (newDidTimeout !== oldDidTimeout && primaryChildParent !== null) { + if (primaryChildParent !== null) { hideOrUnhideAllChildren(primaryChildParent, newDidTimeout); } + return; } case IncompleteClassComponent: diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 79c36c1a7747d..d435d3296aeab 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -39,7 +39,13 @@ import { LazyComponent, IncompleteClassComponent, } from 'shared/ReactWorkTags'; -import {Placement, Ref, Update} from 'shared/ReactSideEffectTags'; +import { + Placement, + Ref, + Update, + NoEffect, + DidCapture, +} from 'shared/ReactSideEffectTags'; import invariant from 'shared/invariant'; import { @@ -75,6 +81,7 @@ import { prepareToHydrateHostTextInstance, popHydrationState, } from './ReactFiberHydrationContext'; +import {ConcurrentMode, NoContext} from './ReactTypeOfMode'; function markUpdate(workInProgress: Fiber) { // Tag the fiber with an update effect. This turns a Placement into @@ -247,8 +254,8 @@ if (supportsMutation) { if (current !== null) { const oldState: SuspenseState = current.memoizedState; const newState: SuspenseState = node.memoizedState; - const oldIsHidden = oldState !== null && oldState.didTimeout; - const newIsHidden = newState !== null && newState.didTimeout; + const oldIsHidden = oldState !== null; + const newIsHidden = newState !== null; if (oldIsHidden !== newIsHidden) { // The placeholder either just timed out or switched back to the normal // children after having previously timed out. Toggle the visibility of @@ -350,8 +357,8 @@ if (supportsMutation) { if (current !== null) { const oldState: SuspenseState = current.memoizedState; const newState: SuspenseState = node.memoizedState; - const oldIsHidden = oldState !== null && oldState.didTimeout; - const newIsHidden = newState !== null && newState.didTimeout; + const oldIsHidden = oldState !== null; + const newIsHidden = newState !== null; if (oldIsHidden !== newIsHidden) { // The placeholder either just timed out or switched back to the normal // children after having previously timed out. Toggle the visibility of @@ -690,12 +697,27 @@ function completeWork( break; case SuspenseComponent: { const nextState = workInProgress.memoizedState; - const prevState = current !== null ? current.memoizedState : null; - const nextDidTimeout = nextState !== null && nextState.didTimeout; - const prevDidTimeout = prevState !== null && prevState.didTimeout; - if (nextDidTimeout !== prevDidTimeout) { - // If this render commits, and it switches between the normal state - // and the timed-out state, schedule an effect. + if ((workInProgress.effectTag & DidCapture) !== NoEffect) { + // Something suspended. Re-render with the fallback children. + workInProgress.expirationTime = renderExpirationTime; + workInProgress.firstEffect = workInProgress.lastEffect = null; + return workInProgress; + } + + const nextDidTimeout = nextState !== null; + const prevDidTimeout = current !== null && current.memoizedState !== null; + // The children either timed out after previously being visible, or + // were restored after previously being hidden. Schedule an effect + // to update their visiblity. + if ( + // + nextDidTimeout !== prevDidTimeout || + // Outside concurrent mode, the primary children commit in an + // inconsistent state, even if they are hidden. So if they are hidden, + // we need to schedule an effect to re-hide them, just in case. + ((workInProgress.effectTag & ConcurrentMode) === NoContext && + nextDidTimeout) + ) { workInProgress.effectTag |= Update; } break; diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index 75717b03d7d17..719a0bdef8a97 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -981,6 +981,12 @@ function completeUnitOfWork(workInProgress: Fiber): Fiber | null { ReactCurrentFiber.resetCurrentFiber(); } + if (nextUnitOfWork !== null) { + // Completing this fiber spawned new work. Work on that next. + nextUnitOfWork.firstEffect = nextUnitOfWork.lastEffect = null; + return nextUnitOfWork; + } + if ( returnFiber !== null && // Do not append effects to parents if a sibling failed to complete diff --git a/packages/react-reconciler/src/ReactFiberSuspenseComponent.js b/packages/react-reconciler/src/ReactFiberSuspenseComponent.js index 0ff04687e3ba3..c31cf93473928 100644 --- a/packages/react-reconciler/src/ReactFiberSuspenseComponent.js +++ b/packages/react-reconciler/src/ReactFiberSuspenseComponent.js @@ -11,17 +11,6 @@ import type {Fiber} from './ReactFiber'; import type {ExpirationTime} from './ReactFiberExpirationTime'; export type SuspenseState = {| - // Whether a component in the child subtree already suspended. If true, - // subsequent suspends should bubble up to the next boundary. - alreadyCaptured: boolean, - // Whether the boundary renders the primary or fallback children. This is - // separate from `alreadyCaptured` because outside of strict mode, when a - // boundary times out, the first commit renders the primary children in an - // incomplete state, then performs a second commit to switch the fallback. - // In that first commit, `alreadyCaptured` is false and `didTimeout` is true. - didTimeout: boolean, - // The time at which the boundary timed out. This is separate from - // `didTimeout` because it's not set unless the boundary actually commits. timedOutAt: ExpirationTime, |}; @@ -36,5 +25,5 @@ export function shouldCaptureSuspense( // If it was the primary children that just suspended, capture and render the // fallback. Otherwise, don't capture and bubble to the next boundary. const nextState: SuspenseState | null = workInProgress.memoizedState; - return nextState === null || !nextState.didTimeout; + return nextState === null; } diff --git a/packages/react-reconciler/src/ReactFiberUnwindWork.js b/packages/react-reconciler/src/ReactFiberUnwindWork.js index 004d5e5f6ab30..6fb2e080687e5 100644 --- a/packages/react-reconciler/src/ReactFiberUnwindWork.js +++ b/packages/react-reconciler/src/ReactFiberUnwindWork.js @@ -32,7 +32,6 @@ import { Incomplete, NoEffect, ShouldCapture, - Callback as CallbackEffect, LifecycleEffectMask, } from 'shared/ReactSideEffectTags'; import {enableSchedulerTracing} from 'shared/ReactFeatureFlags'; @@ -66,7 +65,6 @@ import { import invariant from 'shared/invariant'; import maxSigned31BitInt from './maxSigned31BitInt'; import { - NoWork, Sync, expirationTimeToMs, LOW_PRIORITY_EXPIRATION, @@ -176,7 +174,7 @@ function throwException( const current = workInProgress.alternate; if (current !== null) { const currentState: SuspenseState | null = current.memoizedState; - if (currentState !== null && currentState.didTimeout) { + if (currentState !== null) { // Reached a boundary that already timed out. Do not search // any further. const timedOutAt = currentState.timedOutAt; @@ -238,7 +236,7 @@ function throwException( // inside a concurrent mode tree. If the Suspense is outside of it, we // should *not* suspend the commit. if ((workInProgress.mode & ConcurrentMode) === NoEffect) { - workInProgress.effectTag |= CallbackEffect; + workInProgress.effectTag |= DidCapture; // Unmount the source fiber's children const nextChildren = null; @@ -265,6 +263,10 @@ function throwException( } } + // The source fiber did not complete. Mark it with the current + // render priority to indicate that it still has pending work. + sourceFiber.expirationTime = renderExpirationTime; + // Exit without suspending. return; } @@ -415,33 +417,7 @@ function unwindWork( const effectTag = workInProgress.effectTag; if (effectTag & ShouldCapture) { workInProgress.effectTag = (effectTag & ~ShouldCapture) | DidCapture; - // Captured a suspense effect. Set the boundary's `alreadyCaptured` - // state to true so we know to render the fallback. - const current = workInProgress.alternate; - const currentState: SuspenseState | null = - current !== null ? current.memoizedState : null; - let nextState: SuspenseState | null = workInProgress.memoizedState; - if (nextState === null) { - // No existing state. Create a new object. - nextState = { - alreadyCaptured: true, - didTimeout: false, - timedOutAt: NoWork, - }; - } else if (currentState === nextState) { - // There is an existing state but it's the same as the current tree's. - // Clone the object. - nextState = { - alreadyCaptured: true, - didTimeout: nextState.didTimeout, - timedOutAt: nextState.timedOutAt, - }; - } else { - // Already have a clone, so it's safe to mutate. - nextState.alreadyCaptured = true; - } - workInProgress.memoizedState = nextState; - // Re-render the boundary. + // Captured a suspense effect. Re-render the boundary. return workInProgress; } return null; diff --git a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js index 444663ad346d0..37fd4c70d0a7f 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspense-test.internal.js @@ -401,14 +401,12 @@ describe('ReactSuspense', () => { 'A', 'Suspend! [B:1]', 'C', + 'Loading...', 'Mount [A]', // B's lifecycle should not fire because it suspended // 'Mount [B]', 'Mount [C]', - - // In a subsequent commit, render a placeholder - 'Loading...', 'Mount [Loading...]', ]); expect(root).toMatchRenderedOutput('Loading...'); @@ -483,13 +481,76 @@ describe('ReactSuspense', () => { expect(root).toMatchRenderedOutput('Loading...'); instance.setState({step: 2}); + expect(ReactTestRenderer).toHaveYielded(['Stateful: 2', 'Suspend! [B]']); + expect(root).toMatchRenderedOutput('Loading...'); + + jest.advanceTimersByTime(1000); expect(ReactTestRenderer).toHaveYielded([ - 'Stateful: 2', + 'Promise resolved [B]', + 'B', + 'B', + ]); + expect(root).toMatchRenderedOutput('Stateful: 2B'); + }); + + it('when updating a timed-out tree, always retries the suspended component', () => { + let instance; + class Stateful extends React.Component { + state = {step: 1}; + render() { + instance = this; + return ; + } + } + + const Indirection = React.Fragment; + + function App(props) { + return ( + }> + + + + + + + + + + ); + } + + const root = ReactTestRenderer.create(); + + expect(ReactTestRenderer).toHaveYielded([ + 'Stateful: 1', + 'Suspend! [A]', + 'Loading...', + ]); + + jest.advanceTimersByTime(1000); + expect(ReactTestRenderer).toHaveYielded(['Promise resolved [A]', 'A']); + expect(root).toMatchRenderedOutput('Stateful: 1A'); + + root.update(); + expect(ReactTestRenderer).toHaveYielded([ + 'Stateful: 1', 'Suspend! [B]', 'Loading...', ]); expect(root).toMatchRenderedOutput('Loading...'); + instance.setState({step: 2}); + expect(ReactTestRenderer).toHaveYielded([ + 'Stateful: 2', + + // The suspended component should suspend again. If it doesn't, the + // likely mistake is that the suspended fiber wasn't marked with + // pending work, so it was improperly treated as complete. + 'Suspend! [B]', + ]); + expect(root).toMatchRenderedOutput('Loading...'); + jest.advanceTimersByTime(1000); expect(ReactTestRenderer).toHaveYielded([ 'Promise resolved [B]', diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js index 3d02a0084395b..6b00d94edbf50 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseWithNoopRenderer-test.internal.js @@ -929,11 +929,10 @@ describe('ReactSuspenseWithNoopRenderer', () => { // Suspend during an async render. expect(ReactNoop.flushNextYield()).toEqual(['Suspend! [Step: 2]']); expect(ReactNoop.flush()).toEqual([ - 'Update did commit', - // Switch to the placeholder in a subsequent commit 'Loading (1)', 'Loading (2)', 'Loading (3)', + 'Update did commit', ]); expect(ReactNoop.getChildrenAsJSX()).toEqual( @@ -1012,12 +1011,11 @@ describe('ReactSuspenseWithNoopRenderer', () => { 'Before', 'Suspend! [Async: 1]', 'After', + 'Loading...', 'Before', 'Sync: 1', 'After', 'Did mount', - // The placeholder is rendered in a subsequent commit - 'Loading...', 'Promise resolved [Async: 1]', 'Async: 1', ]); @@ -1051,14 +1049,12 @@ describe('ReactSuspenseWithNoopRenderer', () => { ]); expect(ReactNoop.flush()).toEqual([ 'After', + 'Loading...', 'Before', 'Sync: 2', 'After', 'Update 1 did commit', 'Update 2 did commit', - - // Switch to the placeholder in a subsequent commit - 'Loading...', ]); expect(ReactNoop.getChildrenAsJSX()).toEqual( @@ -1149,12 +1145,11 @@ describe('ReactSuspenseWithNoopRenderer', () => { 'Before', 'Suspend! [Async: 1]', 'After', + 'Loading...', 'Before', 'Sync: 1', 'After', 'Did mount', - // The placeholder is rendered in a subsequent commit - 'Loading...', 'Promise resolved [Async: 1]', 'Async: 1', ]); @@ -1188,14 +1183,12 @@ describe('ReactSuspenseWithNoopRenderer', () => { ]); expect(ReactNoop.flush()).toEqual([ 'After', + 'Loading...', 'Before', 'Sync: 2', 'After', 'Update 1 did commit', 'Update 2 did commit', - - // Switch to the placeholder in a subsequent commit - 'Loading...', ]); expect(ReactNoop.getChildrenAsJSX()).toEqual( @@ -1276,16 +1269,13 @@ describe('ReactSuspenseWithNoopRenderer', () => { 'Suspend! [B]', 'C', + 'Loading...', 'Mount [A]', 'Mount [B]', 'Mount [C]', - 'Commit root', - - // In a subsequent commit, render a placeholder - 'Loading...', - // Force delete all the existing children when switching to the - // placeholder. This should be a mount, not an update. + // This should be a mount, not an update. 'Mount [Loading...]', + 'Commit root', ]); expect(ReactNoop.getChildrenAsJSX()).toEqual( @@ -1453,16 +1443,14 @@ describe('ReactSuspenseWithNoopRenderer', () => { 'A', 'Suspend! [B]', 'C', + 'Loading...', 'Mount [A]', // B's lifecycle should not fire because it suspended // 'Mount [B]', 'Mount [C]', - 'Commit root', - - // In a subsequent commit, render a placeholder - 'Loading...', 'Mount [Loading...]', + 'Commit root', ]); expect(ReactNoop.getChildrenAsJSX()).toEqual( diff --git a/packages/react/src/__tests__/ReactProfiler-test.internal.js b/packages/react/src/__tests__/ReactProfiler-test.internal.js index 18b42dde793c5..a811cec410358 100644 --- a/packages/react/src/__tests__/ReactProfiler-test.internal.js +++ b/packages/react/src/__tests__/ReactProfiler-test.internal.js @@ -2501,7 +2501,7 @@ describe('Profiler', () => { expect(renderer.toJSON()).toEqual(['loading', 'initial']); expect(onInteractionScheduledWorkCompleted).not.toHaveBeenCalled(); - expect(onRender).toHaveBeenCalledTimes(2); // Sync null commit, placeholder commit + expect(onRender).toHaveBeenCalledTimes(1); expect(onRender.mock.calls[0][6]).toMatchInteractions([ initialRenderInteraction, ]); @@ -2535,13 +2535,10 @@ describe('Profiler', () => { }); expect(renderer.toJSON()).toEqual(['loading', 'updated']); - expect(onRender).toHaveBeenCalledTimes(2); // Sync null commit, placeholder commit + expect(onRender).toHaveBeenCalledTimes(1); expect(onRender.mock.calls[0][6]).toMatchInteractions([ highPriUpdateInteraction, ]); - expect(onRender.mock.calls[1][6]).toMatchInteractions([ - highPriUpdateInteraction, - ]); onRender.mockClear(); expect(onInteractionScheduledWorkCompleted).not.toHaveBeenCalled();