Skip to content

Commit

Permalink
Only show most recent transition, per queue
Browse files Browse the repository at this point in the history 
When multiple transitions update the same queue, only the most recent
one should be allowed to finish. Do not display intermediate states.

For example, if you click on multiple tabs in quick succession, we
should not switch to any tab that isn't the last one you clicked.
  • Loading branch information
@acdlite
acdlite committed Dec 19, 2019
1 parent d499067 commit da8c9f2
Show file tree
Hide file tree
Showing 12 changed files with 491 additions and 19 deletions.
41 changes: 37 additions & 4 deletions packages/react-reconciler/src/ReactFiberHooks.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import type {SuspenseConfig} from './ReactFiberSuspenseConfig';
import type {TransitionInstance} from './ReactFiberTransition';
import type {ReactPriorityLevel} from './SchedulerWithReactIntegration';

import {preventIntermediateStates} from 'shared/ReactFeatureFlags';
import ReactSharedInternals from 'shared/ReactSharedInternals';

import {NoWork, Sync} from './ReactFiberExpirationTime';
Expand Down Expand Up @@ -50,6 +51,7 @@ import invariant from 'shared/invariant';
import getComponentName from 'shared/getComponentName';
import is from 'shared/objectIs';
import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork';
import {SuspendOnTask} from './ReactFiberThrow';
import {requestCurrentSuspenseConfig} from './ReactFiberSuspenseConfig';
import {
startTransition,
Expand Down Expand Up @@ -705,7 +707,10 @@ function updateReducer<S, I, A>(
let newBaseQueueFirst = null;
let newBaseQueueLast = null;
let update = first;
let lastProcessedTransitionTime = NoWork;
let lastSkippedTransitionTime = NoWork;
do {
const suspenseConfig = update.suspenseConfig;
const updateExpirationTime = update.expirationTime;
if (updateExpirationTime < renderExpirationTime) {
// Priority is insufficient. Skip this update. If this is the first
Expand All @@ -730,6 +735,16 @@ function updateReducer<S, I, A>(
currentlyRenderingFiber.expirationTime = updateExpirationTime;
markUnprocessedUpdateTime(updateExpirationTime);
}

if (suspenseConfig !== null) {
// This update is part of a transition
if (
lastSkippedTransitionTime === NoWork ||
lastSkippedTransitionTime > updateExpirationTime
) {
lastSkippedTransitionTime = updateExpirationTime;
}
}
} else {
// This update does have sufficient priority.

Expand All @@ -751,10 +766,7 @@ function updateReducer<S, I, A>(
// 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.
markRenderEventTimeAndConfig(
updateExpirationTime,
update.suspenseConfig,
);
markRenderEventTimeAndConfig(updateExpirationTime, suspenseConfig);

// Process this update.
if (update.eagerReducer === reducer) {
Expand All @@ -765,10 +777,31 @@ function updateReducer<S, I, A>(
const action = update.action;
newState = reducer(newState, action);
}

if (suspenseConfig !== null) {
// This update is part of a transition
if (
lastProcessedTransitionTime === NoWork ||
lastProcessedTransitionTime > updateExpirationTime
) {
lastProcessedTransitionTime = updateExpirationTime;
}
}
}
update = update.next;
} while (update !== null && update !== first);

if (
preventIntermediateStates &&
lastProcessedTransitionTime !== NoWork &&
lastSkippedTransitionTime !== NoWork
) {
// There are multiple updates scheduled on this queue, but only some of
// them were processed. To avoid showing an intermediate state, abort
// the current render and restart at a level that includes them all.
throw new SuspendOnTask(lastSkippedTransitionTime);
}

if (newBaseQueueLast === null) {
newBaseState = newState;
} else {
Expand Down
6 changes: 6 additions & 0 deletions packages/react-reconciler/src/ReactFiberThrow.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,6 +61,12 @@ import {Sync} from './ReactFiberExpirationTime';

const PossiblyWeakMap = typeof WeakMap === 'function' ? WeakMap : Map;

// Throw an object with this type to abort the current render and restart at
// a different level.
export function SuspendOnTask(expirationTime: ExpirationTime) {
this.retryTime = expirationTime;
}

function createRootErrorUpdate(
fiber: Fiber,
errorInfo: CapturedValue<mixed>,
Expand Down
54 changes: 42 additions & 12 deletions packages/react-reconciler/src/ReactFiberWorkLoop.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -127,6 +127,7 @@ import {
throwException,
createRootErrorUpdate,
createClassErrorUpdate,
SuspendOnTask,
} from './ReactFiberThrow';
import {
commitBeforeMutationLifeCycles as commitBeforeMutationEffectOnFiber,
Expand Down Expand Up @@ -201,13 +202,14 @@ const LegacyUnbatchedContext = /* */ 0b001000;
const RenderContext = /* */ 0b010000;
const CommitContext = /* */ 0b100000;

type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5;
type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5 | 6;
const RootIncomplete = 0;
const RootFatalErrored = 1;
const RootErrored = 2;
const RootSuspended = 3;
const RootSuspendedWithDelay = 4;
const RootCompleted = 5;
const RootSuspendedOnTask = 2;
const RootErrored = 3;
const RootSuspended = 4;
const RootSuspendedWithDelay = 5;
const RootCompleted = 6;

export type Thenable = {
then(resolve: () => mixed, reject?: () => mixed): Thenable | void,
Expand Down Expand Up @@ -238,7 +240,7 @@ let workInProgressRootCanSuspendUsingConfig: null | SuspenseConfig = null;
// The work left over by components that were visited during this render. Only
// includes unprocessed updates, not work in bailed out children.
let workInProgressRootNextUnprocessedUpdateTime: ExpirationTime = NoWork;

let workInProgressRootRestartTime: ExpirationTime = NoWork;
// If we're pinged while rendering we don't always restart immediately.
// This flag determines if it might be worthwhile to restart if an opportunity
// happens latere.
Expand Down Expand Up @@ -708,7 +710,12 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
throw fatalError;
}

if (workInProgress !== null) {
if (workInProgressRootExitStatus === RootSuspendedOnTask) {
// Can't finish rendering at this level. Exit early and restart at the
// specified time.
markRootSuspendedAtTime(root, expirationTime);
root.nextKnownPendingLevel = workInProgressRootRestartTime;
} else if (workInProgress !== null) {
// There's still work left over. Exit without committing.
stopInterruptedWorkLoopTimer();
} else {
Expand Down Expand Up @@ -749,7 +756,8 @@ function finishConcurrentRender(

switch (exitStatus) {
case RootIncomplete:
case RootFatalErrored: {
case RootFatalErrored:
case RootSuspendedOnTask: {
invariant(false, 'Root did not complete. This is a bug in React.');
}
// Flow knows about invariant, so it complains if I add a break
Expand Down Expand Up @@ -1036,7 +1044,12 @@ function performSyncWorkOnRoot(root) {
throw fatalError;
}

if (workInProgress !== null) {
if (workInProgressRootExitStatus === RootSuspendedOnTask) {
// Can't finish rendering at this level. Exit early and restart at the
// specified time.
markRootSuspendedAtTime(root, expirationTime);
root.nextKnownPendingLevel = workInProgressRootRestartTime;
} else if (workInProgress !== null) {
// This is a sync render, so we should have finished the whole tree.
invariant(
false,
Expand Down Expand Up @@ -1264,6 +1277,7 @@ function prepareFreshStack(root, expirationTime) {
workInProgressRootLatestSuspenseTimeout = Sync;
workInProgressRootCanSuspendUsingConfig = null;
workInProgressRootNextUnprocessedUpdateTime = NoWork;
workInProgressRootRestartTime = NoWork;
workInProgressRootHasPendingPing = false;

if (enableSchedulerTracing) {
Expand All @@ -1284,6 +1298,20 @@ function handleError(root, thrownValue) {
resetHooksAfterThrow();
resetCurrentDebugFiberInDEV();

// Check if this is a SuspendOnTask exception. This is the one type of
// exception that is allowed to happen at the root.
// TODO: I think instanceof is OK here? A brand check seems unnecessary
// since this is always thrown by the renderer and not across realms
// or packages.
if (thrownValue instanceof SuspendOnTask) {
// Can't finish rendering at this level. Exit early and restart at
// the specified time.
workInProgressRootExitStatus = RootSuspendedOnTask;
workInProgressRootRestartTime = thrownValue.retryTime;
workInProgress = null;
return;
}

if (workInProgress === null || workInProgress.return === null) {
// Expected to be working on a non-root fiber. This is a fatal error
// because there's no ancestor that can handle it; the root is
Expand Down Expand Up @@ -2624,15 +2652,17 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
try {
return originalBeginWork(current, unitOfWork, expirationTime);
} catch (originalError) {
// Filter out special exception types
if (
originalError !== null &&
typeof originalError === 'object' &&
typeof originalError.then === 'function'
// Promise
(typeof originalError.then === 'function' ||
// SuspendOnTask exception
originalError instanceof SuspendOnTask)
) {
// Don't replay promises. Treat everything else like an error.
throw originalError;
}

// Keep this code in sync with handleError; any changes here must have
// corresponding changes there.
resetContextDependencies();
Expand Down
45 changes: 42 additions & 3 deletions packages/react-reconciler/src/ReactUpdateQueue.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -97,13 +97,17 @@ import {
} from './ReactFiberNewContext';
import {Callback, ShouldCapture, DidCapture} from 'shared/ReactSideEffectTags';

import {debugRenderPhaseSideEffectsForStrictMode} from 'shared/ReactFeatureFlags';
import {
debugRenderPhaseSideEffectsForStrictMode,
preventIntermediateStates,
} from 'shared/ReactFeatureFlags';

import {StrictMode} from './ReactTypeOfMode';
import {
markRenderEventTimeAndConfig,
markUnprocessedUpdateTime,
} from './ReactFiberWorkLoop';
import {SuspendOnTask} from './ReactFiberThrow';

import invariant from 'shared/invariant';
import {getCurrentPriorityLevel} from './SchedulerWithReactIntegration';
Expand Down Expand Up @@ -413,15 +417,18 @@ export function processUpdateQueue<State>(

if (first !== null) {
let update = first;
let lastProcessedTransitionTime = NoWork;
let lastSkippedTransitionTime = NoWork;
do {
const updateExpirationTime = update.expirationTime;
const suspenseConfig = update.suspenseConfig;
if (updateExpirationTime < renderExpirationTime) {
// Priority is insufficient. Skip this update. If this is the first
// skipped update, the previous update/state is the new base
// update/state.
const clone: Update<State> = {
expirationTime: update.expirationTime,
suspenseConfig: update.suspenseConfig,
expirationTime: updateExpirationTime,
suspenseConfig,

tag: update.tag,
payload: update.payload,
Expand All @@ -439,6 +446,16 @@ export function processUpdateQueue<State>(
if (updateExpirationTime > newExpirationTime) {
newExpirationTime = updateExpirationTime;
}

if (suspenseConfig !== null) {
// This update is part of a transition
if (
lastSkippedTransitionTime === NoWork ||
lastSkippedTransitionTime > updateExpirationTime
) {
lastSkippedTransitionTime = updateExpirationTime;
}
}
} else {
// This update does have sufficient priority.

Expand Down Expand Up @@ -487,6 +504,17 @@ export function processUpdateQueue<State>(
}
}
}

if (suspenseConfig !== null) {
// This update is part of a transition
if (
lastProcessedTransitionTime === NoWork ||
lastProcessedTransitionTime > updateExpirationTime
) {
lastProcessedTransitionTime = updateExpirationTime;
}
}

update = update.next;
if (update === null || update === first) {
pendingQueue = queue.shared.pending;
Expand All @@ -502,6 +530,17 @@ export function processUpdateQueue<State>(
}
}
} while (true);

if (
preventIntermediateStates &&
lastProcessedTransitionTime !== NoWork &&
lastSkippedTransitionTime !== NoWork
) {
// There are multiple updates scheduled on this queue, but only some of
// them were processed. To avoid showing an intermediate state, abort
// the current render and restart at a level that includes them all.
throw new SuspendOnTask(lastSkippedTransitionTime);
}
}

if (newBaseQueueLast === null) {
Expand Down
Loading

0 comments on commit da8c9f2

Please sign in to comment.