diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js index b8043c2b8297b..539ef3ae0c2df 100644 --- a/packages/react-reconciler/src/ReactFiberCommitWork.js +++ b/packages/react-reconciler/src/ReactFiberCommitWork.js @@ -104,6 +104,7 @@ import { captureCommitPhaseError, requestCurrentTime, resolveRetryThenable, + restorePendingSchedulers, } from './ReactFiberScheduler'; import { NoEffect as NoHookEffect, @@ -1169,7 +1170,12 @@ function commitDeletion(current: Fiber): void { detachFiber(current); } -function commitWork(current: Fiber | null, finishedWork: Fiber): void { +function commitWork( + finishedRoot: FiberRoot, + current: Fiber | null, + finishedWork: Fiber, + committedExpirationTime: ExpirationTime, +): void { if (!supportsMutation) { switch (finishedWork.tag) { case FunctionComponent: @@ -1185,7 +1191,11 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void { return; } case SuspenseComponent: { - commitSuspenseComponent(finishedWork); + commitSuspenseComponent( + finishedRoot, + finishedWork, + committedExpirationTime, + ); return; } } @@ -1259,7 +1269,11 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void { return; } case SuspenseComponent: { - commitSuspenseComponent(finishedWork); + commitSuspenseComponent( + finishedRoot, + finishedWork, + committedExpirationTime, + ); return; } case IncompleteClassComponent: { @@ -1278,7 +1292,11 @@ function commitWork(current: Fiber | null, finishedWork: Fiber): void { } } -function commitSuspenseComponent(finishedWork: Fiber) { +function commitSuspenseComponent( + finishedRoot: FiberRoot, + finishedWork: Fiber, + committedExpirationTime: ExpirationTime, +) { let newState: SuspenseState | null = finishedWork.memoizedState; let newDidTimeout; @@ -1320,6 +1338,9 @@ function commitSuspenseComponent(finishedWork: Fiber) { if (!retryCache.has(thenable)) { if (enableSchedulerTracing) { retry = Schedule_tracing_wrap(retry); + + // If we have pending work still, restore the original schedulers + restorePendingSchedulers(finishedRoot, committedExpirationTime); } retryCache.add(thenable); thenable.then(retry, retry); diff --git a/packages/react-reconciler/src/ReactFiberRoot.js b/packages/react-reconciler/src/ReactFiberRoot.js index 94050987a5bf6..25f2f0c20dee4 100644 --- a/packages/react-reconciler/src/ReactFiberRoot.js +++ b/packages/react-reconciler/src/ReactFiberRoot.js @@ -29,7 +29,9 @@ export type Batch = { }; export type PendingInteractionMap = Map>; -export type PendingSchedulerMap = Map>; + +// Map of expiration time to all pending "schedulers" which in turn is a map of Fibers to reference counts. +export type PendingSchedulersMap = Map>; type BaseFiberRootProperties = {| // The type of root (legacy, batched, concurrent, etc.) @@ -86,7 +88,7 @@ type ProfilingOnlyFiberRootProperties = {| // Used to enable DevTools Profiler UI to show which Fibers scheduled a given commit. // May also be useful in the future to expose via the Profiler API somehow? memoizedSchedulers: Set, - pendingSchedulerMap: PendingSchedulerMap, + pendingSchedulersMap: PendingSchedulersMap, |}; // Exported FiberRoot type includes all properties, @@ -123,7 +125,7 @@ function FiberRootNode(containerInfo, tag, hydrate) { this.memoizedInteractions = new Set(); this.pendingInteractionMap = new Map(); this.memoizedSchedulers = new Set(); - this.pendingSchedulerMap = new Map(); + this.pendingSchedulersMap = new Map(); } } diff --git a/packages/react-reconciler/src/ReactFiberScheduler.js b/packages/react-reconciler/src/ReactFiberScheduler.js index 332832f74442a..815b85b7578a6 100644 --- a/packages/react-reconciler/src/ReactFiberScheduler.js +++ b/packages/react-reconciler/src/ReactFiberScheduler.js @@ -337,13 +337,13 @@ export function scheduleUpdateOnFiber( } if (enableSchedulerTracing) { - const pendingSchedulerMap = root.pendingSchedulerMap; - const pendingSchedulers = pendingSchedulerMap.get(expirationTime); - if (pendingSchedulers != null) { - pendingSchedulers.add(fiber); - } else { - pendingSchedulerMap.set(expirationTime, new Set([fiber])); + const pendingSchedulersMap = root.pendingSchedulersMap; + let schedulers = pendingSchedulersMap.get(expirationTime); + if (schedulers == null) { + schedulers = new Set(); + pendingSchedulersMap.set(expirationTime, schedulers); } + schedulers.add(fiber); } root.pingTime = NoWork; @@ -1377,7 +1377,13 @@ function commitRootImpl(root) { nextEffect = firstEffect; do { if (__DEV__) { - invokeGuardedCallback(null, commitMutationEffects, null); + invokeGuardedCallback( + null, + commitMutationEffects, + null, + root, + expirationTime, + ); if (hasCaughtError()) { invariant(nextEffect !== null, 'Should be working on an effect.'); const error = clearCaughtError(); @@ -1386,7 +1392,7 @@ function commitRootImpl(root) { } } else { try { - commitMutationEffects(); + commitMutationEffects(root, expirationTime); } catch (error) { invariant(nextEffect !== null, 'Should be working on an effect.'); captureCommitPhaseError(nextEffect, error); @@ -1540,7 +1546,10 @@ function commitBeforeMutationEffects() { } } -function commitMutationEffects() { +function commitMutationEffects( + root: FiberRoot, + committedExpirationTime: ExpirationTime, +) { // TODO: Should probably move the bulk of this function to commitWork. while (nextEffect !== null) { setCurrentDebugFiberInDEV(nextEffect); @@ -1582,12 +1591,12 @@ function commitMutationEffects() { // Update const current = nextEffect.alternate; - commitWork(current, nextEffect); + commitWork(root, current, nextEffect, committedExpirationTime); break; } case Update: { const current = nextEffect.alternate; - commitWork(current, nextEffect); + commitWork(root, current, nextEffect, committedExpirationTime); break; } case Deletion: { @@ -1843,7 +1852,12 @@ export function retryTimedOutBoundary(boundaryFiber: Fiber) { } } -export function resolveRetryThenable(boundaryFiber: Fiber, thenable: Thenable) { +export function resolveRetryThenable( + boundaryFiber: Fiber, + thenable: Thenable, + root: FiberRoot, + committedExpirationTime: ExpirationTime, +) { let retryCache: WeakSet | Set | null; if (enableSuspenseServerRenderer) { switch (boundaryFiber.tag) { @@ -2161,6 +2175,21 @@ function warnIfNotCurrentlyActingUpdatesInDEV(fiber: Fiber): void { } } +export function restorePendingSchedulers( + root: FiberRoot, + expirationTime: ExpirationTime, +): void { + const pendingSchedulersMap = root.pendingSchedulersMap; + let schedulers = pendingSchedulersMap.get(expirationTime); + if (schedulers == null) { + schedulers = new Set(); + pendingSchedulersMap.set(expirationTime, schedulers); + } + root.memoizedSchedulers.forEach(schedulingFiber => { + ((schedulers: any): Set).add(schedulingFiber); + }); +} + export const warnIfNotCurrentlyActingUpdatesInDev = warnIfNotCurrentlyActingUpdatesInDEV; let componentsWithSuspendedDiscreteUpdates = null; @@ -2294,14 +2323,15 @@ function startWorkOnPendingInteraction(root, expirationTime) { } }, ); - const schedulers: Set = new Set(); - root.pendingSchedulerMap.forEach( - (schedulingFibers, scheduledExpirationTime) => { - if (scheduledExpirationTime >= expirationTime) { - schedulingFibers.forEach(fiber => schedulers.add(fiber)); - } - }, - ); + + const memoizedSchedulers: Set = new Set(); + const pendingSchedulersMap = root.pendingSchedulersMap; + pendingSchedulersMap.forEach((schedulers, scheduledExpirationTime) => { + if (scheduledExpirationTime >= expirationTime) { + pendingSchedulersMap.delete(scheduledExpirationTime); + schedulers.forEach(fiber => memoizedSchedulers.add(fiber)); + } + }); // Store the current set of interactions on the FiberRoot for a few reasons: // We can re-use it in hot functions like renderRoot() without having to @@ -2309,7 +2339,7 @@ function startWorkOnPendingInteraction(root, expirationTime) { // onRender() hooks. This also provides DevTools with a way to access it when // the onCommitRoot() hook is called. root.memoizedInteractions = interactions; - root.memoizedSchedulers = schedulers; + root.memoizedSchedulers = memoizedSchedulers; if (interactions.size > 0) { const subscriber = __subscriberRef.current; @@ -2377,12 +2407,5 @@ function finishPendingInteractions(root, committedExpirationTime) { } }, ); - - const pendingSchedulerMap = root.pendingSchedulerMap; - pendingSchedulerMap.forEach((schedulingFibers, scheduledExpirationTime) => { - if (scheduledExpirationTime > earliestRemainingTimeAfterCommit) { - pendingSchedulerMap.delete(scheduledExpirationTime); - } - }); } } diff --git a/packages/react-reconciler/src/ReactFiberUnwindWork.js b/packages/react-reconciler/src/ReactFiberUnwindWork.js index 1d4a4cbe236eb..50a2841a00969 100644 --- a/packages/react-reconciler/src/ReactFiberUnwindWork.js +++ b/packages/react-reconciler/src/ReactFiberUnwindWork.js @@ -76,6 +76,7 @@ import { pingSuspendedRoot, resolveRetryThenable, checkForWrongSuspensePriorityInDEV, + restorePendingSchedulers, } from './ReactFiberScheduler'; import invariant from 'shared/invariant'; @@ -186,6 +187,9 @@ function attachPingListener( ); if (enableSchedulerTracing) { ping = Schedule_tracing_wrap(ping); + + // If we have pending work still, restore the original schedulers + restorePendingSchedulers(root, renderExpirationTime); } thenable.then(ping, ping); } @@ -318,6 +322,9 @@ function throwException( let retry = resolveRetryThenable.bind(null, workInProgress, thenable); if (enableSchedulerTracing) { retry = Schedule_tracing_wrap(retry); + + // If we have pending work still, restore the original schedulers + restorePendingSchedulers(root, renderExpirationTime); } thenable.then(retry, retry); } diff --git a/packages/react-reconciler/src/__tests__/ReactSchedulers-test.internal.js b/packages/react-reconciler/src/__tests__/ReactSchedulers-test.internal.js new file mode 100644 index 0000000000000..0d470eca1c8ab --- /dev/null +++ b/packages/react-reconciler/src/__tests__/ReactSchedulers-test.internal.js @@ -0,0 +1,201 @@ +/** + * Copyright (c) Facebook, Inc. and its affiliates. + * + * This source code is licensed under the MIT license found in the + * LICENSE file in the root directory of this source tree. + * + * @emails react-core + */ + +'use strict'; + +let React; +let ReactFeatureFlags; +let ReactDOM; +let Scheduler; +let TestUtils; +let mockDevToolsHook; + +function loadModules({ + enableProfilerTimer = true, + enableSchedulerTracing = true, +} = {}) { + jest.resetModules(); + + mockDevToolsHook = { + injectInternals: jest.fn(() => {}), + onCommitRoot: jest.fn(() => {}), + onCommitUnmount: jest.fn(() => {}), + isDevToolsPresent: true, + }; + + jest.mock( + 'react-reconciler/src/ReactFiberDevToolsHook', + () => mockDevToolsHook, + ); + + ReactFeatureFlags = require('shared/ReactFeatureFlags'); + ReactFeatureFlags.enableProfilerTimer = enableProfilerTimer; + ReactFeatureFlags.enableSchedulerTracing = enableSchedulerTracing; + + React = require('react'); + ReactDOM = require('react-dom'); + Scheduler = require('scheduler'); + TestUtils = require('react-dom/test-utils'); +} + +describe('schedulers', () => { + beforeEach(loadModules); + + it('should report the (host) root as the scheduler for root-level render', () => { + const {HostRoot} = require('shared/ReactWorkTags'); + + const Parent = () => ; + const Child = () => null; + const container = document.createElement('div'); + + TestUtils.act(() => { + ReactDOM.render(, container); + }); + expect(mockDevToolsHook.onCommitRoot).toHaveBeenCalledTimes(1); + let root = mockDevToolsHook.onCommitRoot.mock.calls[0][0]; + expect(root.memoizedSchedulers.size).toBe(1); + expect(root.memoizedSchedulers.values().next().value.tag).toBe(HostRoot); + + mockDevToolsHook.onCommitRoot.mockClear(); + + TestUtils.act(() => { + ReactDOM.render(, container); + }); + expect(mockDevToolsHook.onCommitRoot).toHaveBeenCalledTimes(1); + root = mockDevToolsHook.onCommitRoot.mock.calls[0][0]; + expect(root.memoizedSchedulers.size).toBe(1); + expect(root.memoizedSchedulers.values().next().value.tag).toBe(HostRoot); + }); + + it('should report a function component as the scheduler for a hooks update', () => { + const Parent = () => ; + const SchedulingComponent = () => { + const tuple = React.useState(0); + setCount = tuple[1]; + return ; + }; + const Child = () => null; + let setCount = null; + TestUtils.act(() => { + ReactDOM.render(, document.createElement('div')); + }); + + mockDevToolsHook.onCommitRoot.mockClear(); + + expect(setCount).not.toBeNull(); + TestUtils.act(() => { + setCount(1); + }); + expect(mockDevToolsHook.onCommitRoot).toHaveBeenCalledTimes(1); + const root = mockDevToolsHook.onCommitRoot.mock.calls[0][0]; + expect(root.memoizedSchedulers.size).toBe(1); + expect(root.memoizedSchedulers.values().next().value.elementType).toBe( + SchedulingComponent, + ); + }); + + it('should report a class component as the scheduler for a setState update', () => { + const Parent = () => ; + class SchedulingComponent extends React.Component { + state = {}; + render() { + instance = this; + return ; + } + } + const Child = () => null; + let instance; + TestUtils.act(() => { + ReactDOM.render(, document.createElement('div')); + }); + + mockDevToolsHook.onCommitRoot.mockClear(); + + expect(instance).not.toBeNull(); + TestUtils.act(() => { + instance.setState({}); + }); + expect(mockDevToolsHook.onCommitRoot).toHaveBeenCalledTimes(1); + const root = mockDevToolsHook.onCommitRoot.mock.calls[0][0]; + expect(root.memoizedSchedulers.size).toBe(1); + expect(root.memoizedSchedulers.values().next().value.elementType).toBe( + SchedulingComponent, + ); + }); + + it('should cover suspense pings', async done => { + let data = null; + let resolver = null; + let promise = null; + const fakeCacheRead = () => { + if (data === null) { + promise = new Promise(resolve => { + resolver = resolvedData => { + data = resolvedData; + resolve(resolvedData); + }; + }); + throw promise; + } else { + return data; + } + }; + const Parent = () => ( + }> + + + ); + const Fallback = () => null; + let setShouldSuspend = null; + const Suspender = ({suspend}) => { + const tuple = React.useState(false); + setShouldSuspend = tuple[1]; + if (tuple[0] === true) { + return fakeCacheRead(); + } else { + return null; + } + }; + + TestUtils.act(() => { + ReactDOM.render(, document.createElement('div')); + }); + expect(setShouldSuspend).not.toBeNull(); + expect(mockDevToolsHook.onCommitRoot).toHaveBeenCalledTimes(1); + let root = mockDevToolsHook.onCommitRoot.mock.calls[0][0]; + + mockDevToolsHook.onCommitRoot.mockClear(); + + TestUtils.act(() => { + setShouldSuspend(true); + }); + expect(mockDevToolsHook.onCommitRoot).toHaveBeenCalledTimes(1); + root = mockDevToolsHook.onCommitRoot.mock.calls[0][0]; + expect(root.memoizedSchedulers.size).toBe(1); + expect(root.memoizedSchedulers.values().next().value.elementType).toBe( + Suspender, + ); + + mockDevToolsHook.onCommitRoot.mockClear(); + + expect(resolver).not.toBeNull(); + await TestUtils.act(() => { + resolver('abc'); + return promise; + }); + Scheduler.flushAll(); + expect(mockDevToolsHook.onCommitRoot).toHaveBeenCalledTimes(1); + root = mockDevToolsHook.onCommitRoot.mock.calls[0][0]; + expect(root.memoizedSchedulers.size).toBe(1); + expect(root.memoizedSchedulers.values().next().value.elementType).toBe( + Suspender, + ); + done(); + }); +});