diff --git a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js index ba2074789304..2fcf5bf9a57f 100644 --- a/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js +++ b/packages/react-dom-bindings/src/client/ReactFiberConfigDOM.js @@ -3154,7 +3154,6 @@ export function mayResourceSuspendCommit(resource: Resource): boolean { } export function preloadInstance(type: Type, props: Props): boolean { - // Return true to indicate it's already loaded return true; } @@ -3163,10 +3162,11 @@ export function preloadResource(resource: Resource): boolean { resource.type === 'stylesheet' && (resource.state.loading & Settled) === NotLoaded ) { - // we have not finished loading the underlying stylesheet yet. + // Return false to indicate this resource should suspend return false; } - // Return true to indicate it's already loaded + + // Return true to indicate this resource should not suspend return true; } diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js index b2331c17f282..43926bfa160f 100644 --- a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js +++ b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js @@ -369,11 +369,15 @@ describe('ReactDOMFloat', () => { } function loadStylesheets(hrefs) { + loadStylesheetsFrom(document, hrefs); + } + + function loadStylesheetsFrom(root, hrefs) { const event = new window.Event('load'); - const nodes = document.querySelectorAll('link[rel="stylesheet"]'); - resolveLoadables(hrefs, nodes, event, href => - Scheduler.log('load stylesheet: ' + href), - ); + const nodes = root.querySelectorAll('link[rel="stylesheet"]'); + resolveLoadables(hrefs, nodes, event, href => { + Scheduler.log('load stylesheet: ' + href); + }); } function errorStylesheets(hrefs) { @@ -832,6 +836,8 @@ describe('ReactDOMFloat', () => { , ); await waitForAll([]); + loadPreloads(); + await assertLog(['load preload: foo']); expect(getMeaningfulChildren(document)).toEqual( @@ -849,15 +855,9 @@ describe('ReactDOMFloat', () => { resolveText('bar'); }); await act(() => { - const sheets = document.querySelectorAll( - 'link[rel="stylesheet"][data-precedence]', - ); - const event = document.createEvent('Event'); - event.initEvent('load', true, true); - for (let i = 0; i < sheets.length; i++) { - sheets[i].dispatchEvent(event); - } + loadStylesheets(); }); + await assertLog(['load stylesheet: foo', 'load stylesheet: bar']); expect(getMeaningfulChildren(document)).toEqual( @@ -877,15 +877,9 @@ describe('ReactDOMFloat', () => { resolveText('foo'); }); await act(() => { - const sheets = document.querySelectorAll( - 'link[rel="stylesheet"][data-precedence]', - ); - const event = document.createEvent('Event'); - event.initEvent('load', true, true); - for (let i = 0; i < sheets.length; i++) { - sheets[i].dispatchEvent(event); - } + loadStylesheets(); }); + await assertLog([]); expect(getMeaningfulChildren(document)).toEqual( @@ -1128,6 +1122,30 @@ body { , ); await waitForAll([]); + await act(() => { + loadPreloads(); + loadStylesheets(); + }); + await assertLog([ + 'load preload: one4', + 'load preload: three4', + 'load preload: seven1', + 'load preload: one2', + 'load preload: two2', + 'load preload: five1', + 'load preload: three3', + 'load preload: four3', + 'load stylesheet: one1', + 'load stylesheet: one2', + 'load stylesheet: one4', + 'load stylesheet: two2', + 'load stylesheet: three1', + 'load stylesheet: three3', + 'load stylesheet: three4', + 'load stylesheet: four3', + 'load stylesheet: five1', + 'load stylesheet: seven1', + ]); expect(getMeaningfulChildren(document)).toEqual( @@ -1806,6 +1824,16 @@ body { , ); await waitForAll([]); + await act(() => { + loadPreloads(); + loadStylesheets(); + }); + await assertLog([ + 'load preload: baz', + 'load stylesheet: foo', + 'load stylesheet: baz', + 'load stylesheet: bar', + ]); expect(getMeaningfulChildren(document)).toEqual( @@ -2869,6 +2897,16 @@ body { , ); await waitForAll([]); + await act(() => { + loadPreloads(); + loadStylesheets(); + }); + await assertLog([ + 'load preload: third', + 'load stylesheet: first', + 'load stylesheet: second', + 'load stylesheet: third', + ]); expect(getMeaningfulChildren(document)).toEqual( @@ -2946,25 +2984,8 @@ body { , ]); - // Try just this and crash all of Jest errorStylesheets(['bar']); - // // Try this and it fails the test when it shouldn't - // await act(() => { - // errorStylesheets(['bar']); - // }); - - // // Try this there is nothing throwing here which is not really surprising since - // // the error is bubbling up through some kind of unhandled promise rejection thingy but - // // still I thought it was worth confirming - // try { - // await act(() => { - // errorStylesheets(['bar']); - // }); - // } catch (e) { - // console.log(e); - // } - loadStylesheets(['foo']); assertLog(['load stylesheet: foo', 'error stylesheet: bar']); @@ -3127,6 +3148,11 @@ body { , ); await waitForAll([]); + await act(() => { + loadPreloads(['bar']); + loadStylesheets(['bar']); + }); + await assertLog(['load preload: bar', 'load stylesheet: bar']); // The bar stylesheet was inserted. There's still a "foo" preload, even // though that update was superseded. @@ -3197,6 +3223,131 @@ body { ); }); + it('will put a Suspense boundary into fallback if it contains a stylesheet not loaded during a sync update', async () => { + function App({children}) { + return ( + + {children} + + ); + } + const root = ReactDOMClient.createRoot(document); + + await clientAct(() => { + root.render(); + }); + await waitForAll([]); + + await clientAct(() => { + root.render( + + +
+ hello + +
+ + , + ); + }); + await waitForAll([]); + + // Although the commit suspended, a preload was inserted. + expect(getMeaningfulChildren(document)).toEqual( + + + + + loading... + , + ); + + loadPreloads(['foo']); + assertLog(['load preload: foo']); + expect(getMeaningfulChildren(document)).toEqual( + + + + + + loading... + , + ); + + loadStylesheets(['foo']); + assertLog(['load stylesheet: foo']); + expect(getMeaningfulChildren(document)).toEqual( + + + + + + +
hello
+ + , + ); + + await clientAct(() => { + root.render( + + +
+ hello + + +
+ + , + ); + }); + await waitForAll([]); + expect(getMeaningfulChildren(document)).toEqual( + + + + + + + +
hello
loading... + + , + ); + + loadPreloads(['bar']); + assertLog(['load preload: bar']); + expect(getMeaningfulChildren(document)).toEqual( + + + + + + + + +
hello
loading... + + , + ); + + loadStylesheets(['bar']); + assertLog(['load stylesheet: bar']); + expect(getMeaningfulChildren(document)).toEqual( + + + + + + + + +
hello
+ + , + ); + }); + it('can suspend commits on more than one root for the same resource at the same time', async () => { document.body.innerHTML = ''; const container1 = document.createElement('div'); @@ -6465,6 +6616,14 @@ body { , ); await waitForAll([]); + await act(() => { + loadPreloads(); + loadStylesheets(); + }); + await assertLog([ + 'load preload: aresource', + 'load stylesheet: aresource', + ]); expect(getMeaningfulChildren(document)).toEqual( @@ -6556,6 +6715,22 @@ body { , ); await waitForAll([]); + await act(() => { + loadPreloads(); + loadStylesheets(); + }); + await assertLog([ + 'load preload: bar1', + 'load preload: foo3', + 'load preload: default2', + 'load stylesheet: foo1', + 'load stylesheet: foo2', + 'load stylesheet: foo3', + 'load stylesheet: default1', + 'load stylesheet: default2', + 'load stylesheet: bar1', + ]); + expect(getMeaningfulChildren(document)).toEqual( @@ -6590,6 +6765,11 @@ body { , ); await waitForAll([]); + await act(() => { + loadPreloads(); + loadStylesheets(); + }); + await assertLog(['load preload: foo', 'load stylesheet: foo']); root.render( @@ -6640,6 +6820,17 @@ body { }, ); await waitForAll([]); + await act(() => { + loadPreloads(); + loadStylesheets(); + }); + await assertLog([ + 'load preload: qux', + 'load stylesheet: foo', + 'load stylesheet: bar', + 'load stylesheet: qux', + ]); + expect(getMeaningfulChildren(document)).toEqual( @@ -6666,6 +6857,16 @@ body { , ); await waitForAll([]); + await act(() => { + loadPreloads(); + loadStylesheets(); + }); + await assertLog([ + 'load preload: foo', + 'load preload: bar', + 'load stylesheet: foo', + 'load stylesheet: bar', + ]); expect(getMeaningfulChildren(document)).toEqual( @@ -6688,6 +6889,12 @@ body { , ); await waitForAll([]); + await act(() => { + loadPreloads(); + loadStylesheets(); + }); + await assertLog(['load preload: baz', 'load stylesheet: baz']); + // The reason we do not see preloads in the head is they are inserted synchronously // during render and then when the new singleton mounts it resets it's content, retaining only styles expect(getMeaningfulChildren(document)).toEqual( @@ -6702,6 +6909,7 @@ body { , ); }); + it('can support styles inside portals to a shadowRoot', async () => { const shadow = document.body.attachShadow({mode: 'open'}); const root = ReactDOMClient.createRoot(container); @@ -6724,6 +6932,16 @@ body { , ); await waitForAll([]); + await act(() => { + loadPreloads(); + loadStylesheets(); + loadStylesheetsFrom(shadow); + }); + await assertLog([ + 'load preload: foo', + 'load stylesheet: foo', + 'load stylesheet: foo', + ]); expect(getMeaningfulChildren(document)).toEqual( @@ -6745,6 +6963,7 @@ body {
shadow
, ]); }); + it('can support styles inside portals to an element in shadowRoots', async () => { const template = document.createElement('template'); template.innerHTML = @@ -6783,6 +7002,24 @@ body { , ); await waitForAll([]); + await act(() => { + loadPreloads(); + loadStylesheets(); + loadStylesheetsFrom(shadow); + loadStylesheetsFrom(shadowContainer2); + loadStylesheetsFrom(shadowContainer2); + }); + await assertLog([ + 'load preload: foo', + 'load preload: bar', + 'load preload: baz', + 'load preload: qux', + 'load stylesheet: foo', + 'load stylesheet: foo', + 'load stylesheet: baz', + 'load stylesheet: bar', + 'load stylesheet: qux', + ]); expect(getMeaningfulChildren(document)).toEqual( @@ -6854,6 +7091,18 @@ body { } root.render(); await waitForAll([]); + await act(() => { + loadPreloads(); + loadStylesheets(); + }); + await assertLog([ + 'load preload: preload', + 'load preload: with\nnewline', + 'load preload: style"][rel="stylesheet', + 'load stylesheet: style', + 'load stylesheet: with\\slashes', + 'load stylesheet: style"][rel="stylesheet', + ]); expect(getMeaningfulChildren(document)).toEqual( diff --git a/packages/react-native-renderer/src/ReactFiberConfigNative.js b/packages/react-native-renderer/src/ReactFiberConfigNative.js index 3e45bd0db538..f69457c52073 100644 --- a/packages/react-native-renderer/src/ReactFiberConfigNative.js +++ b/packages/react-native-renderer/src/ReactFiberConfigNative.js @@ -535,7 +535,7 @@ export function maySuspendCommit(type: Type, props: Props): boolean { } export function preloadInstance(type: Type, props: Props): boolean { - // Return true to indicate it's already loaded + // Return false to indicate it's already loaded return true; } diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js index 32c3a360fd2c..9f8472df40e3 100644 --- a/packages/react-noop-renderer/src/createReactNoop.js +++ b/packages/react-noop-renderer/src/createReactNoop.js @@ -611,12 +611,11 @@ function createReactNoop(reconciler: Function, useMutation: boolean) { } return false; } else { - // If this is false, React will trigger a fallback, if needed. return record.status === 'fulfilled'; } }, - preloadResource(resource: mixed): boolean { + preloadResource(resource: mixed): number { throw new Error( 'Resources are not implemented for React Noop yet. This method should not be called', ); diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js index 3e8921d30e02..a060d0f00aed 100644 --- a/packages/react-reconciler/src/ReactFiberCompleteWork.js +++ b/packages/react-reconciler/src/ReactFiberCompleteWork.js @@ -152,7 +152,6 @@ import { getRenderTargetTime, getWorkInProgressTransitions, shouldRemainOnPreviousScreen, - getWorkInProgressRootRenderLanes, } from './ReactFiberWorkLoop'; import { OffscreenLane, @@ -161,7 +160,6 @@ import { includesSomeLane, mergeLanes, claimNextRetryLane, - includesOnlyNonUrgentLanes, } from './ReactFiberLane'; import {resetChildFibers} from './ReactChildFiber'; import {createScopeInstance} from './ReactFiberScope'; @@ -534,41 +532,15 @@ function preloadInstanceAndSuspendIfNeeded( // loaded yet. workInProgress.flags |= MaySuspendCommit; - // Check if we're rendering at a "non-urgent" priority. This is the same - // check that `useDeferredValue` does to determine whether it needs to - // defer. This is partly for gradual adoption purposes (i.e. shouldn't start - // suspending until you opt in with startTransition or Suspense) but it - // also happens to be the desired behavior for the concrete use cases we've - // thought of so far, like CSS loading, fonts, images, etc. - // - // We check the "root" render lanes here rather than the "subtree" render - // because during a retry or offscreen prerender, the "subtree" render - // lanes may include additional "base" lanes that were deferred during - // a previous render. - // TODO: We may decide to expose a way to force a fallback even during a - // sync update. - const rootRenderLanes = getWorkInProgressRootRenderLanes(); - if (!includesOnlyNonUrgentLanes(rootRenderLanes)) { - // This is an urgent render. Don't suspend or show a fallback. Also, - // there's no need to preload, because we're going to commit this - // synchronously anyway. - // TODO: Could there be benefit to preloading even during a synchronous - // render? The main thread will be blocked until the commit phase, but - // maybe the browser would be able to start loading off thread anyway? - // Likely a micro-optimization either way because typically new content - // is loaded during a transition, not an urgent render. - } else { - // Preload the instance - const isReady = preloadInstance(type, props); - if (!isReady) { - if (shouldRemainOnPreviousScreen()) { - // It's OK to suspend. Mark the fiber so we know to suspend before the - // commit phase. Then continue rendering. - workInProgress.flags |= ShouldSuspendCommit; - } else { - // Trigger a fallback rather than block the render. - suspendCommit(); - } + // preload the instance if necessary. Even if this is an urgent render there + // could be benefits to preloading early. + // @TODO we should probably do the preload in begin work + const isReady = preloadInstance(type, props); + if (!isReady) { + if (shouldRemainOnPreviousScreen()) { + workInProgress.flags |= ShouldSuspendCommit; + } else { + suspendCommit(); } } } @@ -588,17 +560,12 @@ function preloadResourceAndSuspendIfNeeded( workInProgress.flags |= MaySuspendCommit; - const rootRenderLanes = getWorkInProgressRootRenderLanes(); - if (!includesOnlyNonUrgentLanes(rootRenderLanes)) { - // This is an urgent render. Don't suspend or show a fallback. - } else { - const isReady = preloadResource(resource); - if (!isReady) { - if (shouldRemainOnPreviousScreen()) { - workInProgress.flags |= ShouldSuspendCommit; - } else { - suspendCommit(); - } + const isReady = preloadResource(resource); + if (!isReady) { + if (shouldRemainOnPreviousScreen()) { + workInProgress.flags |= ShouldSuspendCommit; + } else { + suspendCommit(); } } } diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js index 00391bb354ee..e68b4de70f99 100644 --- a/packages/react-reconciler/src/ReactFiberWorkLoop.js +++ b/packages/react-reconciler/src/ReactFiberWorkLoop.js @@ -23,6 +23,7 @@ import type { } from './ReactFiberTracingMarkerComponent'; import type {OffscreenInstance} from './ReactFiberActivityComponent'; import type {RenderTaskFn} from './ReactFiberRootScheduler'; +import type {Resource} from './ReactFiberConfig'; import { enableCreateEventHandleAPI, @@ -75,6 +76,7 @@ import { startSuspendingCommit, waitForCommitToBeReady, preloadInstance, + preloadResource, supportsHydration, setCurrentUpdatePriority, getCurrentUpdatePriority, @@ -123,6 +125,7 @@ import { MountPassiveDev, MountLayoutDev, DidDefer, + ShouldSuspendCommit, } from './ReactFiberFlags'; import { NoLanes, @@ -151,7 +154,6 @@ import { movePendingFibersToMemoized, addTransitionToLanesMap, getTransitionsForLanes, - includesOnlyNonUrgentLanes, includesSomeLane, OffscreenLane, SyncUpdateLanes, @@ -1182,7 +1184,7 @@ function commitRootWhenReady( ) { // TODO: Combine retry throttling with Suspensey commits. Right now they run // one after the other. - if (includesOnlyNonUrgentLanes(lanes)) { + if (finishedWork.subtreeFlags & ShouldSuspendCommit) { // Before committing, ask the renderer whether the host tree is ready. // If it's not, we'll wait until it notifies us. startSuspendingCommit(); @@ -2219,9 +2221,13 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { break; } case SuspendedOnInstanceAndReadyToContinue: { + let resource: null | Resource = null; switch (workInProgress.tag) { + case HostHoistable: { + resource = workInProgress.memoizedState; + } + // intentional fallthrough case HostComponent: - case HostHoistable: case HostSingleton: { // Before unwinding the stack, check one more time if the // instance is ready. It may have loaded when React yielded to @@ -2232,7 +2238,9 @@ function renderRootConcurrent(root: FiberRoot, lanes: Lanes) { const hostFiber = workInProgress; const type = hostFiber.type; const props = hostFiber.pendingProps; - const isReady = preloadInstance(type, props); + const isReady = resource + ? preloadResource(resource) + : preloadInstance(type, props); if (isReady) { // The data resolved. Resume the work loop as if nothing // suspended. Unlike when a user component suspends, we don't diff --git a/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js b/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js index 52fd9ef3f27d..8a52167a934b 100644 --- a/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js +++ b/packages/react-reconciler/src/__tests__/ReactSuspenseyCommitPhase-test.js @@ -44,6 +44,7 @@ describe('ReactSuspenseyCommitPhase', () => { return ( Scheduler.log(`Image requested [${src}]`)} /> ); @@ -104,7 +105,49 @@ describe('ReactSuspenseyCommitPhase', () => { expect(root).toMatchRenderedOutput(); }); - test('does not suspend commit during urgent update', async () => { + test('suspend commit during initial mount at the root', async () => { + const root = ReactNoop.createRoot(); + await act(async () => { + startTransition(() => { + root.render(); + }); + }); + assertLog(['Image requested [A]']); + expect(getSuspenseyThingStatus('A')).toBe('pending'); + expect(root).toMatchRenderedOutput(null); + + resolveSuspenseyThing('A'); + expect(getSuspenseyThingStatus('A')).toBe('fulfilled'); + expect(root).toMatchRenderedOutput(); + }); + + test('suspend commit during update at the root', async () => { + const root = ReactNoop.createRoot(); + await act(() => resolveSuspenseyThing('A')); + expect(getSuspenseyThingStatus('A')).toBe('fulfilled'); + expect(root).toMatchRenderedOutput(null); + await act(async () => { + startTransition(() => { + root.render(); + }); + }); + expect(root).toMatchRenderedOutput(); + + await act(async () => { + startTransition(() => { + root.render(); + }); + }); + assertLog(['Image requested [B]']); + expect(getSuspenseyThingStatus('B')).toBe('pending'); + expect(root).toMatchRenderedOutput(); + + resolveSuspenseyThing('B'); + expect(getSuspenseyThingStatus('B')).toBe('fulfilled'); + expect(root).toMatchRenderedOutput(); + }); + + test('suspend commit during urgent initial mount', async () => { const root = ReactNoop.createRoot(); await act(async () => { root.render( @@ -113,16 +156,141 @@ describe('ReactSuspenseyCommitPhase', () => { , ); }); - // We intentionally don't preload during an urgent update because the - // resource will be inserted synchronously, anyway. - // TODO: Maybe we should, though? Could be that the browser is able to start - // the preload in background even though the main thread is blocked. Likely - // a micro-optimization either way because typically new content is loaded - // during a transition, not an urgent render. - expect(getSuspenseyThingStatus('A')).toBe(null); + assertLog(['Image requested [A]', 'Loading...']); + expect(getSuspenseyThingStatus('A')).toBe('pending'); + expect(root).toMatchRenderedOutput('Loading...'); + + resolveSuspenseyThing('A'); + expect(getSuspenseyThingStatus('A')).toBe('fulfilled'); + expect(root).toMatchRenderedOutput(); + }); + + test('suspend commit during urgent update', async () => { + const root = ReactNoop.createRoot(); + await act(() => resolveSuspenseyThing('A')); + expect(getSuspenseyThingStatus('A')).toBe('fulfilled'); + await act(async () => { + root.render( + }> + + , + ); + }); + expect(root).toMatchRenderedOutput(); + + resolveSuspenseyThing('A'); + expect(getSuspenseyThingStatus('A')).toBe('fulfilled'); + expect(root).toMatchRenderedOutput(); + + await act(async () => { + root.render( + }> + + , + ); + }); + assertLog(['Image requested [B]', 'Loading...']); + expect(getSuspenseyThingStatus('B')).toBe('pending'); + expect(root).toMatchRenderedOutput( + <> +