Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Suspense] Change Suspending and Restarting Heuristics #15769

Merged
merged 13 commits into from
May 30, 2019
Merged
Show file tree
Hide file tree
Changes from 12 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
13 changes: 4 additions & 9 deletions packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -1413,6 +1413,9 @@ function validateFunctionComponentInDev(workInProgress: Fiber, Component: any) {
}
}

// TODO: This is now an empty object. Should we just make it a boolean?
const SUSPENDED_MARKER: SuspenseState = ({}: any);

function updateSuspenseComponent(
current,
workInProgress,
Expand Down Expand Up @@ -1440,17 +1443,9 @@ function updateSuspenseComponent(
(ForceSuspenseFallback: SuspenseContext),
)
) {
// This either already captured or is a new mount that was forced into its fallback
// state by a parent.
const attemptedState: SuspenseState | null = workInProgress.memoizedState;
// Something in this boundary's subtree already suspended. Switch to
// rendering the fallback children.
nextState = {
fallbackExpirationTime:
attemptedState !== null
? attemptedState.fallbackExpirationTime
: NoWork,
};
nextState = SUSPENDED_MARKER;
nextDidTimeout = true;
workInProgress.effectTag &= ~DidCapture;
} else {
Expand Down
17 changes: 2 additions & 15 deletions packages/react-reconciler/src/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,6 @@ import invariant from 'shared/invariant';
import warningWithoutStack from 'shared/warningWithoutStack';
import warning from 'shared/warning';

import {
NoWork,
computeAsyncExpirationNoBucket,
} from './ReactFiberExpirationTime';
import {onCommitUnmount} from './ReactFiberDevToolsHook';
import {startPhaseTimer, stopPhaseTimer} from './ReactDebugFiberPerf';
import {getStackByFiberInDevAndProd} from './ReactCurrentFiber';
Expand Down Expand Up @@ -102,8 +98,8 @@ import {
} from './ReactFiberHostConfig';
import {
captureCommitPhaseError,
requestCurrentTime,
resolveRetryThenable,
markCommitTimeOfFallback,
} from './ReactFiberWorkLoop';
import {
NoEffect as NoHookEffect,
Expand Down Expand Up @@ -1288,16 +1284,7 @@ function commitSuspenseComponent(finishedWork: Fiber) {
} else {
newDidTimeout = true;
primaryChildParent = finishedWork.child;
if (newState.fallbackExpirationTime === NoWork) {
// If the children had not already timed out, record the time.
// This is used to compute the elapsed time during subsequent
// attempts to render the children.
// We model this as a normal pri expiration time since that's
// how we infer start time for updates.
newState.fallbackExpirationTime = computeAsyncExpirationNoBucket(
requestCurrentTime(),
);
}
markCommitTimeOfFallback();
}

if (supportsMutation && primaryChildParent !== null) {
Expand Down
16 changes: 7 additions & 9 deletions packages/react-reconciler/src/ReactFiberCompleteWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,7 +101,6 @@ import {
enableEventAPI,
} from 'shared/ReactFeatureFlags';
import {
markRenderEventTimeAndConfig,
renderDidSuspend,
renderDidSuspendDelayIfPossible,
} from './ReactFiberWorkLoop';
Expand Down Expand Up @@ -702,14 +701,6 @@ function completeWork(
prevDidTimeout = prevState !== null;
if (!nextDidTimeout && prevState !== null) {
// We just switched from the fallback to the normal children.

// Mark the event time of the switching from fallback to normal children,
// based on the start of when we first showed the fallback. This time
// was given a normal pri expiration time at the time it was shown.
const fallbackExpirationTime: ExpirationTime =
prevState.fallbackExpirationTime;
markRenderEventTimeAndConfig(fallbackExpirationTime, null);

// Delete the fallback.
// TODO: Would it be better to store the fallback fragment on
// the stateNode during the begin phase?
Expand Down Expand Up @@ -737,6 +728,13 @@ function completeWork(
// in the concurrent tree already suspended during this render.
// This is a known bug.
if ((workInProgress.mode & BatchedMode) !== NoMode) {
// TODO: Move this back to throwException because this is too late
// if this is a large tree which is common for initial loads. We
// don't know if we should restart a render or not until we get
// this marker, and this is too late.
// If this render already had a ping or lower pri updates,
// and this is the first time we know we're going to suspend we
// should be able to immediately restart from within throwException.
const hasInvisibleChildContext =
current === null &&
workInProgress.memoizedProps.unstable_avoidThisFallback !== true;
Expand Down
8 changes: 0 additions & 8 deletions packages/react-reconciler/src/ReactFiberExpirationTime.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,14 +83,6 @@ export function computeSuspenseExpiration(
);
}

// Same as computeAsyncExpiration but without the bucketing logic. This is
// used to compute timestamps instead of actual expiration times.
export function computeAsyncExpirationNoBucket(
currentTime: ExpirationTime,
): ExpirationTime {
return currentTime - LOW_PRIORITY_EXPIRATION / UNIT_SIZE;
}

// We intentionally set a higher expiration time for interactive updates in
// dev than in production.
//
Expand Down
6 changes: 2 additions & 4 deletions packages/react-reconciler/src/ReactFiberSuspenseComponent.js
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,9 @@
*/

import type {Fiber} from './ReactFiber';
import type {ExpirationTime} from './ReactFiberExpirationTime';

export type SuspenseState = {|
fallbackExpirationTime: ExpirationTime,
|};
// TODO: This is now an empty object. Should we switch this to a boolean?
export type SuspenseState = {||};

export function shouldCaptureSuspense(
workInProgress: Fiber,
Expand Down
Loading