From 0ae348018d5b3a3f1ccdd92de85d9cc581b2b98d Mon Sep 17 00:00:00 2001 From: Andrew Clark Date: Fri, 31 Mar 2023 15:45:45 -0400 Subject: [PATCH] [Float] Suspend unstyled content for up to 1 minute (#26532) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit We almost never want to show content before its styles have loaded. But eventually we will give up and allow unstyled content. So this extends the timeout to a full minute. This somewhat arbitrary — big enough that you'd only reach it under extreme circumstances. Note that, like regular Suspense, the app is still interactive while we're waiting for content to load. Only the unstyled content is blocked from appearing, not updates in general. A new update will interrupt it. We should figure out what the browser engines do during initial page load and consider aligning our behavior with that. It's supposed to be render blocking by default but there may be some cases where they, too, give up and FOUC. --- .../src/client/ReactDOMHostConfig.js | 36 +++++++++++-------- .../src/__tests__/ReactDOMFloat-test.js | 19 ++++++++-- 2 files changed, 38 insertions(+), 17 deletions(-) diff --git a/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js b/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js index 253b2da401cc8..48b1f41b85fc7 100644 --- a/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js +++ b/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js @@ -3333,28 +3333,34 @@ export function waitForCommitToBeReady(): null | (Function => Function) { // tasks to wait on. if (state.count > 0) { return commit => { - unsuspendAfterTimeout(state); + // We almost never want to show content before its styles have loaded. But + // eventually we will give up and allow unstyled content. So this number is + // somewhat arbitrary — big enough that you'd only reach it under + // extreme circumstances. + // TODO: Figure out what the browser engines do during initial page load and + // consider aligning our behavior with that. + const stylesheetTimer = setTimeout(() => { + if (state.stylesheets) { + insertSuspendedStylesheets(state, state.stylesheets); + } + if (state.unsuspend) { + const unsuspend = state.unsuspend; + state.unsuspend = null; + unsuspend(); + } + }, 60000); // one minute + state.unsuspend = commit; - return () => (state.unsuspend = null); + return () => { + state.unsuspend = null; + clearTimeout(stylesheetTimer); + }; }; } return null; } -function unsuspendAfterTimeout(state: SuspendedState) { - setTimeout(() => { - if (state.stylesheets) { - insertSuspendedStylesheets(state, state.stylesheets); - } - if (state.unsuspend) { - const unsuspend = state.unsuspend; - state.unsuspend = null; - unsuspend(); - } - }, 500); -} - function onUnsuspend(this: SuspendedState) { this.count--; if (this.count === 0) { diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index d089b25ffad03..d362e1b94a4a9 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -3163,7 +3163,7 @@ body { ); }); - it('can unsuspend after a timeout even if some assets never load', async () => { + it('stylesheets block render, with a really long timeout', async () => { function App({children}) { return ( @@ -3191,7 +3191,22 @@ body { , ); - jest.advanceTimersByTime(1000); + // Advance time by 50 seconds. Even still, the transition is suspended. + jest.advanceTimersByTime(50000); + await waitForAll([]); + expect(getMeaningfulChildren(document)).toEqual( + + + + + + , + ); + + // Advance time by 10 seconds more. A full minute total has elapsed. At this + // point, something must have really gone wrong, so we time out and allow + // unstyled content to be displayed. + jest.advanceTimersByTime(10000); expect(getMeaningfulChildren(document)).toEqual(