Skip to content

Commit d1845ad

Browse files
acdliterickhanlonii
acdlite
and
rickhanlonii
authored
Default updates should not interrupt transitions (#20771)
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 d1845ad

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)