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 <rickhanlonii@gmail.com>
Andrew Clark committed
Feb 10, 2021 at 02:00 UTC
d1845ad0ff6c69ea377d710d4053d9bd65e0d0ff
14 files changed
+239
-4
packages/react-reconciler/src/ReactFiberLane.new.js
+11
-1
@@ -39,6 +39,7 @@ import invariant from 'shared/invariant';
39
import {
40
enableCache,
41
enableTransitionEntanglement,
42
+ enableNonInterruptingNormalPri,
43
} from 'shared/ReactFeatureFlags';
44
45
import {
@@ -327,7 +328,16 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
328
) {
329
getHighestPriorityLanes(wipLanes);
330
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.
341
return wipLanes;
342
} else {
343
return_highestLanePriority = nextLanePriority;
packages/react-reconciler/src/ReactFiberLane.old.js
+11
-1
@@ -39,6 +39,7 @@ import invariant from 'shared/invariant';
39
import {
40
enableCache,
41
enableTransitionEntanglement,
42
+ enableNonInterruptingNormalPri,
43
} from 'shared/ReactFeatureFlags';
44
45
import {
@@ -327,7 +328,16 @@ export function getNextLanes(root: FiberRoot, wipLanes: Lanes): Lanes {
328
) {
329
getHighestPriorityLanes(wipLanes);
330
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.
341
return wipLanes;
342
} else {
343
return_highestLanePriority = nextLanePriority;
packages/react-reconciler/src/__tests__/ReactIncrementalErrorHandling-test.internal.js
+4
-2
@@ -1849,10 +1849,12 @@ describe('ReactIncrementalErrorHandling', () => {
1849
// the queue.
1850
expect(Scheduler).toFlushAndYieldThrough(['Everything is fine.']);
1851
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.
1853
// The root should capture this error. But since there's still a pending
1854
// update on the root, the error should be suppressed.
1855
- setShouldThrow(true);
1855
+ ReactNoop.discreteUpdates(() => {
1856
+ setShouldThrow(true);
1857
+ });
1858
});
1859
// Should render the final state without throwing the error.
1860
expect(Scheduler).toHaveYielded(['Everything is fine.']);
packages/react-reconciler/src/__tests__/ReactTransition-test.js
+202
@@ -15,6 +15,7 @@ let ReactNoop;
15
let Scheduler;
16
let Suspense;
17
let useState;
18
+let useLayoutEffect;
19
let useTransition;
20
let startTransition;
21
let act;
@@ -30,6 +31,7 @@ describe('ReactTransition', () => {
31
ReactNoop = require('react-noop-renderer');
32
Scheduler = require('scheduler');
33
useState = React.useState;
34
+ useLayoutEffect = React.useLayoutEffect;
35
useTransition = React.unstable_useTransition;
36
Suspense = React.Suspense;
37
startTransition = React.unstable_startTransition;
@@ -773,4 +775,204 @@ describe('ReactTransition', () => {
775
});
776
},
777
);
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
+ });
978
});
packages/shared/ReactFeatureFlags.js
+2
@@ -150,6 +150,8 @@ export const disableSchedulerTimeoutInWorkLoop = false;
150
// Experiment to simplify/improve how transitions are scheduled
151
export const enableTransitionEntanglement = false;
152
153
+export const enableNonInterruptingNormalPri = false;
154
+
155
export const enableDiscreteEventMicroTasks = false;
156
157
export const enableNativeEventPriorityInference = false;
packages/shared/forks/ReactFeatureFlags.native-fb.js
+1
@@ -58,6 +58,7 @@ export const enableUseRefAccessWarning = false;
58
export const enableRecursiveCommitTraversal = false;
59
export const disableSchedulerTimeoutInWorkLoop = false;
60
export const enableTransitionEntanglement = false;
61
+export const enableNonInterruptingNormalPri = false;
62
export const enableDiscreteEventMicroTasks = false;
63
export const enableNativeEventPriorityInference = false;
64
packages/shared/forks/ReactFeatureFlags.native-oss.js
+1
@@ -57,6 +57,7 @@ export const enableUseRefAccessWarning = false;
57
export const enableRecursiveCommitTraversal = false;
58
export const disableSchedulerTimeoutInWorkLoop = false;
59
export const enableTransitionEntanglement = false;
60
+export const enableNonInterruptingNormalPri = false;
61
export const enableDiscreteEventMicroTasks = false;
62
export const enableNativeEventPriorityInference = false;
63
packages/shared/forks/ReactFeatureFlags.test-renderer.js
+1
@@ -57,6 +57,7 @@ export const enableUseRefAccessWarning = false;
57
export const enableRecursiveCommitTraversal = false;
58
export const disableSchedulerTimeoutInWorkLoop = false;
59
export const enableTransitionEntanglement = false;
60
+export const enableNonInterruptingNormalPri = false;
61
export const enableDiscreteEventMicroTasks = false;
62
export const enableNativeEventPriorityInference = false;
63
packages/shared/forks/ReactFeatureFlags.test-renderer.native.js
+1
@@ -57,6 +57,7 @@ export const enableUseRefAccessWarning = false;
57
export const enableRecursiveCommitTraversal = false;
58
export const disableSchedulerTimeoutInWorkLoop = false;
59
export const enableTransitionEntanglement = false;
60
+export const enableNonInterruptingNormalPri = false;
61
export const enableDiscreteEventMicroTasks = false;
62
export const enableNativeEventPriorityInference = false;
63
packages/shared/forks/ReactFeatureFlags.test-renderer.www.js
+1
@@ -57,6 +57,7 @@ export const enableUseRefAccessWarning = false;
57
export const enableRecursiveCommitTraversal = false;
58
export const disableSchedulerTimeoutInWorkLoop = false;
59
export const enableTransitionEntanglement = false;
60
+export const enableNonInterruptingNormalPri = false;
61
export const enableDiscreteEventMicroTasks = false;
62
export const enableNativeEventPriorityInference = false;
63
packages/shared/forks/ReactFeatureFlags.testing.js
+1
@@ -57,6 +57,7 @@ export const enableUseRefAccessWarning = false;
57
export const enableRecursiveCommitTraversal = false;
58
export const disableSchedulerTimeoutInWorkLoop = false;
59
export const enableTransitionEntanglement = false;
60
+export const enableNonInterruptingNormalPri = false;
61
export const enableDiscreteEventMicroTasks = false;
62
export const enableNativeEventPriorityInference = false;
63
packages/shared/forks/ReactFeatureFlags.testing.www.js
+1
@@ -57,6 +57,7 @@ export const enableUseRefAccessWarning = false;
57
export const enableRecursiveCommitTraversal = false;
58
export const disableSchedulerTimeoutInWorkLoop = false;
59
export const enableTransitionEntanglement = false;
60
+export const enableNonInterruptingNormalPri = false;
61
export const enableDiscreteEventMicroTasks = false;
62
export const enableNativeEventPriorityInference = false;
63
packages/shared/forks/ReactFeatureFlags.www-dynamic.js
+1
@@ -56,5 +56,6 @@ export const enableUseRefAccessWarning = __VARIANT__;
56
export const enableProfilerNestedUpdateScheduledHook = __VARIANT__;
57
export const disableSchedulerTimeoutInWorkLoop = __VARIANT__;
58
export const enableTransitionEntanglement = __VARIANT__;
59
+export const enableNonInterruptingNormalPri = __VARIANT__;
60
export const enableDiscreteEventMicroTasks = __VARIANT__;
61
export const enableNativeEventPriorityInference = __VARIANT__;
packages/shared/forks/ReactFeatureFlags.www.js
+1
@@ -32,6 +32,7 @@ export const {
32
disableNativeComponentFrames,
33
disableSchedulerTimeoutInWorkLoop,
34
enableTransitionEntanglement,
35
+ enableNonInterruptingNormalPri,
36
enableDiscreteEventMicroTasks,
37
enableNativeEventPriorityInference,
38
} = dynamicFeatureFlags;