From 1faf9e3dd5d6492f3607d5c721055819e4106bc6 Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Wed, 30 Sep 2020 14:57:33 -0500 Subject: [PATCH] Suspense for CPU-bound trees (#19936) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Adds a new prop to the Suspense component type, `unstable_expectedLoadTime`. The presence of this prop indicates that the content is computationally expensive to render. During the initial mount, React will skip over expensive trees by rendering a placeholder — just like we do with trees that are waiting for data to resolve. That will help unblock the initial skeleton for the new screen. Then we will continue rendering in the next commit. For now, while we experiment with the API internally, any number passed to `unstable_expectedLoadTime` will be treated as "computationally expensive", no matter how large or small. So it's basically a boolean. The reason it's a number is that, in the future, we may try to be clever with this additional information. For example, SuspenseList could use it as part of its heuristic to determine whether to keep rendering additional rows. Background ---------- Much of our early messaging and research into Suspense focused on its ability to throttle the appearance of placeholder UIs. Our theory was that, on a fast network, if everything loads quickly, excessive placeholders will contribute to a janky user experience. This was backed up by user research and has held up in practice. However, our original demos made an even stronger assertion: not only is it preferable to throttle successive loading states, but up to a certain threshold, it’s also preferable to remain on the previous screen; or in other words, to delay the transition. This strategy has produced mixed results. We’ve found it works well for certain transitions, but not for all them. When performing a full page transition, showing an initial skeleton as soon as possible is crucial to making the transition feel snappy. You still want throttle the nested loading states as they pop in, but you need to show something on the new route. Remaining on the previous screen can make the app feel unresponsive. That’s not to say that delaying the previous screen always leads to a bad user experience. Especially if you can guarantee that the delay is small enough that the user won’t notice it. This threshold is a called a Just Noticeable Difference (JND). If we can stay under the JND, then it’s worth skipping the first placeholder to reduce overall thrash. Delays that are larger than the JND have some use cases, too. The main one we’ve found is to refresh existing data, where it’s often preferable to keep stale content on screen while the new data loads in the background. It’s also useful as a fallback strategy if something suspends unexpectedly, to avoid hiding parts of the UI that are already visible. We’re still in the process of optimizing our heuristics for the most common patterns. In general, though, we are trending toward being more aggressive about prioritizing the initial skeleton. For example, Suspense is usually thought of as a feature for displaying placeholders when the UI is missing data — that is, when rendering is bound by pending IO. But it turns out that the same principles apply to CPU-bound transitions, too. It’s worth deferring a tree that’s slow to render if doing so unblocks the rest of the transition — regardless of whether it’s slow because of missing data or because of expensive CPU work. We already take advantage of this idea in a few places, such as hydration. Instead of hydrating server-rendered UI in a single pass, React splits it into chunks. It can do this because the initial HTML acts as its own placeholder. React can defer hydrating a chunk of UI as long as it wants until the user interacts it. The boundary we use to split the UI into chunks is the same one we use for IO-bound subtrees: the component. SuspenseList does something similar. When streaming in a list of items, it will occasionally stop to commit whatever items have already finished, before continuing where it left off. It does this by showing a placeholder for the remaining items, again using the same component API, even if the item is CPU-bound. Unresolved questions -------------------- There is a concern that showing a placeholder without also loading new data could be disorienting. Users are trained to believe that a placeholder signals fresh content. So there are still some questions we’ll need to resolve. --- .../src/ReactFiberBeginWork.new.js | 36 ++- .../src/ReactFiberBeginWork.old.js | 36 ++- .../src/ReactFiberSuspenseComponent.new.js | 11 + .../src/ReactFiberSuspenseComponent.old.js | 11 + .../src/__tests__/ReactCPUSuspense-test.js | 287 ++++++++++++++++++ 5 files changed, 375 insertions(+), 6 deletions(-) create mode 100644 packages/react-reconciler/src/__tests__/ReactCPUSuspense-test.js diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.new.js b/packages/react-reconciler/src/ReactFiberBeginWork.new.js index 198d07b87727..99dd385674ee 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.new.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.new.js @@ -109,6 +109,7 @@ import { SyncLane, OffscreenLane, DefaultHydrationLane, + SomeRetryLane, NoTimestamp, includesSomeLane, laneToLanes, @@ -1658,6 +1659,7 @@ function updateSuspenseOffscreenState( }; } +// TODO: Probably should inline this back function shouldRemainOnFallback( suspenseContext: SuspenseContext, current: null | Fiber, @@ -1790,9 +1792,9 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) { } } + const nextPrimaryChildren = nextProps.children; + const nextFallbackChildren = nextProps.fallback; if (showFallback) { - const nextPrimaryChildren = nextProps.children; - const nextFallbackChildren = nextProps.fallback; const fallbackFragment = mountSuspenseFallbackChildren( workInProgress, nextPrimaryChildren, @@ -1805,8 +1807,36 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) { ); workInProgress.memoizedState = SUSPENDED_MARKER; return fallbackFragment; + } else if (typeof nextProps.unstable_expectedLoadTime === 'number') { + // This is a CPU-bound tree. Skip this tree and show a placeholder to + // unblock the surrounding content. Then immediately retry after the + // initial commit. + const fallbackFragment = mountSuspenseFallbackChildren( + workInProgress, + nextPrimaryChildren, + nextFallbackChildren, + renderLanes, + ); + const primaryChildFragment: Fiber = (workInProgress.child: any); + primaryChildFragment.memoizedState = mountSuspenseOffscreenState( + renderLanes, + ); + workInProgress.memoizedState = SUSPENDED_MARKER; + + // Since nothing actually suspended, there will nothing to ping this to + // get it started back up to attempt the next item. While in terms of + // priority this work has the same priority as this current render, it's + // not part of the same transition once the transition has committed. If + // it's sync, we still want to yield so that it can be painted. + // Conceptually, this is really the same as pinging. We can use any + // RetryLane even if it's the one currently rendering since we're leaving + // it behind on this node. + workInProgress.lanes = SomeRetryLane; + if (enableSchedulerTracing) { + markSpawnedWork(SomeRetryLane); + } + return fallbackFragment; } else { - const nextPrimaryChildren = nextProps.children; return mountSuspensePrimaryChildren( workInProgress, nextPrimaryChildren, diff --git a/packages/react-reconciler/src/ReactFiberBeginWork.old.js b/packages/react-reconciler/src/ReactFiberBeginWork.old.js index 23a97b297ce6..ef41dff89a0e 100644 --- a/packages/react-reconciler/src/ReactFiberBeginWork.old.js +++ b/packages/react-reconciler/src/ReactFiberBeginWork.old.js @@ -109,6 +109,7 @@ import { SyncLane, OffscreenLane, DefaultHydrationLane, + SomeRetryLane, NoTimestamp, includesSomeLane, laneToLanes, @@ -1657,6 +1658,7 @@ function updateSuspenseOffscreenState( }; } +// TODO: Probably should inline this back function shouldRemainOnFallback( suspenseContext: SuspenseContext, current: null | Fiber, @@ -1789,9 +1791,9 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) { } } + const nextPrimaryChildren = nextProps.children; + const nextFallbackChildren = nextProps.fallback; if (showFallback) { - const nextPrimaryChildren = nextProps.children; - const nextFallbackChildren = nextProps.fallback; const fallbackFragment = mountSuspenseFallbackChildren( workInProgress, nextPrimaryChildren, @@ -1804,8 +1806,36 @@ function updateSuspenseComponent(current, workInProgress, renderLanes) { ); workInProgress.memoizedState = SUSPENDED_MARKER; return fallbackFragment; + } else if (typeof nextProps.unstable_expectedLoadTime === 'number') { + // This is a CPU-bound tree. Skip this tree and show a placeholder to + // unblock the surrounding content. Then immediately retry after the + // initial commit. + const fallbackFragment = mountSuspenseFallbackChildren( + workInProgress, + nextPrimaryChildren, + nextFallbackChildren, + renderLanes, + ); + const primaryChildFragment: Fiber = (workInProgress.child: any); + primaryChildFragment.memoizedState = mountSuspenseOffscreenState( + renderLanes, + ); + workInProgress.memoizedState = SUSPENDED_MARKER; + + // Since nothing actually suspended, there will nothing to ping this to + // get it started back up to attempt the next item. While in terms of + // priority this work has the same priority as this current render, it's + // not part of the same transition once the transition has committed. If + // it's sync, we still want to yield so that it can be painted. + // Conceptually, this is really the same as pinging. We can use any + // RetryLane even if it's the one currently rendering since we're leaving + // it behind on this node. + workInProgress.lanes = SomeRetryLane; + if (enableSchedulerTracing) { + markSpawnedWork(SomeRetryLane); + } + return fallbackFragment; } else { - const nextPrimaryChildren = nextProps.children; return mountSuspensePrimaryChildren( workInProgress, nextPrimaryChildren, diff --git a/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js b/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js index 2f9d9f365a98..385a2ebdfa67 100644 --- a/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js +++ b/packages/react-reconciler/src/ReactFiberSuspenseComponent.new.js @@ -7,6 +7,7 @@ * @flow */ +import type {ReactNodeList, Wakeable} from 'shared/ReactTypes'; import type {Fiber} from './ReactInternalTypes'; import type {SuspenseInstance} from './ReactFiberHostConfig'; import type {Lane} from './ReactFiberLane'; @@ -17,6 +18,16 @@ import { isSuspenseInstanceFallback, } from './ReactFiberHostConfig'; +export type SuspenseProps = {| + children?: ReactNodeList, + fallback?: ReactNodeList, + + // TODO: Add "unstable_" prefix? + suspenseCallback?: (Set | null) => mixed, + + unstable_expectedLoadTime?: number, +|}; + // A null SuspenseState represents an unsuspended normal Suspense boundary. // A non-null SuspenseState means that it is blocked for one reason or another. // - A non-null dehydrated field means it's blocked pending hydration. diff --git a/packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js b/packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js index ee8f4c891892..6e14aacd77de 100644 --- a/packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js +++ b/packages/react-reconciler/src/ReactFiberSuspenseComponent.old.js @@ -7,6 +7,7 @@ * @flow */ +import type {ReactNodeList, Wakeable} from 'shared/ReactTypes'; import type {Fiber} from './ReactInternalTypes'; import type {SuspenseInstance} from './ReactFiberHostConfig'; import type {Lane} from './ReactFiberLane'; @@ -17,6 +18,16 @@ import { isSuspenseInstanceFallback, } from './ReactFiberHostConfig'; +export type SuspenseProps = {| + children?: ReactNodeList, + fallback?: ReactNodeList, + + // TODO: Add "unstable_" prefix? + suspenseCallback?: (Set | null) => mixed, + + unstable_expectedLoadTime?: number, +|}; + // A null SuspenseState represents an unsuspended normal Suspense boundary. // A non-null SuspenseState means that it is blocked for one reason or another. // - A non-null dehydrated field means it's blocked pending hydration. diff --git a/packages/react-reconciler/src/__tests__/ReactCPUSuspense-test.js b/packages/react-reconciler/src/__tests__/ReactCPUSuspense-test.js new file mode 100644 index 000000000000..1de9050caa4a --- /dev/null +++ b/packages/react-reconciler/src/__tests__/ReactCPUSuspense-test.js @@ -0,0 +1,287 @@ +let React; +let ReactNoop; +let Scheduler; +let Suspense; +let useState; +let textCache; + +let readText; +let resolveText; +// let rejectText; + +describe('ReactSuspenseWithNoopRenderer', () => { + beforeEach(() => { + jest.resetModules(); + + React = require('react'); + ReactNoop = require('react-noop-renderer'); + Scheduler = require('scheduler'); + Suspense = React.Suspense; + useState = React.useState; + + textCache = new Map(); + + readText = text => { + const record = textCache.get(text); + if (record !== undefined) { + switch (record.status) { + case 'pending': + throw record.promise; + case 'rejected': + throw Error('Failed to load: ' + text); + case 'resolved': + return text; + } + } else { + let ping; + const promise = new Promise(resolve => (ping = resolve)); + const newRecord = { + status: 'pending', + ping: ping, + promise, + }; + textCache.set(text, newRecord); + throw promise; + } + }; + + resolveText = text => { + const record = textCache.get(text); + if (record !== undefined) { + if (record.status === 'pending') { + record.ping(); + record.ping = null; + record.status = 'resolved'; + record.promise = null; + } + } else { + const newRecord = { + ping: null, + status: 'resolved', + promise: null, + }; + textCache.set(text, newRecord); + } + }; + + // rejectText = text => { + // const record = textCache.get(text); + // if (record !== undefined) { + // if (record.status === 'pending') { + // Scheduler.unstable_yieldValue(`Promise rejected [${text}]`); + // record.ping(); + // record.status = 'rejected'; + // clearTimeout(record.promise._timer); + // record.promise = null; + // } + // } else { + // const newRecord = { + // ping: null, + // status: 'rejected', + // promise: null, + // }; + // textCache.set(text, newRecord); + // } + // }; + }); + + function Text(props) { + Scheduler.unstable_yieldValue(props.text); + return props.text; + } + + function AsyncText(props) { + const text = props.text; + try { + readText(text); + Scheduler.unstable_yieldValue(text); + return text; + } catch (promise) { + if (typeof promise.then === 'function') { + Scheduler.unstable_yieldValue(`Suspend! [${text}]`); + } else { + Scheduler.unstable_yieldValue(`Error! [${text}]`); + } + throw promise; + } + } + + it('skips CPU-bound trees on initial mount', async () => { + function App() { + return ( + <> + +
+ }> + + +
+ + ); + } + + const root = ReactNoop.createRoot(); + await ReactNoop.act(async () => { + root.render(); + expect(Scheduler).toFlushUntilNextPaint(['Outer', 'Loading...']); + expect(root).toMatchRenderedOutput( + <> + Outer +
Loading...
+ , + ); + }); + // Inner contents finish in separate commit from outer + expect(Scheduler).toHaveYielded(['Inner']); + expect(root).toMatchRenderedOutput( + <> + Outer +
Inner
+ , + ); + }); + + it('does not skip CPU-bound trees during updates', async () => { + let setCount; + + function App() { + const [count, _setCount] = useState(0); + setCount = _setCount; + return ( + <> + +
+ }> + + +
+ + ); + } + + // Initial mount + const root = ReactNoop.createRoot(); + await ReactNoop.act(async () => { + root.render(); + }); + // Inner contents finish in separate commit from outer + expect(Scheduler).toHaveYielded(['Outer', 'Loading...', 'Inner [0]']); + expect(root).toMatchRenderedOutput( + <> + Outer +
Inner [0]
+ , + ); + + // Update + await ReactNoop.act(async () => { + setCount(1); + }); + // Entire update finishes in a single commit + expect(Scheduler).toHaveYielded(['Outer', 'Inner [1]']); + expect(root).toMatchRenderedOutput( + <> + Outer +
Inner [1]
+ , + ); + }); + + it('suspend inside CPU-bound tree', async () => { + function App() { + return ( + <> + +
+ }> + + +
+ + ); + } + + const root = ReactNoop.createRoot(); + await ReactNoop.act(async () => { + root.render(); + expect(Scheduler).toFlushUntilNextPaint(['Outer', 'Loading...']); + expect(root).toMatchRenderedOutput( + <> + Outer +
Loading...
+ , + ); + }); + // Inner contents suspended, so we continue showing a fallback. + expect(Scheduler).toHaveYielded(['Suspend! [Inner]']); + expect(root).toMatchRenderedOutput( + <> + Outer +
Loading...
+ , + ); + + // Resolve the data and finish rendering + await ReactNoop.act(async () => { + await resolveText('Inner'); + }); + expect(Scheduler).toHaveYielded(['Inner']); + expect(root).toMatchRenderedOutput( + <> + Outer +
Inner
+ , + ); + }); + + it('nested CPU-bound trees', async () => { + function App() { + return ( + <> + +
+ }> + +
+ }> + + +
+ +
+ + ); + } + + const root = ReactNoop.createRoot(); + await ReactNoop.act(async () => { + root.render(); + }); + // Each level commits separately + expect(Scheduler).toHaveYielded([ + 'A', + 'Loading B...', + 'B', + 'Loading C...', + 'C', + ]); + expect(root).toMatchRenderedOutput( + <> + A +
+ B
C
+
+ , + ); + }); +});