Always insert a dummy node with an ID into fallbacks (#21272)
Sebastian Markbåge committed
Apr 14, 2021 at 18:39 UTC
81ef5395358b6e0f44a02df618901eaef830fe78
2 files changed
+91
-4
packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
+76
-2
@@ -103,6 +103,7 @@ describe('ReactDOMFizzServer', () => {
103
if (
104
node.tagName !== 'SCRIPT' &&
105
node.tagName !== 'TEMPLATE' &&
106
+ node.tagName !== 'template' &&
107
!node.hasAttribute('hidden') &&
108
!node.hasAttribute('aria-hidden')
109
) {
@@ -153,7 +154,6 @@ describe('ReactDOMFizzServer', () => {
154
}
155
}
156
156
- /*
157
function rejectText(text, error) {
158
const record = textCache.get(text);
159
if (record === undefined) {
@@ -169,7 +169,6 @@ describe('ReactDOMFizzServer', () => {
169
thenable.pings.forEach(t => t());
170
}
171
}
172
- */
172
173
function readText(text) {
174
const record = textCache.get(text);
@@ -800,4 +799,79 @@ describe('ReactDOMFizzServer', () => {
799
</div>,
800
);
801
});
802
+
803
+ it('client renders a boundary if it errors before finishing the fallback', async () => {
804
+ function App({isClient}) {
805
+ return (
806
+ <Suspense fallback="Loading root...">
807
+ <div>
808
+ <Suspense fallback={<AsyncText text="Loading..." />}>
809
+ <h1>
810
+ {isClient ? <Text text="Hello" /> : <AsyncText text="Hello" />}
811
+ </h1>
812
+ </Suspense>
813
+ </div>
814
+ </Suspense>
815
+ );
816
+ }
817
+
818
+ const loggedErrors = [];
819
+ let controls;
820
+ await act(async () => {
821
+ controls = ReactDOMFizzServer.pipeToNodeWritable(
822
+ <App isClient={false} />,
823
+ writable,
824
+ {
825
+ onError(x) {
826
+ loggedErrors.push(x);
827
+ },
828
+ },
829
+ );
830
+ controls.startWriting();
831
+ });
832
+
833
+ // We're still showing a fallback.
834
+
835
+ // Attempt to hydrate the content.
836
+ const root = ReactDOM.unstable_createRoot(container, {hydrate: true});
837
+ root.render(<App isClient={true} />);
838
+ Scheduler.unstable_flushAll();
839
+
840
+ // We're still loading because we're waiting for the server to stream more content.
841
+ expect(getVisibleChildren(container)).toEqual('Loading root...');
842
+
843
+ expect(loggedErrors).toEqual([]);
844
+
845
+ const theError = new Error('Test');
846
+ // Error the content, but we don't have a fallback yet.
847
+ await act(async () => {
848
+ rejectText('Hello', theError);
849
+ });
850
+
851
+ expect(loggedErrors).toEqual([theError]);
852
+
853
+ // We still can't render it on the client because we haven't unblocked the parent.
854
+ Scheduler.unstable_flushAll();
855
+ expect(getVisibleChildren(container)).toEqual('Loading root...');
856
+
857
+ // Unblock the loading state
858
+ await act(async () => {
859
+ resolveText('Loading...');
860
+ });
861
+
862
+ // Now we're able to show the inner boundary.
863
+ expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);
864
+
865
+ // That will let us client render it instead.
866
+ Scheduler.unstable_flushAll();
867
+
868
+ // The client rendered HTML is now in place.
869
+ expect(getVisibleChildren(container)).toEqual(
870
+ <div>
871
+ <h1>Hello</h1>
872
+ </div>,
873
+ );
874
+
875
+ expect(loggedErrors).toEqual([theError]);
876
+ });
877
});
packages/react-server/src/ReactFizzServer.js
+15
-2
@@ -419,17 +419,30 @@ function renderSuspenseBoundary(
419
task.blockedSegment = parentSegment;
420
}
421
422
+ // This injects an extra segment just to contain an empty tag with an ID.
423
+ // This means that we're not actually using the assignID anywhere.
424
+ // TODO: Rethink the assignID approach.
425
+ pushEmpty(boundarySegment.chunks, request.responseState, newBoundary.id);
426
+ const innerSegment = createPendingSegment(
427
+ request,
428
+ boundarySegment.chunks.length,
429
+ null,
430
+ boundarySegment.formatContext,
431
+ );
432
+ boundarySegment.status = COMPLETED;
433
+ boundarySegment.children.push(innerSegment);
434
+
435
// We create suspended task for the fallback because we don't want to actually work
436
// on it yet in case we finish the main content, so we queue for later.
437
const suspendedFallbackTask = createTask(
438
request,
439
fallback,
440
parentBoundary,
428
- boundarySegment,
441
+ innerSegment,
442
fallbackAbortSet,
443
task.legacyContext,
444
task.context,
432
- newBoundary.id, // This is the ID we want to give this fallback so we can replace it later.
445
+ null,
446
);
447
// TODO: This should be queued at a separate lower priority queue so that we only work
448
// on preparing fallbacks if we don't have any more main content to task on.