Implement suspensey css for float

Implements waitForCommitToBeReady for resources. currently it is only opted into when a special prop is passed. This will be removed in the next commit when I update all the tests that now require different mechanics to simulate resource loading. The general approach is to track how many things we are waiting on and when we hit zero proceed with the commit. For Float CSS in particular we wait for all stylesheet preloads before inserting any uninserted stylesheets. When all the stylesheets have loaded we continue the commit as usual.
facebook · Mar 21, 2023 · e85161a · e85161a
1 parent 7ffcac8
commit e85161a
Show file tree

Hide file tree

Showing 13 changed files with 486 additions and 81 deletions.
diff --git a/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js b/packages/react-dom-bindings/src/client/ReactDOMHostConfig.js
diff --git a/...dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetExternalRuntime.js b/...dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetExternalRuntime.js
@@ -6,8 +6,6 @@ import {
   clientRenderBoundary,
   completeBoundary,
   completeSegment,
-  LOADED,
-  ERRORED,
 } from './ReactDOMFizzInstructionSetShared';
 
 export {clientRenderBoundary, completeBoundary, completeSegment};
@@ -46,10 +44,6 @@ export function completeBoundaryWithStyles(
   const dependencies = [];
   let href, precedence, attr, loadingState, resourceEl, media;
 
-  function setStatus(s) {
-    this['s'] = s;
-  }
-
   // Sheets Mode
   let sheetMode = true;
   while (true) {
@@ -84,14 +78,10 @@ export function completeBoundaryWithStyles(
         while ((attr = stylesheetDescriptor[j++])) {
           resourceEl.setAttribute(attr, stylesheetDescriptor[j++]);
         }
-        loadingState = resourceEl['_p'] = new Promise((re, rj) => {
-          resourceEl.onload = re;
-          resourceEl.onerror = rj;
+        loadingState = resourceEl['_p'] = new Promise((resolve, reject) => {
+          resourceEl.onload = resolve;
+          resourceEl.onerror = reject;
         });
-        loadingState.then(
-          setStatus.bind(loadingState, LOADED),
-          setStatus.bind(loadingState, ERRORED),
-        );
         // Save this resource element so we can bailout if it is used again
         resourceMap.set(href, resourceEl);
       }

diff --git a/...m-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings.js b/...m-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineCodeStrings.js
diff --git a/...ct-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineSource.js b/...ct-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetInlineSource.js
@@ -8,8 +8,6 @@ import {
   clientRenderBoundary,
   completeBoundary,
   completeSegment,
-  LOADED,
-  ERRORED,
 } from './ReactDOMFizzInstructionSetShared';
 
 export {clientRenderBoundary, completeBoundary, completeSegment};
@@ -49,10 +47,6 @@ export function completeBoundaryWithStyles(
   const dependencies = [];
   let href, precedence, attr, loadingState, resourceEl, media;
 
-  function setStatus(s) {
-    this['s'] = s;
-  }
-
   // Sheets Mode
   let sheetMode = true;
   while (true) {
@@ -87,14 +81,10 @@ export function completeBoundaryWithStyles(
         while ((attr = stylesheetDescriptor[j++])) {
           resourceEl.setAttribute(attr, stylesheetDescriptor[j++]);
         }
-        loadingState = resourceEl['_p'] = new Promise((re, rj) => {
-          resourceEl.onload = re;
-          resourceEl.onerror = rj;
+        loadingState = resourceEl['_p'] = new Promise((resolve, reject) => {
+          resourceEl.onload = resolve;
+          resourceEl.onerror = reject;
         });
-        loadingState.then(
-          setStatus.bind(loadingState, LOADED),
-          setStatus.bind(loadingState, ERRORED),
-        );
         // Save this resource element so we can bailout if it is used again
         resourceMap.set(href, resourceEl);
       }

diff --git a/...es/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetShared.js b/...es/react-dom-bindings/src/server/fizz-instruction-set/ReactDOMFizzInstructionSetShared.js
@@ -8,8 +8,6 @@ export const SUSPENSE_START_DATA = '$';
 export const SUSPENSE_END_DATA = '/$';
 export const SUSPENSE_PENDING_START_DATA = '$?';
 export const SUSPENSE_FALLBACK_START_DATA = '$!';
-export const LOADED = 'l';
-export const ERRORED = 'e';
 
 // TODO: Symbols that are referenced outside this module use dynamic accessor
 // notation instead of dot notation to prevent Closure's advanced compilation

diff --git a/packages/react-dom/src/__tests__/ReactDOMFloat-test.js b/packages/react-dom/src/__tests__/ReactDOMFloat-test.js
@@ -2676,6 +2676,65 @@ body {
     );
   });
 
+  it('can delay commit until css resources load', async () => {
+    const root = ReactDOMClient.createRoot(container);
+    expect(getMeaningfulChildren(container)).toBe(undefined);
+    React.startTransition(() => {
+      root.render(
+        <>
+          <link
+            rel="stylesheet"
+            href="foo"
+            precedence="default"
+            data-suspensey={true}
+          />
+          <div>hello</div>
+        </>,
+      );
+    });
+    await waitForAll([]);
+    expect(getMeaningfulChildren(container)).toBe(undefined);
+    expect(getMeaningfulChildren(document.head)).toEqual(
+      <link rel="preload" as="style" href="foo" />,
+    );
+
+    const preload = document.querySelector('link[rel="preload"][as="style"]');
+    const loadEvent = document.createEvent('Events');
+    loadEvent.initEvent('load', true, true);
+    preload.dispatchEvent(loadEvent);
+
+    // We expect that the stylesheet is inserted now but the commit has not happened yet.
+    expect(getMeaningfulChildren(container)).toBe(undefined);
+    expect(getMeaningfulChildren(document.head)).toEqual([
+      <link
+        rel="stylesheet"
+        href="foo"
+        data-precedence="default"
+        data-suspensey="true"
+      />,
+      <link rel="preload" as="style" href="foo" />,
+    ]);
+
+    const stylesheet = document.querySelector(
+      'link[rel="stylesheet"][data-precedence]',
+    );
+    const loadEvent2 = document.createEvent('Events');
+    loadEvent2.initEvent('load', true, true);
+    stylesheet.dispatchEvent(loadEvent2);
+
+    // We expect that the commit finishes synchronously after the stylesheet loads.
+    expect(getMeaningfulChildren(container)).toEqual(<div>hello</div>);
+    expect(getMeaningfulChildren(document.head)).toEqual([
+      <link
+        rel="stylesheet"
+        href="foo"
+        data-precedence="default"
+        data-suspensey="true"
+      />,
+      <link rel="preload" as="style" href="foo" />,
+    ]);
+  });
+
   describe('ReactDOM.prefetchDNS(href)', () => {
     it('creates a dns-prefetch resource when called', async () => {
       function App({url}) {

diff --git a/packages/react-noop-renderer/src/createReactNoop.js b/packages/react-noop-renderer/src/createReactNoop.js
@@ -578,6 +578,12 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
       return type === 'suspensey-thing' && typeof props.src === 'string';
     },
 
+    mayResourceSuspendCommit(resource: mixed): boolean {
+      throw new Error(
+        'Resources are not implemented for React Noop yet. This method should not be called',
+      );
+    },
+
     preloadInstance(type: string, props: Props): boolean {
       if (type !== 'suspensey-thing' || typeof props.src !== 'string') {
         throw new Error('Attempted to preload unexpected instance: ' + type);
@@ -608,8 +614,21 @@ function createReactNoop(reconciler: Function, useMutation: boolean) {
       }
     },
 
+    preloadResource(resource: mixed): boolean {
+      throw new Error(
+        'Resources are not implemented for React Noop yet. This method should not be called',
+      );
+    },
+
     startSuspendingCommit,
     suspendInstance,
+
+    suspendResource(resource: mixed): void {
+      throw new Error(
+        'Resources are not implemented for React Noop yet. This method should not be called',
+      );
+    },
+
     waitForCommitToBeReady,
 
     prepareRendererToRender() {},

diff --git a/packages/react-reconciler/src/ReactFiberCommitWork.js b/packages/react-reconciler/src/ReactFiberCommitWork.js
@@ -160,6 +160,7 @@ import {
   unmountHoistable,
   prepareToCommitHoistables,
   suspendInstance,
+  suspendResource,
 } from './ReactFiberHostConfig';
 import {
   captureCommitPhaseError,
@@ -4064,23 +4065,72 @@ export function commitPassiveUnmountEffects(finishedWork: Fiber): void {
   resetCurrentDebugFiberInDEV();
 }
 
-export function recursivelyAccumulateSuspenseyCommit(parentFiber: Fiber): void {
+export function accumulateSuspenseyCommit(finishedWork: Fiber): void {
+  accumulateSuspenseyCommitOnFiber(finishedWork);
+}
+
+function recursivelyAccumulateSuspenseyCommit(parentFiber: Fiber): void {
   if (parentFiber.subtreeFlags & SuspenseyCommit) {
     let child = parentFiber.child;
     while (child !== null) {
-      recursivelyAccumulateSuspenseyCommit(child);
-      switch (child.tag) {
-        case HostComponent:
-        case HostHoistable: {
-          if (child.flags & SuspenseyCommit) {
-            const type = child.type;
-            const props = child.memoizedProps;
-            suspendInstance(type, props);
-          }
-          break;
+      accumulateSuspenseyCommitOnFiber(child);
+      child = child.sibling;
+    }
+  }
+}
+
+function accumulateSuspenseyCommitOnFiber(fiber: Fiber) {
+  switch (fiber.tag) {
+    case HostHoistable: {
+      recursivelyAccumulateSuspenseyCommit(fiber);
+      if (fiber.flags & SuspenseyCommit) {
+        if (fiber.memoizedState !== null) {
+          suspendResource(
+            // This should always be set by visiting HostRoot first
+            (currentHoistableRoot: any),
+            fiber.memoizedState,
+            fiber.memoizedProps,
+          );
+        } else {
+          const type = fiber.type;
+          const props = fiber.memoizedProps;
+          suspendInstance(type, props);
         }
       }
-      child = child.sibling;
+      break;
+    }
+    case HostComponent: {
+      recursivelyAccumulateSuspenseyCommit(fiber);
+      if (fiber.flags & SuspenseyCommit) {
+        const type = fiber.type;
+        const props = fiber.memoizedProps;
+        suspendInstance(type, props);
+      }
+      break;
+    }
+    case HostRoot: {
+      if (enableFloat && supportsResources) {
+        const previousHoistableRoot = currentHoistableRoot;
+        currentHoistableRoot = getHoistableRoot(fiber.stateNode.containerInfo);
+
+        recursivelyAccumulateSuspenseyCommit(fiber);
+        currentHoistableRoot = previousHoistableRoot;
+        break;
+      }
+    }
+    // eslint-disable-next-line-no-fallthrough
+    case HostPortal: {
+      if (enableFloat && supportsResources) {
+        const previousHoistableRoot = currentHoistableRoot;
+        currentHoistableRoot = getHoistableRoot(fiber.stateNode.containerInfo);
+        recursivelyAccumulateSuspenseyCommit(fiber);
+        currentHoistableRoot = previousHoistableRoot;
+        break;
+      }
+    }
+    // eslint-disable-next-line-no-fallthrough
+    default: {
+      recursivelyAccumulateSuspenseyCommit(fiber);
     }
   }
 }

diff --git a/packages/react-reconciler/src/ReactFiberCompleteWork.js b/packages/react-reconciler/src/ReactFiberCompleteWork.js
@@ -110,6 +110,7 @@ import {
   preparePortalMount,
   prepareScopeUpdate,
   maySuspendCommit,
+  mayResourceSuspendCommit,
   preloadInstance,
 } from './ReactFiberHostConfig';
 import {
@@ -521,7 +522,17 @@ function preloadInstanceAndSuspendIfNeeded(
   renderLanes: Lanes,
 ) {
   // Ask the renderer if this instance should suspend the commit.
-  if (!maySuspendCommit(type, props)) {
+  if (workInProgress.memoizedState !== null) {
+    if (!mayResourceSuspendCommit(workInProgress.memoizedState)) {
+      // If this flag was set previously, we can remove it. The flag represents
+      // whether this particular set of props might ever need to suspend. The
+      // safest thing to do is for shouldSuspendCommit to always return true, but
+      // if the renderer is reasonably confident that the underlying resource
+      // won't be evicted, it can return false as a performance optimization.
+      workInProgress.flags &= ~SuspenseyCommit;
+      return;
+    }
+  } else if (!maySuspendCommit(type, props)) {
     // If this flag was set previously, we can remove it. The flag represents
     // whether this particular set of props might ever need to suspend. The
     // safest thing to do is for maySuspendCommit to always return true, but

diff --git a/packages/react-reconciler/src/ReactFiberHostConfigWithNoResources.js b/packages/react-reconciler/src/ReactFiberHostConfigWithNoResources.js
@@ -32,3 +32,6 @@ export const mountHoistable = shim;
 export const unmountHoistable = shim;
 export const createHoistableInstance = shim;
 export const prepareToCommitHoistables = shim;
+export const mayResourceSuspendCommit = shim;
+export const preloadResource = shim;
+export const suspendResource = shim;
diff --git a/packages/react-reconciler/src/ReactFiberWorkLoop.js b/packages/react-reconciler/src/ReactFiberWorkLoop.js
@@ -209,7 +209,7 @@ import {
   invokePassiveEffectMountInDEV,
   invokeLayoutEffectUnmountInDEV,
   invokePassiveEffectUnmountInDEV,
-  recursivelyAccumulateSuspenseyCommit,
+  accumulateSuspenseyCommit,
 } from './ReactFiberCommitWork';
 import {enqueueUpdate} from './ReactFiberClassUpdateQueue';
 import {resetContextDependencies} from './ReactFiberNewContext';
@@ -1444,7 +1444,7 @@ function commitRootWhenReady(
     // the suspensey resources. The renderer is responsible for accumulating
     // all the load events. This all happens in a single synchronous
     // transaction, so it track state in its own module scope.
-    recursivelyAccumulateSuspenseyCommit(finishedWork);
+    accumulateSuspenseyCommit(finishedWork);
     // At the end, ask the renderer if it's ready to commit, or if we should
     // suspend. If it's not ready, it will return a callback to subscribe to
     // a ready event.

diff --git a/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js b/packages/react-reconciler/src/forks/ReactFiberHostConfig.custom.js
@@ -220,6 +220,9 @@ export const unmountHoistable = $$$hostConfig.unmountHoistable;
 export const createHoistableInstance = $$$hostConfig.createHoistableInstance;
 export const prepareToCommitHoistables =
   $$$hostConfig.prepareToCommitHoistables;
+export const mayResourceSuspendCommit = $$$hostConfig.mayResourceSuspendCommit;
+export const preloadResource = $$$hostConfig.preloadResource;
+export const suspendResource = $$$hostConfig.suspendResource;
 
 // -------------------
 //     Singletons

diff --git a/scripts/error-codes/codes.json b/scripts/error-codes/codes.json
@@ -459,5 +459,6 @@
   "471": "BigInt (%s) is not yet supported as an argument to a Server Function.",
   "472": "Type %s is not supported as an argument to a Server Function.",
   "473": "React doesn't accept base64 encoded file uploads because we don't except form data passed from a browser to ever encode data that way. If that's the wrong assumption, we can easily fix it.",
-  "474": "Suspense Exception: This is not a real error, and should not leak into userspace. If you're seeing this, it's likely a bug in React."
+  "474": "Suspense Exception: This is not a real error, and should not leak into userspace. If you're seeing this, it's likely a bug in React.",
+  "475": "Internal React Error: suspendedState null when it was expected to exists. Please report this as a React bug."
 }