Skip to content

Commit 3721e62

Browse files
acdliterickhanlonii
andcommitted
Default updates should not interrupt transitions
The only difference between default updates and transition updates is that default updates do not support suspended refreshes — they will instantly display a fallback. Co-authored-by: Rick Hanlon <[email protected]>
1 parent 3499c34 commit 3721e62

14 files changed

+239
-4
lines changed

packages/react-reconciler/src/ReactFiberLane.new.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import invariant from 'shared/invariant';
3939
import {
4040
enableCache,
4141
enableTransitionEntanglement,
42+
enableNonInterruptingNormalPri,
4243
} from 'shared/ReactFeatureFlags';
4344

4445
import {
@@ -327,7 +328,16 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
327328
) {
328329
getHighestPriorityLanes(wipLanes);
329330
const wipLanePriority = return_highestLanePriority;
330-
if (nextLanePriority <= wipLanePriority) {
331+
if (
332+
nextLanePriority <= wipLanePriority ||
333+
// Default priority updates should not interrupt transition updates. The
334+
// only difference between default updates and transition updates is that
335+
// default updates do not support refresh transitions.
336+
(enableNonInterruptingNormalPri &&
337+
nextLanePriority === DefaultLanePriority &&
338+
wipLanePriority === TransitionPriority)
339+
) {
340+
// Keep working on the existing in-progress tree. Do not interrupt.
331341
return wipLanes;
332342
} else {
333343
return_highestLanePriority = nextLanePriority;

packages/react-reconciler/src/ReactFiberLane.old.js

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ import invariant from 'shared/invariant';
3939
import {
4040
enableCache,
4141
enableTransitionEntanglement,
42+
enableNonInterruptingNormalPri,
4243
} from 'shared/ReactFeatureFlags';
4344

4445
import {
@@ -327,7 +328,16 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
327328
) {
328329
getHighestPriorityLanes(wipLanes);
329330
const wipLanePriority = return_highestLanePriority;
330-
if (nextLanePriority <= wipLanePriority) {
331+
if (
332+
nextLanePriority <= wipLanePriority ||
333+
// Default priority updates should not interrupt transition updates. The
334+
// only difference between default updates and transition updates is that
335+
// default updates do not support refresh transitions.
336+
(enableNonInterruptingNormalPri &&
337+
nextLanePriority === DefaultLanePriority &&
338+
wipLanePriority === TransitionPriority)
339+
) {
340+
// Keep working on the existing in-progress tree. Do not interrupt.
331341
return wipLanes;
332342
} else {
333343
return_highestLanePriority = nextLanePriority;

packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1849,10 +1849,12 @@ describe('ReactIncrementalErrorHandling', () => {
18491849
// the queue.
18501850
expect(Scheduler).toFlushAndYieldThrough(['Everything is fine.']);
18511851

1852-
// Schedule a default pri update on a child that triggers an error.
1852+
// Schedule a discrete update on a child that triggers an error.
18531853
// The root should capture this error. But since there's still a pending
18541854
// update on the root, the error should be suppressed.
1855-
setShouldThrow(true);
1855+
ReactNoop.discreteUpdates(() => {
1856+
setShouldThrow(true);
1857+
});
18561858
});
18571859
// Should render the final state without throwing the error.
18581860
expect(Scheduler).toHaveYielded(['Everything is fine.']);

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

Lines changed: 202 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@ let ReactNoop;
1515
let Scheduler;
1616
let Suspense;
1717
let useState;
18+
let useLayoutEffect;
1819
let useTransition;
1920
let startTransition;
2021
let act;
@@ -30,6 +31,7 @@ describe('ReactTransition', () => {
3031
ReactNoop = require('react-noop-renderer');
3132
Scheduler = require('scheduler');
3233
useState = React.useState;
34+
useLayoutEffect = React.useLayoutEffect;
3335
useTransition = React.unstable_useTransition;
3436
Suspense = React.Suspense;
3537
startTransition = React.unstable_startTransition;
@@ -773,4 +775,204 @@ describe('ReactTransition', () => {
773775
});
774776
},
775777
);
778+
779+
// @gate experimental
780+
// @gate enableCache
781+
it('should render normal pri updates scheduled after transitions before transitions', async () => {
782+
let updateTransitionPri;
783+
let updateNormalPri;
784+
function App() {
785+
const [normalPri, setNormalPri] = useState(0);
786+
const [transitionPri, setTransitionPri] = useState(0);
787+
updateTransitionPri = () =>
788+
startTransition(() => setTransitionPri(n => n + 1));
789+
updateNormalPri = () => setNormalPri(n => n + 1);
790+
791+
useLayoutEffect(() => {
792+
Scheduler.unstable_yieldValue('Commit');
793+
});
794+
795+
return (
796+
<Suspense fallback={<Text text="Loading..." />}>
797+
<Text text={'Transition pri: ' + transitionPri} />
798+
{', '}
799+
<Text text={'Normal pri: ' + normalPri} />
800+
</Suspense>
801+
);
802+
}
803+
804+
const root = ReactNoop.createRoot();
805+
await act(async () => {
806+
root.render(<App />);
807+
});
808+
809+
// Initial render.
810+
expect(Scheduler).toHaveYielded([
811+
'Transition pri: 0',
812+
'Normal pri: 0',
813+
'Commit',
814+
]);
815+
expect(root).toMatchRenderedOutput('Transition pri: 0, Normal pri: 0');
816+
817+
await act(async () => {
818+
updateTransitionPri();
819+
updateNormalPri();
820+
});
821+
822+
expect(Scheduler).toHaveYielded([
823+
// Normal update first.
824+
'Transition pri: 0',
825+
'Normal pri: 1',
826+
'Commit',
827+
828+
// Then transition update.
829+
'Transition pri: 1',
830+
'Normal pri: 1',
831+
'Commit',
832+
]);
833+
expect(root).toMatchRenderedOutput('Transition pri: 1, Normal pri: 1');
834+
});
835+
836+
// @gate experimental
837+
// @gate enableCache
838+
it('should render normal pri updates before transition suspense retries', async () => {
839+
let updateTransitionPri;
840+
let updateNormalPri;
841+
function App() {
842+
const [transitionPri, setTransitionPri] = useState(false);
843+
const [normalPri, setNormalPri] = useState(0);
844+
845+
updateTransitionPri = () => startTransition(() => setTransitionPri(true));
846+
updateNormalPri = () => setNormalPri(n => n + 1);
847+
848+
useLayoutEffect(() => {
849+
Scheduler.unstable_yieldValue('Commit');
850+
});
851+
852+
return (
853+
<Suspense fallback={<Text text="Loading..." />}>
854+
{transitionPri ? <AsyncText text="Async" /> : <Text text="(empty)" />}
855+
{', '}
856+
<Text text={'Normal pri: ' + normalPri} />
857+
</Suspense>
858+
);
859+
}
860+
861+
const root = ReactNoop.createRoot();
862+
await act(async () => {
863+
root.render(<App />);
864+
});
865+
866+
// Initial render.
867+
expect(Scheduler).toHaveYielded(['(empty)', 'Normal pri: 0', 'Commit']);
868+
expect(root).toMatchRenderedOutput('(empty), Normal pri: 0');
869+
870+
await act(async () => {
871+
updateTransitionPri();
872+
});
873+
874+
expect(Scheduler).toHaveYielded([
875+
// Suspend.
876+
'Suspend! [Async]',
877+
'Normal pri: 0',
878+
'Loading...',
879+
]);
880+
expect(root).toMatchRenderedOutput('(empty), Normal pri: 0');
881+
882+
await act(async () => {
883+
await resolveText('Async');
884+
updateNormalPri();
885+
});
886+
887+
expect(Scheduler).toHaveYielded([
888+
// Normal pri update.
889+
'(empty)',
890+
'Normal pri: 1',
891+
'Commit',
892+
893+
// Promise resolved, retry flushed.
894+
'Async',
895+
'Normal pri: 1',
896+
'Commit',
897+
]);
898+
expect(root).toMatchRenderedOutput('Async, Normal pri: 1');
899+
});
900+
901+
// @gate experimental
902+
// @gate enableCache
903+
it('should not interrupt transitions with normal pri updates', async () => {
904+
let updateNormalPri;
905+
let updateTransitionPri;
906+
function App() {
907+
const [transitionPri, setTransitionPri] = useState(0);
908+
const [normalPri, setNormalPri] = useState(0);
909+
updateTransitionPri = () =>
910+
startTransition(() => setTransitionPri(n => n + 1));
911+
updateNormalPri = () => setNormalPri(n => n + 1);
912+
913+
useLayoutEffect(() => {
914+
Scheduler.unstable_yieldValue('Commit');
915+
});
916+
return (
917+
<>
918+
<Text text={'Transition pri: ' + transitionPri} />
919+
{', '}
920+
<Text text={'Normal pri: ' + normalPri} />
921+
</>
922+
);
923+
}
924+
925+
const root = ReactNoop.createRoot();
926+
await ReactNoop.act(async () => {
927+
root.render(<App />);
928+
});
929+
expect(Scheduler).toHaveYielded([
930+
'Transition pri: 0',
931+
'Normal pri: 0',
932+
'Commit',
933+
]);
934+
expect(root).toMatchRenderedOutput('Transition pri: 0, Normal pri: 0');
935+
936+
await ReactNoop.act(async () => {
937+
updateTransitionPri();
938+
939+
expect(Scheduler).toFlushAndYieldThrough([
940+
// Start transition update.
941+
'Transition pri: 1',
942+
]);
943+
944+
// Schedule normal pri update during transition update.
945+
// This should not interrupt.
946+
updateNormalPri();
947+
});
948+
949+
if (gate(flags => flags.enableNonInterruptingNormalPri)) {
950+
expect(Scheduler).toHaveYielded([
951+
// Finish transition update.
952+
'Normal pri: 0',
953+
'Commit',
954+
955+
// Normal pri update.
956+
'Transition pri: 1',
957+
'Normal pri: 1',
958+
'Commit',
959+
]);
960+
961+
expect(root).toMatchRenderedOutput('Transition pri: 1, Normal pri: 1');
962+
} else {
963+
expect(Scheduler).toHaveYielded([
964+
// Interrupt! Render normal pri update.
965+
'Transition pri: 0',
966+
'Normal pri: 1',
967+
'Commit',
968+
969+
// Restart transition update.
970+
'Transition pri: 1',
971+
'Normal pri: 1',
972+
'Commit',
973+
]);
974+
975+
expect(root).toMatchRenderedOutput('Transition pri: 1, Normal pri: 1');
976+
}
977+
});
776978
});

packages/shared/ReactFeatureFlags.js

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -150,6 +150,8 @@ export const disableSchedulerTimeoutInWorkLoop = false;
150150
// Experiment to simplify/improve how transitions are scheduled
151151
export const enableTransitionEntanglement = false;
152152

153+
export const enableNonInterruptingNormalPri = false;
154+
153155
export const enableDiscreteEventMicroTasks = false;
154156

155157
export const enableNativeEventPriorityInference = false;

packages/shared/forks/ReactFeatureFlags.native-fb.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,7 @@ export const enableUseRefAccessWarning = false;
5858
export const enableRecursiveCommitTraversal = false;
5959
export const disableSchedulerTimeoutInWorkLoop = false;
6060
export const enableTransitionEntanglement = false;
61+
export const enableNonInterruptingNormalPri = false;
6162
export const enableDiscreteEventMicroTasks = false;
6263
export const enableNativeEventPriorityInference = false;
6364

packages/shared/forks/ReactFeatureFlags.native-oss.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ export const enableUseRefAccessWarning = false;
5757
export const enableRecursiveCommitTraversal = false;
5858
export const disableSchedulerTimeoutInWorkLoop = false;
5959
export const enableTransitionEntanglement = false;
60+
export const enableNonInterruptingNormalPri = false;
6061
export const enableDiscreteEventMicroTasks = false;
6162
export const enableNativeEventPriorityInference = false;
6263

packages/shared/forks/ReactFeatureFlags.test-renderer.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ export const enableUseRefAccessWarning = false;
5757
export const enableRecursiveCommitTraversal = false;
5858
export const disableSchedulerTimeoutInWorkLoop = false;
5959
export const enableTransitionEntanglement = false;
60+
export const enableNonInterruptingNormalPri = false;
6061
export const enableDiscreteEventMicroTasks = false;
6162
export const enableNativeEventPriorityInference = false;
6263

packages/shared/forks/ReactFeatureFlags.test-renderer.native.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ export const enableUseRefAccessWarning = false;
5757
export const enableRecursiveCommitTraversal = false;
5858
export const disableSchedulerTimeoutInWorkLoop = false;
5959
export const enableTransitionEntanglement = false;
60+
export const enableNonInterruptingNormalPri = false;
6061
export const enableDiscreteEventMicroTasks = false;
6162
export const enableNativeEventPriorityInference = false;
6263

packages/shared/forks/ReactFeatureFlags.test-renderer.www.js

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ export const enableUseRefAccessWarning = false;
5757
export const enableRecursiveCommitTraversal = false;
5858
export const disableSchedulerTimeoutInWorkLoop = false;
5959
export const enableTransitionEntanglement = false;
60+
export const enableNonInterruptingNormalPri = false;
6061
export const enableDiscreteEventMicroTasks = false;
6162
export const enableNativeEventPriorityInference = false;
6263

0 commit comments

Comments
 (0)