@samitouri / QOS-React-2 / commits / bff6be8eb1

[Fizz] Track postpones in fallbacks (#27421)

This fixes so that you can postpone in a fallback. This postpones the parent boundary. I track the fallbacks in a separate replay node so that when we resume, we can replay the fallback itself and finish the fallback and then possibly later the content. By doing this we also ensure we don't complete the parent too early since now it has a render task on it. There is one case that this surfaces that isn't limited to prerender/resume but also render/hydrateRoot. I left todos in the tests for this. If you postpone in a fallback, and suspend in the content but eventually don't postpone in the content then we should be able to just skip postponing since the content rendered and we no longer need the fallback. This is a bit of a weird edge case though since fallbacks are supposed to be very minimal. This happens because in both cases the fallback starts rendering early as soon as the content suspends. This also ensures that the parent doesn't complete early by increasing the blocking tasks. Unfortunately, the fallback will irreversibly postpone its parent boundary as soon as it hits a postpone. When you suspend, the same thing happens but we typically deal with this by doing a "soft" abort on the fallback since we don't need it anymore which unblocks the parent boundary. We can't do that with postpone right now though since it's considered a terminal state. I think I'll just leave this as is for now since it's an edge case but it's an annoying exception in the model. Makes me feel I haven't quite nailed it just yet.

Sebastian Markbåge committed Sep 25, 2023 at 19:02 UTC bff6be8eb1d77980c13f3e01be63cb813a377058
3 files changed +380 -53
packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
+143
@@ -6261,6 +6261,59 @@ describe('ReactDOMFizzServer', () => {
6261 expect(fatalErrors).toEqual(['testing postpone']);
6262 });
6263
6264 + // @gate enablePostpone
6265 + it('can postpone in a fallback', async () => {
6266 + function Postponed({isClient}) {
6267 + if (!isClient) {
6268 + React.unstable_postpone('testing postpone');
6269 + }
6270 + return 'loading...';
6271 + }
6272 +
6273 + const lazyText = React.lazy(async () => {
6274 + await 0; // causes the fallback to start work
6275 + return {default: 'Hello'};
6276 + });
6277 +
6278 + function App({isClient}) {
6279 + return (
6280 + <div>
6281 + <Suspense fallback="Outer">
6282 + <Suspense fallback={<Postponed isClient={isClient} />}>
6283 + {lazyText}
6284 + </Suspense>
6285 + </Suspense>
6286 + </div>
6287 + );
6288 + }
6289 +
6290 + const errors = [];
6291 +
6292 + await act(() => {
6293 + const {pipe} = renderToPipeableStream(<App isClient={false} />, {
6294 + onError(error) {
6295 + errors.push(error.message);
6296 + },
6297 + });
6298 + pipe(writable);
6299 + });
6300 +
6301 + // TODO: This should actually be fully resolved because the value could eventually
6302 + // resolve on the server even though the fallback couldn't so we should have been
6303 + // able to render it.
6304 + expect(getVisibleChildren(container)).toEqual(<div>Outer</div>);
6305 +
6306 + ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
6307 + onRecoverableError(error) {
6308 + errors.push(error.message);
6309 + },
6310 + });
6311 + await waitForAll([]);
6312 + // Postponing should not be logged as a recoverable error since it's intentional.
6313 + expect(errors).toEqual([]);
6314 + expect(getVisibleChildren(container)).toEqual(<div>Hello</div>);
6315 + });
6316 +
6317 it(
6318 'a transition that flows into a dehydrated boundary should not suspend ' +
6319 'if the boundary is showing a fallback',
@@ -6830,4 +6883,94 @@ describe('ReactDOMFizzServer', () => {
6883 ],
6884 );
6885 });
6886 +
6887 + // @gate enablePostpone
6888 + it('can postpone in fallback', async () => {
6889 + let prerendering = true;
6890 + function Postpone() {
6891 + if (prerendering) {
6892 + React.unstable_postpone();
6893 + }
6894 + return 'Hello';
6895 + }
6896 +
6897 + let resolve;
6898 + const promise = new Promise(r => (resolve = r));
6899 +
6900 + function PostponeAndDelay() {
6901 + if (prerendering) {
6902 + React.unstable_postpone();
6903 + }
6904 + return React.use(promise);
6905 + }
6906 +
6907 + const Lazy = React.lazy(async () => {
6908 + await 0;
6909 + return {default: Postpone};
6910 + });
6911 +
6912 + function App() {
6913 + return (
6914 + <div>
6915 + <Suspense fallback="Outer">
6916 + <Suspense fallback={<Postpone />}>
6917 + <PostponeAndDelay /> World
6918 + </Suspense>
6919 + <Suspense fallback={<Postpone />}>
6920 + <Lazy />
6921 + </Suspense>
6922 + </Suspense>
6923 + </div>
6924 + );
6925 + }
6926 +
6927 + const prerendered = await ReactDOMFizzStatic.prerenderToNodeStream(<App />);
6928 + expect(prerendered.postponed).not.toBe(null);
6929 +
6930 + prerendering = false;
6931 +
6932 + // Create a separate stream so it doesn't close the writable. I.e. simple concat.
6933 + const preludeWritable = new Stream.PassThrough();
6934 + preludeWritable.setEncoding('utf8');
6935 + preludeWritable.on('data', chunk => {
6936 + writable.write(chunk);
6937 + });
6938 +
6939 + await act(() => {
6940 + prerendered.prelude.pipe(preludeWritable);
6941 + });
6942 +
6943 + const resumed = await ReactDOMFizzServer.resumeToPipeableStream(
6944 + <App />,
6945 + JSON.parse(JSON.stringify(prerendered.postponed)),
6946 + );
6947 +
6948 + expect(getVisibleChildren(container)).toEqual(<div>Outer</div>);
6949 +
6950 + // Read what we've completed so far
6951 + await act(() => {
6952 + resumed.pipe(writable);
6953 + });
6954 +
6955 + // Should have now resolved the postponed loading state, but not the promise
6956 + expect(getVisibleChildren(container)).toEqual(
6957 + <div>
6958 + {'Hello'}
6959 + {'Hello'}
6960 + </div>,
6961 + );
6962 +
6963 + // Resolve the final promise
6964 + await act(() => {
6965 + resolve('Hi');
6966 + });
6967 +
6968 + expect(getVisibleChildren(container)).toEqual(
6969 + <div>
6970 + {'Hi'}
6971 + {' World'}
6972 + {'Hello'}
6973 + </div>,
6974 + );
6975 + });
6976 });
packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js
+96 -2
@@ -870,8 +870,6 @@ describe('ReactDOMFizzStaticBrowser', () => {
870
871 prerendering = false;
872
873 - console.log(JSON.stringify(prerendered.postponed, null, 2));
874 -
873 const resumed = await ReactDOMFizzServer.resume(
874 <App />,
875 JSON.parse(JSON.stringify(prerendered.postponed)),
@@ -887,4 +885,100 @@ describe('ReactDOMFizzStaticBrowser', () => {
885 <div>{['Hello', 'Hello', 'Hello']}</div>,
886 );
887 });
888 +
889 + // @gate enablePostpone
890 + it('can postpone in fallback', async () => {
891 + let prerendering = true;
892 + function Postpone() {
893 + if (prerendering) {
894 + React.unstable_postpone();
895 + }
896 + return 'Hello';
897 + }
898 +
899 + const Lazy = React.lazy(async () => {
900 + await 0;
901 + return {default: Postpone};
902 + });
903 +
904 + function App() {
905 + return (
906 + <div>
907 + <Suspense fallback="Outer">
908 + <Suspense fallback={<Postpone />}>
909 + <Postpone /> World
910 + </Suspense>
911 + <Suspense fallback={<Postpone />}>
912 + <Lazy />
913 + </Suspense>
914 + </Suspense>
915 + </div>
916 + );
917 + }
918 +
919 + const prerendered = await ReactDOMFizzStatic.prerender(<App />);
920 + expect(prerendered.postponed).not.toBe(null);
921 +
922 + prerendering = false;
923 +
924 + const resumed = await ReactDOMFizzServer.resume(
925 + <App />,
926 + JSON.parse(JSON.stringify(prerendered.postponed)),
927 + );
928 +
929 + await readIntoContainer(prerendered.prelude);
930 +
931 + expect(getVisibleChildren(container)).toEqual(<div>Outer</div>);
932 +
933 + await readIntoContainer(resumed);
934 +
935 + expect(getVisibleChildren(container)).toEqual(
936 + <div>
937 + {'Hello'}
938 + {' World'}
939 + {'Hello'}
940 + </div>,
941 + );
942 + });
943 +
944 + // @gate enablePostpone
945 + it('can postpone in fallback without postponing the tree', async () => {
946 + function Postpone() {
947 + React.unstable_postpone();
948 + }
949 +
950 + const lazyText = React.lazy(async () => {
951 + await 0; // causes the fallback to start work
952 + return {default: 'Hello'};
953 + });
954 +
955 + function App() {
956 + return (
957 + <div>
958 + <Suspense fallback="Outer">
959 + <Suspense fallback={<Postpone />}>{lazyText}</Suspense>
960 + </Suspense>
961 + </div>
962 + );
963 + }
964 +
965 + const prerendered = await ReactDOMFizzStatic.prerender(<App />);
966 + // TODO: This should actually be null because we should've been able to fully
967 + // resolve the render on the server eventually, even though the fallback postponed.
968 + // So we should not need to resume.
969 + expect(prerendered.postponed).not.toBe(null);
970 +
971 + await readIntoContainer(prerendered.prelude);
972 +
973 + expect(getVisibleChildren(container)).toEqual(<div>Outer</div>);
974 +
975 + const resumed = await ReactDOMFizzServer.resume(
976 + <App />,
977 + JSON.parse(JSON.stringify(prerendered.postponed)),
978 + );
979 +
980 + await readIntoContainer(resumed);
981 +
982 + expect(getVisibleChildren(container)).toEqual(<div>Hello</div>);
983 + });
984 });
packages/react-server/src/ReactFizzServer.js
+141 -51
@@ -170,8 +170,9 @@ type ResumeSlots =
170 type ReplaySuspenseBoundary = [
171 string | null /* name */,
172 string | number /* key */,
173 - Array<ReplayNode> /* keyed children */,
174 - ResumeSlots /* resumable slots */,
173 + Array<ReplayNode> /* content keyed children */,
174 + ResumeSlots /* content resumable slots */,
175 + null | ReplayNode /* fallback content */,
176 number /* rootSegmentID */,
177 ];
178
@@ -208,7 +209,8 @@ type SuspenseBoundary = {
209 byteSize: number, // used to determine whether to inline children boundaries.
210 fallbackAbortableTasks: Set<Task>, // used to cancel task on the fallback if the boundary completes or gets canceled.
211 resources: BoundaryResources,
211 - keyPath: Root | KeyNode,
212 + trackedContentKeyPath: null | KeyNode, // used to track the path for replay nodes
213 + trackedFallbackNode: null | ReplayNode, // used to track the fallback for replay nodes
214 };
215
216 type RenderTask = {
@@ -549,7 +551,6 @@ function pingTask(request: Request, task: Task): void {
551 function createSuspenseBoundary(
552 request: Request,
553 fallbackAbortableTasks: Set<Task>,
552 - keyPath: Root | KeyNode,
554 ): SuspenseBoundary {
555 return {
556 status: PENDING,
@@ -561,7 +562,8 @@ function createSuspenseBoundary(
562 fallbackAbortableTasks,
563 errorDigest: null,
564 resources: createBoundaryResources(),
564 - keyPath,
565 + trackedContentKeyPath: null,
566 + trackedFallbackNode: null,
567 };
568 }
569
@@ -823,11 +825,10 @@ function renderSuspenseBoundary(
825 const content: ReactNodeList = props.children;
826
827 const fallbackAbortSet: Set<Task> = new Set();
826 - const newBoundary = createSuspenseBoundary(
827 - request,
828 - fallbackAbortSet,
829 - keyPath,
830 - );
828 + const newBoundary = createSuspenseBoundary(request, fallbackAbortSet);
829 + if (request.trackedPostpones !== null) {
830 + newBoundary.trackedContentKeyPath = keyPath;
831 + }
832 const insertionIndex = parentSegment.chunks.length;
833 // The children of the boundary segment is actually the fallback.
834 const boundarySegment = createPendingSegment(
@@ -930,6 +931,28 @@ function renderSuspenseBoundary(
931 task.keyPath = prevKeyPath;
932 }
933
934 + const fallbackKeyPath = [keyPath[0], 'Suspense Fallback', keyPath[2]];
935 + const trackedPostpones = request.trackedPostpones;
936 + if (trackedPostpones !== null) {
937 + // We create a detached replay node to track any postpones inside the fallback.
938 + const fallbackReplayNode: ReplayNode = [
939 + fallbackKeyPath[1],
940 + fallbackKeyPath[2],
941 + ([]: Array<ReplayNode>),
942 + null,
943 + ];
944 + trackedPostpones.workingMap.set(fallbackKeyPath, fallbackReplayNode);
945 + if (newBoundary.status === POSTPONED) {
946 + // This must exist now.
947 + const boundaryReplayNode: ReplaySuspenseBoundary =
948 + (trackedPostpones.workingMap.get(keyPath): any);
949 + boundaryReplayNode[4] = fallbackReplayNode;
950 + } else {
951 + // We might not inject it into the postponed tree, unless the content actually
952 + // postpones too. We need to keep track of it until that happpens.
953 + newBoundary.trackedFallbackNode = fallbackReplayNode;
954 + }
955 + }
956 // We create suspended task for the fallback because we don't want to actually work
957 // on it yet in case we finish the main content, so we queue for later.
958 const suspendedFallbackTask = createRenderTask(
@@ -940,8 +963,7 @@ function renderSuspenseBoundary(
963 parentBoundary,
964 boundarySegment,
965 fallbackAbortSet,
943 - // TODO: Should distinguish key path of fallback and primary tasks
944 - keyPath,
966 + fallbackKeyPath,
967 task.formatContext,
968 task.legacyContext,
969 task.context,
@@ -965,6 +987,8 @@ function replaySuspenseBoundary(
987 id: number,
988 childNodes: Array<ReplayNode>,
989 childSlots: ResumeSlots,
990 + fallbackNodes: Array<ReplayNode>,
991 + fallbackSlots: ResumeSlots,
992 ): void {
993 pushBuiltInComponentStackInDEV(task, 'Suspense');
994
@@ -974,13 +998,10 @@ function replaySuspenseBoundary(
998 const parentBoundary = task.blockedBoundary;
999
1000 const content: ReactNodeList = props.children;
1001 + const fallback: ReactNodeList = props.fallback;
1002
1003 const fallbackAbortSet: Set<Task> = new Set();
979 - const resumedBoundary = createSuspenseBoundary(
980 - request,
981 - fallbackAbortSet,
982 - task.keyPath,
983 - );
1004 + const resumedBoundary = createSuspenseBoundary(request, fallbackAbortSet);
1005 resumedBoundary.parentFlushed = true;
1006 // We restore the same id of this boundary as was used during prerender.
1007 resumedBoundary.rootSegmentID = id;
@@ -1003,13 +1024,6 @@ function replaySuspenseBoundary(
1024 } else {
1025 renderNode(request, task, content, -1);
1026 }
1006 - if (
1007 - resumedBoundary.pendingTasks === 0 &&
1008 - resumedBoundary.status === PENDING
1009 - ) {
1010 - resumedBoundary.status = COMPLETED;
1011 - request.completedBoundaries.push(resumedBoundary);
1012 - }
1027 if (task.replay.pendingTasks === 1 && task.replay.nodes.length > 0) {
1028 throw new Error(
1029 "Couldn't find all resumable slots by key/index during replaying. " +
@@ -1017,6 +1031,18 @@ function replaySuspenseBoundary(
1031 );
1032 }
1033 task.replay.pendingTasks--;
1034 + if (
1035 + resumedBoundary.pendingTasks === 0 &&
1036 + resumedBoundary.status === PENDING
1037 + ) {
1038 + resumedBoundary.status = COMPLETED;
1039 + request.completedBoundaries.push(resumedBoundary);
1040 + // This must have been the last segment we were waiting on. This boundary is now complete.
1041 + // Therefore we won't need the fallback. We early return so that we don't have to create
1042 + // the fallback.
1043 + popComponentStackInDEV(task);
1044 + return;
1045 + }
1046 } catch (error) {
1047 resumedBoundary.status = CLIENT_RENDERED;
1048 let errorDigest;
@@ -1057,6 +1083,66 @@ function replaySuspenseBoundary(
1083 task.replay = previousReplaySet;
1084 task.keyPath = prevKeyPath;
1085 }
1086 +
1087 + const fallbackKeyPath = [keyPath[0], 'Suspense Fallback', keyPath[2]];
1088 +
1089 + let suspendedFallbackTask;
1090 + // We create suspended task for the fallback because we don't want to actually work
1091 + // on it yet in case we finish the main content, so we queue for later.
1092 + if (typeof fallbackSlots === 'number') {
1093 + // Resuming directly in the fallback.
1094 + const resumedSegment = createPendingSegment(
1095 + request,
1096 + 0,
1097 + null,
1098 + task.formatContext,
1099 + false,
1100 + false,
1101 + );
1102 + resumedSegment.id = fallbackSlots;
1103 + resumedSegment.parentFlushed = true;
1104 + suspendedFallbackTask = createRenderTask(
1105 + request,
1106 + null,
1107 + fallback,
1108 + -1,
1109 + parentBoundary,
1110 + resumedSegment,
1111 + fallbackAbortSet,
1112 + fallbackKeyPath,
1113 + task.formatContext,
1114 + task.legacyContext,
1115 + task.context,
1116 + task.treeContext,
1117 + );
1118 + } else {
1119 + const fallbackReplay = {
1120 + nodes: fallbackNodes,
1121 + slots: fallbackSlots,
1122 + pendingTasks: 0,
1123 + };
1124 + suspendedFallbackTask = createReplayTask(
1125 + request,
1126 + null,
1127 + fallbackReplay,
1128 + fallback,
1129 + -1,
1130 + parentBoundary,
1131 + fallbackAbortSet,
1132 + fallbackKeyPath,
1133 + task.formatContext,
1134 + task.legacyContext,
1135 + task.context,
1136 + task.treeContext,
1137 + );
1138 + }
1139 + if (__DEV__) {
1140 + suspendedFallbackTask.componentStack = task.componentStack;
1141 + }
1142 + // TODO: This should be queued at a separate lower priority queue so that we only work
1143 + // on preparing fallbacks if we don't have any more main content to task on.
1144 + request.pingedTasks.push(suspendedFallbackTask);
1145 +
1146 // TODO: Should this be in the finally?
1147 popComponentStackInDEV(task);
1148 }
@@ -2025,9 +2111,11 @@ function replayElement(
2111 task,
2112 keyPath,
2113 props,
2028 - node[4],
2114 + node[5],
2115 node[2],
2116 node[3],
2117 + node[4] === null ? [] : node[4][2],
2118 + node[4] === null ? null : node[4][3],
2119 );
2120 }
2121 // We finished rendering this node, so now we can consume this
@@ -2467,13 +2555,15 @@ function trackPostpone(
2555 // it before flushing and we know that we can't inline it.
2556 boundary.rootSegmentID = request.nextSegmentId++;
2557
2470 - const boundaryKeyPath = boundary.keyPath;
2558 + const boundaryKeyPath = boundary.trackedContentKeyPath;
2559 if (boundaryKeyPath === null) {
2560 throw new Error(
2561 'It should not be possible to postpone at the root. This is a bug in React.',
2562 );
2563 }
2564
2565 + const fallbackReplayNode = boundary.trackedFallbackNode;
2566 +
2567 const children: Array<ReplayNode> = [];
2568 if (boundaryKeyPath === keyPath && task.childIndex === -1) {
2569 // Since we postponed directly in the Suspense boundary we can't have written anything
@@ -2485,8 +2575,10 @@ function trackPostpone(
2575 boundaryKeyPath[2],
2576 children,
2577 boundary.rootSegmentID,
2578 + fallbackReplayNode,
2579 boundary.rootSegmentID,
2580 ];
2581 + trackedPostpones.workingMap.set(boundaryKeyPath, boundaryNode);
2582 addToReplayParent(boundaryNode, boundaryKeyPath[0], trackedPostpones);
2583 return;
2584 } else {
@@ -2498,14 +2590,16 @@ function trackPostpone(
2590 boundaryKeyPath[2],
2591 children,
2592 null,
2593 + fallbackReplayNode,
2594 boundary.rootSegmentID,
2595 ];
2596 trackedPostpones.workingMap.set(boundaryKeyPath, boundaryNode);
2597 addToReplayParent(boundaryNode, boundaryKeyPath[0], trackedPostpones);
2598 } else {
2599 // Upgrade to ReplaySuspenseBoundary.
2507 - ((boundaryNode: any): ReplaySuspenseBoundary)[4] =
2508 - boundary.rootSegmentID;
2600 + const suspenseBoundary: ReplaySuspenseBoundary = (boundaryNode: any);
2601 + suspenseBoundary[4] = fallbackReplayNode;
2602 + suspenseBoundary[5] = boundary.rootSegmentID;
2603 }
2604 // Fall through to add the child node.
2605 }
@@ -2528,13 +2622,19 @@ function trackPostpone(
2622 if (keyPath === null) {
2623 trackedPostpones.rootSlots = segment.id;
2624 } else {
2531 - const resumableElement: ReplayNode = [
2532 - keyPath[1],
2533 - keyPath[2],
2534 - ([]: Array<ReplayNode>),
2535 - segment.id,
2536 - ];
2537 - addToReplayParent(resumableElement, keyPath[0], trackedPostpones);
2625 + const workingMap = trackedPostpones.workingMap;
2626 + let resumableNode = workingMap.get(keyPath);
2627 + if (resumableNode === undefined) {
2628 + resumableNode = [
2629 + keyPath[1],
2630 + keyPath[2],
2631 + ([]: Array<ReplayNode>),
2632 + segment.id,
2633 + ];
2634 + addToReplayParent(resumableNode, keyPath[0], trackedPostpones);
2635 + } else {
2636 + resumableNode[3] = segment.id;
2637 + }
2638 }
2639 } else {
2640 let slots;
@@ -2963,11 +3063,7 @@ function abortRemainingSuspenseBoundary(
3063 error: mixed,
3064 errorDigest: ?string,
3065 ): void {
2966 - const resumedBoundary = createSuspenseBoundary(
2967 - request,
2968 - new Set(),
2969 - null, // The keyPath doesn't matter at this point so we don't bother rebuilding it.
2970 - );
3066 + const resumedBoundary = createSuspenseBoundary(request, new Set());
3067 resumedBoundary.parentFlushed = true;
3068 // We restore the same id of this boundary as was used during prerender.
3069 resumedBoundary.rootSegmentID = rootSegmentID;
@@ -3017,7 +3113,7 @@ function abortRemainingReplayNodes(
3113 );
3114 } else {
3115 const boundaryNode: ReplaySuspenseBoundary = node;
3020 - const rootSegmentID = boundaryNode[4];
3116 + const rootSegmentID = boundaryNode[5];
3117 abortRemainingSuspenseBoundary(
3118 request,
3119 rootSegmentID,
@@ -3835,9 +3931,7 @@ function flushCompletedQueues(
3931 destination,
3932 request.resumableState,
3933 request.renderState,
3838 - request.allPendingTasks === 0 &&
3839 - (request.trackedPostpones === null ||
3840 - request.trackedPostpones.workingMap.size === 0),
3934 + request.allPendingTasks === 0 && request.trackedPostpones === null,
3935 );
3936 }
3937
@@ -3932,13 +4026,7 @@ function flushCompletedQueues(
4026 if (enableFloat) {
4027 // We write the trailing tags but only if don't have any data to resume.
4028 // If we need to resume we'll write the postamble in the resume instead.
3935 - if (
3936 - !enablePostpone ||
3937 - request.trackedPostpones === null ||
3938 - // We check the working map instead of the root because the root could've
3939 - // been mutated at this point if it was passed straight through to resume().
3940 - request.trackedPostpones.workingMap.size === 0
3941 - ) {
4029 + if (!enablePostpone || request.trackedPostpones === null) {
4030 writePostamble(destination, request.resumableState);
4031 }
4032 }
@@ -4090,6 +4178,8 @@ export function getPostponedState(request: Request): null | PostponedState {
4178 (trackedPostpones.rootNodes.length === 0 &&
4179 trackedPostpones.rootSlots === null)
4180 ) {
4181 + // Reset. Let the flushing behave as if we completed the whole document.
4182 + request.trackedPostpones = null;
4183 return null;
4184 }
4185 return {