Skip to content

Commit

Permalink
Check if suspensey instance resolves in immediate task (#26427)
Browse files Browse the repository at this point in the history
When rendering a suspensey resource that we haven't seen before, it may
have loaded in the background while we were rendering. We should yield
to the main thread to see if the load event fires in an immediate task.

For example, if the resource for a link element has already loaded, its
load event will fire in a task right after React yields to the main
thread. Because the continuation task is not scheduled until right
before React yields, the load event will ping React before it resumes.

If this happens, we can resume rendering without showing a fallback.

I don't think this matters much for images, because the `completed`
property tells us whether the image has loaded, and during a non-urgent
render, we never block the main thread for more than 5ms at a time (for
now — we might increase this in the future). It matters more for
stylesheets because the only way to check if it has loaded is by
listening for the load event.

This is essentially the same trick that `use` does for userspace
promises, but a bit simpler because we don't need to replay the host
component's begin phase; the work-in-progress fiber already completed,
so we can just continue onto the next sibling without any additional
work.

As part of this change, I split the `shouldSuspendCommit` host config
method into separate `maySuspendCommit` and `preloadInstance` methods.
Previously `shouldSuspendCommit` was used for both.

This raised a question of whether we should preload resources during a
synchronous render. My initial instinct was that we shouldn't, because
we're going to synchronously block the main thread until the resource is
inserted into the DOM, anyway. But I wonder if the browser is able to
initiate the preload even while the main thread is blocked. It's
probably a micro-optimization either way because most resources will be
loaded during transitions, not urgent renders.

DiffTrain build for [0131d0c](0131d0c)
  • Loading branch information
acdlite committed Mar 20, 2023
1 parent e49c66b commit c596d33
Show file tree
Hide file tree
Showing 19 changed files with 2,147 additions and 1,144 deletions.
2 changes: 1 addition & 1 deletion compiled/facebook-www/REVISION
Original file line number Diff line number Diff line change
@@ -1 +1 @@
db281b3d9cd033cdc3d63e00fc9f3153c03aa70c
0131d0cff40d4054ac72c857d3a13c5173c46e0a
2 changes: 1 addition & 1 deletion compiled/facebook-www/React-dev.modern.js
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ if (
}
"use strict";

var ReactVersion = "18.3.0-www-modern-eb67b0a3";
var ReactVersion = "18.3.0-www-modern-1ad200fa";

// ATTENTION
// When adding new symbols to this file,
Expand Down
111 changes: 100 additions & 11 deletions compiled/facebook-www/ReactART-dev.classic.js
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ function _assertThisInitialized(self) {
return self;
}

var ReactVersion = "18.3.0-www-classic-5da08f8d";
var ReactVersion = "18.3.0-www-classic-ca4ffb9c";

var LegacyRoot = 0;
var ConcurrentRoot = 1;
Expand Down Expand Up @@ -2921,6 +2921,10 @@ function unhideTextInstance(textInstance, text) {
function getInstanceFromNode(node) {
throw new Error("Not implemented.");
}
function preloadInstance(type, props) {
// Return true to indicate it's already loaded
return true;
}
function waitForCommitToBeReady() {
return null;
} // eslint-disable-next-line no-undef
Expand Down Expand Up @@ -5453,6 +5457,10 @@ var SuspenseException = new Error(
"unexpected behavior.\n\n" +
"To handle async errors, wrap your component in an error boundary, or " +
"call the promise's `.catch` method and pass the result to `use`"
);
var SuspenseyCommitException = new Error(
"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."
); // This is a noop thenable that we use to trigger a fallback in throwException.
// TODO: It would be better to refactor throwException into multiple functions
// so we can trigger a fallback directly without having to check the type. But
Expand Down Expand Up @@ -17636,7 +17644,6 @@ function updateHostComponent(
// we won't touch this node even if children changed.
return;
} // If we get updated because one of our children updated, we don't
suspendHostCommitIfNeeded(workInProgress);
getHostContext(); // TODO: Experiencing an error where oldProps is null. Suggests a host
// component is hitting the resume path. Figure out why. Possibly
// related to `hidden`.
Expand All @@ -17655,12 +17662,17 @@ function updateHostComponent(
// that suspend don't have children, so it doesn't matter. But that might not
// always be true in the future.

function suspendHostCommitIfNeeded(workInProgress, type, props, renderLanes) {
function preloadInstanceAndSuspendIfNeeded(
workInProgress,
type,
props,
renderLanes
) {
// Ask the renderer if this instance should suspend the commit.
{
// 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
// safest thing to do is for maySuspendCommit 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;
Expand Down Expand Up @@ -18141,15 +18153,18 @@ function completeWork(current, workInProgress, renderLanes) {
workInProgress.stateNode = _instance3; // Certain renderers require commit-time effects for initial mount.
}

suspendHostCommitIfNeeded(workInProgress);

if (workInProgress.ref !== null) {
// If there is a ref on a host node we need to schedule a callback
markRef(workInProgress);
}
}

bubbleProperties(workInProgress);
bubbleProperties(workInProgress); // This must come at the very end of the complete phase, because it might
// throw to suspend, and if the resource immediately loads, the work loop
// will resume rendering as if the work-in-progress completed. So it must
// fully complete.

preloadInstanceAndSuspendIfNeeded(workInProgress);
return null;
}

Expand Down Expand Up @@ -23119,9 +23134,11 @@ var NotSuspended = 0;
var SuspendedOnError = 1;
var SuspendedOnData = 2;
var SuspendedOnImmediate = 3;
var SuspendedOnDeprecatedThrowPromise = 4;
var SuspendedAndReadyToContinue = 5;
var SuspendedOnHydration = 6; // When this is true, the work-in-progress fiber just suspended (or errored) and
var SuspendedOnInstance = 4;
var SuspendedOnInstanceAndReadyToContinue = 5;
var SuspendedOnDeprecatedThrowPromise = 6;
var SuspendedAndReadyToContinue = 7;
var SuspendedOnHydration = 8; // When this is true, the work-in-progress fiber just suspended (or errored) and
// we've yet to unwind the stack. In some cases, we may yield to the main thread
// after this happens. If the fiber is pinged before we resume, we can retry
// immediately instead of unwinding the stack.
Expand Down Expand Up @@ -24477,6 +24494,9 @@ function handleThrow(root, thrownValue) {
: // immediately resolved (i.e. in a microtask). Otherwise, trigger the
// nearest Suspense fallback.
SuspendedOnImmediate;
} else if (thrownValue === SuspenseyCommitException) {
thrownValue = getSuspendedThenable();
workInProgressSuspendedReason = SuspendedOnInstance;
} else if (thrownValue === SelectiveHydrationException) {
// An update flowed into a dehydrated boundary. Before we can apply the
// update, we need to finish hydrating. Interrupt the work-in-progress
Expand Down Expand Up @@ -24847,7 +24867,7 @@ function renderRootConcurrent(root, lanes) {
var unitOfWork = workInProgress;
var thrownValue = workInProgressThrownValue;

switch (workInProgressSuspendedReason) {
resumeOrUnwind: switch (workInProgressSuspendedReason) {
case SuspendedOnError: {
// Unwind then continue with the normal work loop.
workInProgressSuspendedReason = NotSuspended;
Expand Down Expand Up @@ -24899,6 +24919,12 @@ function renderRootConcurrent(root, lanes) {
break outer;
}

case SuspendedOnInstance: {
workInProgressSuspendedReason =
SuspendedOnInstanceAndReadyToContinue;
break outer;
}

case SuspendedAndReadyToContinue: {
var _thenable = thrownValue;

Expand All @@ -24917,6 +24943,69 @@ function renderRootConcurrent(root, lanes) {
break;
}

case SuspendedOnInstanceAndReadyToContinue: {
switch (workInProgress.tag) {
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
// the main thread.
// Assigning this to a constant so Flow knows the binding won't
// be mutated by `preloadInstance`.
var hostFiber = workInProgress;
var type = hostFiber.type;
var props = hostFiber.pendingProps;
var isReady = 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
// have to replay anything because the host fiber
// already completed.
workInProgressSuspendedReason = NotSuspended;
workInProgressThrownValue = null;
var sibling = hostFiber.sibling;

if (sibling !== null) {
workInProgress = sibling;
} else {
var returnFiber = hostFiber.return;

if (returnFiber !== null) {
workInProgress = returnFiber;
completeUnitOfWork(returnFiber);
} else {
workInProgress = null;
}
}

break resumeOrUnwind;
}

break;
}

default: {
// This will fail gracefully but it's not correct, so log a
// warning in dev.
if (true) {
error(
"Unexpected type of fiber triggered a suspensey commit. " +
"This is a bug in React."
);
}

break;
}
} // Otherwise, unwind then continue with the normal work loop.

workInProgressSuspendedReason = NotSuspended;
workInProgressThrownValue = null;
unwindSuspendedUnitOfWork(unitOfWork, thrownValue);
break;
}

case SuspendedOnDeprecatedThrowPromise: {
// Suspended by an old implementation that uses the `throw promise`
// pattern. The newer replaying behavior can cause subtle issues
Expand Down
Loading

0 comments on commit c596d33

Please sign in to comment.