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

Track Event Time as the Start Time for Suspense #15358

Merged
merged 6 commits into from
Apr 10, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,10 @@ describe('React hooks DevTools integration', () => {
</div>,
{unstable_isConcurrent: true},
);

expect(Scheduler).toFlushAndYield([]);
// Ensure we timeout any suspense time.
jest.advanceTimersByTime(1000);
const fiber = renderer.root._currentFiber().child;
if (__DEV__) {
// First render was locked
Expand Down
3 changes: 2 additions & 1 deletion packages/react-reconciler/src/ReactFiberBeginWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -1413,7 +1413,8 @@ function updateSuspenseComponent(
// Something in this boundary's subtree already suspended. Switch to
// rendering the fallback children.
nextState = {
timedOutAt: nextState !== null ? nextState.timedOutAt : NoWork,
fallbackExpirationTime:
nextState !== null ? nextState.fallbackExpirationTime : NoWork,
};
nextDidTimeout = true;
workInProgress.effectTag &= ~DidCapture;
Expand Down
13 changes: 10 additions & 3 deletions packages/react-reconciler/src/ReactFiberCommitWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,10 @@ import invariant from 'shared/invariant';
import warningWithoutStack from 'shared/warningWithoutStack';
import warning from 'shared/warning';

import {NoWork} from './ReactFiberExpirationTime';
import {
NoWork,
computeAsyncExpirationNoBucket,
} from './ReactFiberExpirationTime';
import {onCommitUnmount} from './ReactFiberDevToolsHook';
import {startPhaseTimer, stopPhaseTimer} from './ReactDebugFiberPerf';
import {getStackByFiberInDevAndProd} from './ReactCurrentFiber';
Expand Down Expand Up @@ -1272,11 +1275,15 @@ function commitSuspenseComponent(finishedWork: Fiber) {
} else {
newDidTimeout = true;
primaryChildParent = finishedWork.child;
if (newState.timedOutAt === NoWork) {
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.
newState.timedOutAt = requestCurrentTime();
// We model this as a normal pri expiration time since that's
// how we infer start time for updates.
newState.fallbackExpirationTime = computeAsyncExpirationNoBucket(
requestCurrentTime(),
);
}
}

Expand Down
66 changes: 47 additions & 19 deletions packages/react-reconciler/src/ReactFiberCompleteWork.js
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import type {
ChildSet,
} from './ReactFiberHostConfig';
import type {ReactEventComponentInstance} from 'shared/ReactTypes';
import type {SuspenseState} from './ReactFiberSuspenseComponent';

import {
IndeterminateComponent,
Expand All @@ -42,6 +43,7 @@ import {
EventComponent,
EventTarget,
} from 'shared/ReactWorkTags';
import {ConcurrentMode, NoContext} from './ReactTypeOfMode';
import {
Placement,
Ref,
Expand Down Expand Up @@ -92,6 +94,7 @@ import {
enableSuspenseServerRenderer,
enableEventAPI,
} from 'shared/ReactFeatureFlags';
import {markRenderEventTime, renderDidSuspend} from './ReactFiberScheduler';

function markUpdate(workInProgress: Fiber) {
// Tag the fiber with an update effect. This turns a Placement into
Expand Down Expand Up @@ -665,7 +668,7 @@ function completeWork(
case ForwardRef:
break;
case SuspenseComponent: {
const nextState = workInProgress.memoizedState;
const nextState: null | SuspenseState = workInProgress.memoizedState;
if ((workInProgress.effectTag & DidCapture) !== NoEffect) {
// Something suspended. Re-render with the fallback children.
workInProgress.expirationTime = renderExpirationTime;
Expand All @@ -674,34 +677,58 @@ function completeWork(
}

const nextDidTimeout = nextState !== null;
const prevDidTimeout = current !== null && current.memoizedState !== null;

let prevDidTimeout = false;
if (current === null) {
// In cases where we didn't find a suitable hydration boundary we never
// downgraded this to a DehydratedSuspenseComponent, but we still need to
// pop the hydration state since we might be inside the insertion tree.
popHydrationState(workInProgress);
} else if (!nextDidTimeout && prevDidTimeout) {
// We just switched from the fallback to the normal children. Delete
// the fallback.
// TODO: Would it be better to store the fallback fragment on
// the stateNode during the begin phase?
const currentFallbackChild: Fiber | null = (current.child: any).sibling;
if (currentFallbackChild !== null) {
// Deletions go at the beginning of the return fiber's effect list
const first = workInProgress.firstEffect;
if (first !== null) {
workInProgress.firstEffect = currentFallbackChild;
currentFallbackChild.nextEffect = first;
} else {
workInProgress.firstEffect = workInProgress.lastEffect = currentFallbackChild;
currentFallbackChild.nextEffect = null;
} else {
const prevState: null | SuspenseState = current.memoizedState;
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 fallbackExpirationTimeExpTime: ExpirationTime =
prevState.fallbackExpirationTime;
markRenderEventTime(fallbackExpirationTimeExpTime);

// Delete the fallback.
// TODO: Would it be better to store the fallback fragment on
// the stateNode during the begin phase?
const currentFallbackChild: Fiber | null = (current.child: any)
.sibling;
if (currentFallbackChild !== null) {
// Deletions go at the beginning of the return fiber's effect list
const first = workInProgress.firstEffect;
if (first !== null) {
workInProgress.firstEffect = currentFallbackChild;
currentFallbackChild.nextEffect = first;
} else {
workInProgress.firstEffect = workInProgress.lastEffect = currentFallbackChild;
currentFallbackChild.nextEffect = null;
}
currentFallbackChild.effectTag = Deletion;
}
currentFallbackChild.effectTag = Deletion;
}
}

if (nextDidTimeout && !prevDidTimeout) {
// If this subtreee is running in concurrent mode we can suspend,
// otherwise we won't suspend.
// TODO: This will still suspend a synchronous tree if anything
// in the concurrent tree already suspended during this render.
// This is a known bug.
if ((workInProgress.mode & ConcurrentMode) !== NoContext) {
renderDidSuspend();
}
}

if (supportsPersistence) {
// TODO: Only schedule updates if not prevDidTimeout.
if (nextDidTimeout) {
// If this boundary just timed out, schedule an effect to attach a
// retry listener to the proimse. This flag is also used to hide the
Expand All @@ -710,6 +737,7 @@ function completeWork(
}
}
if (supportsMutation) {
// TODO: Only schedule updates if these values are non equal, i.e. it changed.
if (nextDidTimeout || prevDidTimeout) {
// If this boundary just timed out, schedule an effect to attach a
// retry listener to the proimse. This flag is also used to hide the
Expand Down
8 changes: 8 additions & 0 deletions packages/react-reconciler/src/ReactFiberExpirationTime.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,14 @@ export function computeAsyncExpiration(
);
}

// 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
11 changes: 11 additions & 0 deletions packages/react-reconciler/src/ReactFiberHooks.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import {
flushPassiveEffects,
requestCurrentTime,
warnIfNotCurrentlyActingUpdatesInDev,
markRenderEventTime,
} from './ReactFiberScheduler';

import invariant from 'shared/invariant';
Expand Down Expand Up @@ -718,6 +719,16 @@ function updateReducer<S, I, A>(
remainingExpirationTime = updateExpirationTime;
}
} else {
// This update does have sufficient priority.

// Mark the event time of this update as relevant to this render pass.
// TODO: This should ideally use the true event time of this update rather than
// its priority which is a derived and not reverseable value.
// TODO: We should skip this update if it was already committed but currently
// we have no way of detecting the difference between a committed and suspended
// update here.
markRenderEventTime(updateExpirationTime);

// Process this update.
if (update.eagerReducer === reducer) {
// If this update was processed eagerly, and its reducer matches the
Expand Down
17 changes: 0 additions & 17 deletions packages/react-reconciler/src/ReactFiberPendingPriority.js
Original file line number Diff line number Diff line change
Expand Up @@ -219,23 +219,6 @@ function clearPing(root, completedTime) {
}
}

export function findEarliestOutstandingPriorityLevel(
root: FiberRoot,
renderExpirationTime: ExpirationTime,
): ExpirationTime {
let earliestExpirationTime = renderExpirationTime;

const earliestPendingTime = root.earliestPendingTime;
const earliestSuspendedTime = root.earliestSuspendedTime;
if (earliestPendingTime > earliestExpirationTime) {
earliestExpirationTime = earliestPendingTime;
}
if (earliestSuspendedTime > earliestExpirationTime) {
earliestExpirationTime = earliestSuspendedTime;
}
return earliestExpirationTime;
}

export function didExpireAtExpirationTime(
root: FiberRoot,
currentTime: ExpirationTime,
Expand Down
10 changes: 5 additions & 5 deletions packages/react-reconciler/src/ReactFiberScheduler.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import {
computeExpirationForFiber as computeExpirationForFiber_old,
captureCommitPhaseError as captureCommitPhaseError_old,
onUncaughtError as onUncaughtError_old,
markRenderEventTime as markRenderEventTime_old,
renderDidSuspend as renderDidSuspend_old,
renderDidError as renderDidError_old,
pingSuspendedRoot as pingSuspendedRoot_old,
Expand All @@ -34,14 +35,14 @@ import {
computeUniqueAsyncExpiration as computeUniqueAsyncExpiration_old,
flushPassiveEffects as flushPassiveEffects_old,
warnIfNotCurrentlyActingUpdatesInDev as warnIfNotCurrentlyActingUpdatesInDev_old,
inferStartTimeFromExpirationTime as inferStartTimeFromExpirationTime_old,
} from './ReactFiberScheduler.old';

import {
requestCurrentTime as requestCurrentTime_new,
computeExpirationForFiber as computeExpirationForFiber_new,
captureCommitPhaseError as captureCommitPhaseError_new,
onUncaughtError as onUncaughtError_new,
markRenderEventTime as markRenderEventTime_new,
renderDidSuspend as renderDidSuspend_new,
renderDidError as renderDidError_new,
pingSuspendedRoot as pingSuspendedRoot_new,
Expand All @@ -62,7 +63,6 @@ import {
computeUniqueAsyncExpiration as computeUniqueAsyncExpiration_new,
flushPassiveEffects as flushPassiveEffects_new,
warnIfNotCurrentlyActingUpdatesInDev as warnIfNotCurrentlyActingUpdatesInDev_new,
inferStartTimeFromExpirationTime as inferStartTimeFromExpirationTime_new,
} from './ReactFiberScheduler.new';

export const requestCurrentTime = enableNewScheduler
Expand All @@ -77,6 +77,9 @@ export const captureCommitPhaseError = enableNewScheduler
export const onUncaughtError = enableNewScheduler
? onUncaughtError_new
: onUncaughtError_old;
export const markRenderEventTime = enableNewScheduler
? markRenderEventTime_new
: markRenderEventTime_old;
export const renderDidSuspend = enableNewScheduler
? renderDidSuspend_new
: renderDidSuspend_old;
Expand Down Expand Up @@ -133,9 +136,6 @@ export const flushPassiveEffects = enableNewScheduler
export const warnIfNotCurrentlyActingUpdatesInDev = enableNewScheduler
? warnIfNotCurrentlyActingUpdatesInDev_new
: warnIfNotCurrentlyActingUpdatesInDev_old;
export const inferStartTimeFromExpirationTime = enableNewScheduler
? inferStartTimeFromExpirationTime_new
: inferStartTimeFromExpirationTime_old;

export type Thenable = {
then(resolve: () => mixed, reject?: () => mixed): void | Thenable,
Expand Down