Skip to content

Commit

Permalink
Always insert a dummy node with an ID into fallbacks
Browse files Browse the repository at this point in the history
  • Loading branch information
sebmarkbage committed Apr 14, 2021
1 parent 266c26a commit 3e88e81
Show file tree
Hide file tree
Showing 2 changed files with 91 additions and 4 deletions.
78 changes: 76 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ describe('ReactDOMFizzServer', () => {
if (
node.tagName !== 'SCRIPT' &&
node.tagName !== 'TEMPLATE' &&
node.tagName !== 'template' &&
!node.hasAttribute('hidden') &&
!node.hasAttribute('aria-hidden')
) {
Expand Down Expand Up @@ -153,7 +154,6 @@ describe('ReactDOMFizzServer', () => {
}
}

/*
function rejectText(text, error) {
const record = textCache.get(text);
if (record === undefined) {
Expand All @@ -169,7 +169,6 @@ describe('ReactDOMFizzServer', () => {
thenable.pings.forEach(t => t());
}
}
*/

function readText(text) {
const record = textCache.get(text);
Expand Down Expand Up @@ -800,4 +799,79 @@ describe('ReactDOMFizzServer', () => {
</div>,
);
});

it('client renders a boundary if it errors before finishing the fallback', async () => {
function App({isClient}) {
return (
<Suspense fallback="Loading root...">
<div>
<Suspense fallback={<AsyncText text="Loading..." />}>
<h1>
{isClient ? <Text text="Hello" /> : <AsyncText text="Hello" />}
</h1>
</Suspense>
</div>
</Suspense>
);
}

const loggedErrors = [];
let controls;
await act(async () => {
controls = ReactDOMFizzServer.pipeToNodeWritable(
<App isClient={false} />,
writable,
{
onError(x) {
loggedErrors.push(x);
},
},
);
controls.startWriting();
});

// We're still showing a fallback.

// Attempt to hydrate the content.
const root = ReactDOM.unstable_createRoot(container, {hydrate: true});
root.render(<App isClient={true} />);
Scheduler.unstable_flushAll();

// We're still loading because we're waiting for the server to stream more content.
expect(getVisibleChildren(container)).toEqual('Loading root...');

expect(loggedErrors).toEqual([]);

const theError = new Error('Test');
// Error the content, but we don't have a fallback yet.
await act(async () => {
rejectText('Hello', theError);
});

expect(loggedErrors).toEqual([theError]);

// We still can't render it on the client because we haven't unblocked the parent.
Scheduler.unstable_flushAll();
expect(getVisibleChildren(container)).toEqual('Loading root...');

// Unblock the loading state
await act(async () => {
resolveText('Loading...');
});

// Now we're able to show the inner boundary.
expect(getVisibleChildren(container)).toEqual(<div>Loading...</div>);

// That will let us client render it instead.
Scheduler.unstable_flushAll();

// The client rendered HTML is now in place.
expect(getVisibleChildren(container)).toEqual(
<div>
<h1>Hello</h1>
</div>,
);

expect(loggedErrors).toEqual([theError]);
});
});
17 changes: 15 additions & 2 deletions packages/react-server/src/ReactFizzServer.js
Original file line number Diff line number Diff line change
Expand Up @@ -419,17 +419,30 @@ function renderSuspenseBoundary(
task.blockedSegment = parentSegment;
}

// This injects an extra segment just to contain an empty tag with an ID.
// This means that we're not actually using the assignID anywhere.
// TODO: Rethink the assignID approach.
pushEmpty(boundarySegment.chunks, request.responseState, newBoundary.id);
const innerSegment = createPendingSegment(
request,
boundarySegment.chunks.length,
null,
boundarySegment.formatContext,
);
boundarySegment.status = COMPLETED;
boundarySegment.children.push(innerSegment);

// We create suspended task for the fallback because we don't want to actually work
// on it yet in case we finish the main content, so we queue for later.
const suspendedFallbackTask = createTask(
request,
fallback,
parentBoundary,
boundarySegment,
innerSegment,
fallbackAbortSet,
task.legacyContext,
task.context,
newBoundary.id, // This is the ID we want to give this fallback so we can replace it later.
null,
);
// TODO: This should be queued at a separate lower priority queue so that we only work
// on preparing fallbacks if we don't have any more main content to task on.
Expand Down

0 comments on commit 3e88e81

Please sign in to comment.