Skip to content

Commit 2c169a5

Browse files
acdliteacdlite
committed
Only show most recent transition, per queue
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.
1 parent 10ea752 commit 2c169a5

File tree

4 files changed

+255
-15
lines changed

4 files changed

+255
-15
lines changed

‎packages/react-reconciler/src/ReactFiberHooks.js

Lines changed: 35 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -52,6 +52,7 @@ import warning from 'shared/warning';
5252
import getComponentName from 'shared/getComponentName';
5353
import is from 'shared/objectIs';
5454
import {markWorkInProgressReceivedUpdate} from './ReactFiberBeginWork';
55+
import {SuspendOnTask} from './ReactFiberThrow';
5556
import {requestCurrentSuspenseConfig} from './ReactFiberSuspenseConfig';
5657
import {
5758
startTransition,
@@ -761,7 +762,10 @@ function updateReducer<S, I, A>(
761762
let prevUpdate = baseUpdate;
762763
let update = first;
763764
let didSkip = false;
765+
let lastProcessedTransitionTime = NoWork;
766+
let lastSkippedTransitionTime = NoWork;
764767
do {
768+
const suspenseConfig = update.suspenseConfig;
765769
const updateExpirationTime = update.expirationTime;
766770
if (updateExpirationTime < renderExpirationTime) {
767771
// Priority is insufficient. Skip this update. If this is the first
@@ -777,6 +781,16 @@ function updateReducer<S, I, A>(
777781
remainingExpirationTime = updateExpirationTime;
778782
markUnprocessedUpdateTime(remainingExpirationTime);
779783
}
784+
785+
if (suspenseConfig !== null) {
786+
// This update is part of a transition
787+
if (
788+
lastSkippedTransitionTime === NoWork ||
789+
lastSkippedTransitionTime > updateExpirationTime
790+
) {
791+
lastSkippedTransitionTime = updateExpirationTime;
792+
}
793+
}
780794
} else {
781795
// This update does have sufficient priority.
782796
// Mark the event time of this update as relevant to this render pass.
@@ -785,10 +799,7 @@ function updateReducer<S, I, A>(
785799
// TODO: We should skip this update if it was already committed but currently
786800
// we have no way of detecting the difference between a committed and suspended
787801
// update here.
788-
markRenderEventTimeAndConfig(
789-
updateExpirationTime,
790-
update.suspenseConfig,
791-
);
802+
markRenderEventTimeAndConfig(updateExpirationTime, suspenseConfig);
792803
793804
// Process this update.
794805
if (update.eagerReducer === reducer) {
@@ -799,12 +810,32 @@ function updateReducer<S, I, A>(
799810
const action = update.action;
800811
newState = reducer(newState, action);
801812
}
813+
814+
if (suspenseConfig !== null) {
815+
// This update is part of a transition
816+
if (
817+
lastProcessedTransitionTime === NoWork ||
818+
lastProcessedTransitionTime > updateExpirationTime
819+
) {
820+
lastProcessedTransitionTime = updateExpirationTime;
821+
}
822+
}
802823
}
803824
804825
prevUpdate = update;
805826
update = update.next;
806827
} while (update !== null && update !== first);
807828
829+
if (
830+
lastProcessedTransitionTime !== NoWork &&
831+
lastSkippedTransitionTime !== NoWork
832+
) {
833+
// There are multiple updates scheduled on this queue, but only some of
834+
// them were processed. To avoid showing an intermediate state, abort
835+
// the current render and restart at a level that includes them all.
836+
throw SuspendOnTask(lastSkippedTransitionTime);
837+
}
838+
808839
if (!didSkip) {
809840
newBaseUpdate = prevUpdate;
810841
newBaseState = newState;

‎packages/react-reconciler/src/ReactFiberThrow.js

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -62,6 +62,14 @@ import {Sync} from './ReactFiberExpirationTime';
6262

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

65+
// Throw an object with this type to abort the current render and restart at
66+
// a different level.
67+
export function SuspendOnTask(expirationTime: ExpirationTime) {
68+
return {
69+
_retryTime: expirationTime,
70+
};
71+
}
72+
6573
function createRootErrorUpdate(
6674
fiber: Fiber,
6775
errorInfo: CapturedValue<mixed>,

‎packages/react-reconciler/src/ReactFiberWorkLoop.js

Lines changed: 36 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -200,13 +200,14 @@ const LegacyUnbatchedContext = /* */ 0b001000;
200200
const RenderContext = /* */ 0b010000;
201201
const CommitContext = /* */ 0b100000;
202202

203-
type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5;
203+
type RootExitStatus = 0 | 1 | 2 | 3 | 4 | 5 | 6;
204204
const RootIncomplete = 0;
205205
const RootFatalErrored = 1;
206-
const RootErrored = 2;
207-
const RootSuspended = 3;
208-
const RootSuspendedWithDelay = 4;
209-
const RootCompleted = 5;
206+
const RootSuspendedOnTask = 2;
207+
const RootErrored = 3;
208+
const RootSuspended = 4;
209+
const RootSuspendedWithDelay = 5;
210+
const RootCompleted = 6;
210211

211212
export type Thenable = {
212213
then(resolve: () => mixed, reject?: () => mixed): Thenable | void,
@@ -237,7 +238,7 @@ let workInProgressRootCanSuspendUsingConfig: null | SuspenseConfig = null;
237238
// The work left over by components that were visited during this render. Only
238239
// includes unprocessed updates, not work in bailed out children.
239240
let workInProgressRootNextUnprocessedUpdateTime: ExpirationTime = NoWork;
240-
241+
let workInProgressRootRestartTime: ExpirationTime = NoWork;
241242
// If we're pinged while rendering we don't always restart immediately.
242243
// This flag determines if it might be worthwhile to restart if an opportunity
243244
// happens latere.
@@ -697,7 +698,12 @@ function performConcurrentWorkOnRoot(root, didTimeout) {
697698
throw fatalError;
698699
}
699700

700-
if (workInProgress !== null) {
701+
if (workInProgressRootExitStatus === RootSuspendedOnTask) {
702+
// Can't finish rendering at this level. Exit early and restart at the
703+
// specified time.
704+
markRootSuspendedAtTime(root, expirationTime);
705+
root.nextKnownPendingLevel = workInProgressRootRestartTime;
706+
} else if (workInProgress !== null) {
701707
// There's still work left over. Exit without committing.
702708
stopInterruptedWorkLoopTimer();
703709
} else {
@@ -738,7 +744,8 @@ function finishConcurrentRender(
738744

739745
switch (exitStatus) {
740746
case RootIncomplete:
741-
case RootFatalErrored: {
747+
case RootFatalErrored:
748+
case RootSuspendedOnTask: {
742749
invariant(false, 'Root did not complete. This is a bug in React.');
743750
}
744751
// Flow knows about invariant, so it complains if I add a break
@@ -1262,6 +1269,7 @@ function prepareFreshStack(root, expirationTime) {
12621269
workInProgressRootLatestSuspenseTimeout = Sync;
12631270
workInProgressRootCanSuspendUsingConfig = null;
12641271
workInProgressRootNextUnprocessedUpdateTime = NoWork;
1272+
workInProgressRootRestartTime = NoWork;
12651273
workInProgressRootHasPendingPing = false;
12661274

12671275
if (enableSchedulerTracing) {
@@ -1299,6 +1307,21 @@ function handleError(root, thrownValue) {
12991307
stopProfilerTimerIfRunningAndRecordDelta(workInProgress, true);
13001308
}
13011309

1310+
// Check if this is a SuspendOnTask exception.
1311+
// TODO: Should this be a brand check?
1312+
if (
1313+
thrownValue !== null &&
1314+
typeof thrownValue === 'object' &&
1315+
typeof thrownValue._retryTime === 'number'
1316+
) {
1317+
// Can't finish rendering at this level. Exit early and restart at
1318+
// the specified time.
1319+
workInProgressRootExitStatus = RootSuspendedOnTask;
1320+
workInProgressRootRestartTime = thrownValue._retryTime;
1321+
workInProgress = null;
1322+
return;
1323+
}
1324+
13021325
throwException(
13031326
root,
13041327
workInProgress.return,
@@ -2623,15 +2646,17 @@ if (__DEV__ && replayFailedUnitOfWorkWithInvokeGuardedCallback) {
26232646
try {
26242647
return originalBeginWork(current, unitOfWork, expirationTime);
26252648
} catch (originalError) {
2649+
// Filter out special exception types
26262650
if (
26272651
originalError !== null &&
26282652
typeof originalError === 'object' &&
2629-
typeof originalError.then === 'function'
2653+
// Promise
2654+
(typeof originalError.then === 'function' ||
2655+
// SuspendOnTask exception
2656+
typeof originalError._retryTime === 'number')
26302657
) {
2631-
// Don't replay promises. Treat everything else like an error.
26322658
throw originalError;
26332659
}
2634-
26352660
// Keep this code in sync with handleError; any changes here must have
26362661
// corresponding changes there.
26372662
resetContextDependencies();

‎packages/react-reconciler/src/__tests__/ReactTransition-test.internal.js

Lines changed: 176 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ describe('ReactTransition', () => {
2727
ReactFeatureFlags.debugRenderPhaseSideEffectsForStrictMode = false;
2828
ReactFeatureFlags.enableSchedulerTracing = true;
2929
ReactFeatureFlags.flushSuspenseFallbacksInTests = false;
30+
ReactFeatureFlags.replayFailedUnitOfWorkWithInvokeGuardedCallback = false;
3031
React = require('react');
3132
ReactNoop = require('react-noop-renderer');
3233
Scheduler = require('scheduler');
@@ -472,4 +473,179 @@ describe('ReactTransition', () => {
472473
);
473474
},
474475
);
476+
477+
// TODO: Same behavior for classes
478+
it.experimental(
479+
'when multiple transitions update the same queue, only the most recent ' +
480+
'one is allowed to finish (no intermediate states)',
481+
async () => {
482+
const CONFIG = {
483+
timeoutMs: 100000,
484+
};
485+
486+
const Tab = React.forwardRef(({label, setTab}, ref) => {
487+
const [startTransition, isPending] = useTransition(CONFIG);
488+
489+
React.useImperativeHandle(
490+
ref,
491+
() => ({
492+
go() {
493+
startTransition(() => setTab(label));
494+
},
495+
}),
496+
[label],
497+
);
498+
499+
return (
500+
<Text text={'Tab ' + label + (isPending ? ' (pending...)' : '')} />
501+
);
502+
});
503+
504+
const tabButtonA = React.createRef(null);
505+
const tabButtonB = React.createRef(null);
506+
const tabButtonC = React.createRef(null);
507+
508+
const ContentA = createAsyncText('A');
509+
const ContentB = createAsyncText('B');
510+
const ContentC = createAsyncText('C');
511+
512+
function App() {
513+
Scheduler.unstable_yieldValue('App');
514+
515+
const [tab, setTab] = useState('A');
516+
517+
let content;
518+
switch (tab) {
519+
case 'A':
520+
content = <ContentA />;
521+
break;
522+
case 'B':
523+
content = <ContentB />;
524+
break;
525+
case 'C':
526+
content = <ContentC />;
527+
break;
528+
default:
529+
content = <ContentA />;
530+
break;
531+
}
532+
533+
return (
534+
<>
535+
<ul>
536+
<li>
537+
<Tab ref={tabButtonA} label="A" setTab={setTab} />
538+
</li>
539+
<li>
540+
<Tab ref={tabButtonB} label="B" setTab={setTab} />
541+
</li>
542+
<li>
543+
<Tab ref={tabButtonC} label="C" setTab={setTab} />
544+
</li>
545+
</ul>
546+
<Suspense fallback={<Text text="Loading..." />}>{content}</Suspense>
547+
</>
548+
);
549+
}
550+
551+
// Initial render
552+
const root = ReactNoop.createRoot();
553+
await act(async () => {
554+
root.render(<App />);
555+
await ContentA.resolve();
556+
});
557+
expect(Scheduler).toHaveYielded(['App', 'Tab A', 'Tab B', 'Tab C', 'A']);
558+
expect(root).toMatchRenderedOutput(
559+
<>
560+
<ul>
561+
<li>Tab A</li>
562+
<li>Tab B</li>
563+
<li>Tab C</li>
564+
</ul>
565+
A
566+
</>,
567+
);
568+
569+
// Navigate to tab B
570+
await act(async () => {
571+
tabButtonB.current.go();
572+
expect(Scheduler).toFlushAndYieldThrough([
573+
// Turn on B's pending state
574+
'Tab B (pending...)',
575+
// Partially render B
576+
'App',
577+
'Tab A',
578+
'Tab B',
579+
]);
580+
581+
// While we're still in the middle of rendering B, switch to C.
582+
tabButtonC.current.go();
583+
});
584+
expect(Scheduler).toHaveYielded([
585+
// Toggle the pending flags
586+
'Tab B',
587+
'Tab C (pending...)',
588+
589+
// Start rendering B...
590+
'App',
591+
// ...but bail out, since C is more recent. These should not be logged:
592+
// 'Tab A',
593+
// 'Tab B',
594+
// 'Tab C (pending...)',
595+
// 'Suspend! [B]',
596+
// 'Loading...',
597+
598+
// Now render C
599+
'App',
600+
'Tab A',
601+
'Tab B',
602+
'Tab C',
603+
'Suspend! [C]',
604+
'Loading...',
605+
]);
606+
expect(root).toMatchRenderedOutput(
607+
<>
608+
<ul>
609+
<li>Tab A</li>
610+
<li>Tab B</li>
611+
<li>Tab C (pending...)</li>
612+
</ul>
613+
A
614+
</>,
615+
);
616+
617+
// Finish loading B
618+
await act(async () => {
619+
ContentB.resolve();
620+
});
621+
// Should not switch to tab B because we've since clicked on C.
622+
expect(Scheduler).toHaveYielded([]);
623+
expect(root).toMatchRenderedOutput(
624+
<>
625+
<ul>
626+
<li>Tab A</li>
627+
<li>Tab B</li>
628+
<li>Tab C (pending...)</li>
629+
</ul>
630+
A
631+
</>,
632+
);
633+
634+
// Finish loading C
635+
await act(async () => {
636+
ContentC.resolve();
637+
});
638+
expect(Scheduler).toHaveYielded(['App', 'Tab A', 'Tab B', 'Tab C', 'C']);
639+
expect(root).toMatchRenderedOutput(
640+
<>
641+
<ul>
642+
<li>Tab A</li>
643+
<li>Tab B</li>
644+
<li>Tab C</li>
645+
</ul>
646+
C
647+
</>,
648+
);
649+
},
650+
);
475651
});

0 commit comments

Comments
 (0)