Skip to content

Commit

Permalink
[Fizz] Track postpones in fallbacks (facebook#27421)
Browse files Browse the repository at this point in the history
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.
  • Loading branch information
sebmarkbage authored and AndyPengc12 committed Apr 15, 2024
1 parent a51a9f1 commit 813e43f
Show file tree
Hide file tree
Showing 3 changed files with 380 additions and 53 deletions.
143 changes: 143 additions & 0 deletions packages/react-dom/src/__tests__/ReactDOMFizzServer-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -6261,6 +6261,59 @@ describe('ReactDOMFizzServer', () => {
expect(fatalErrors).toEqual(['testing postpone']);
});

// @gate enablePostpone
it('can postpone in a fallback', async () => {
function Postponed({isClient}) {
if (!isClient) {
React.unstable_postpone('testing postpone');
}
return 'loading...';
}

const lazyText = React.lazy(async () => {
await 0; // causes the fallback to start work
return {default: 'Hello'};
});

function App({isClient}) {
return (
<div>
<Suspense fallback="Outer">
<Suspense fallback={<Postponed isClient={isClient} />}>
{lazyText}
</Suspense>
</Suspense>
</div>
);
}

const errors = [];

await act(() => {
const {pipe} = renderToPipeableStream(<App isClient={false} />, {
onError(error) {
errors.push(error.message);
},
});
pipe(writable);
});

// TODO: This should actually be fully resolved because the value could eventually
// resolve on the server even though the fallback couldn't so we should have been
// able to render it.
expect(getVisibleChildren(container)).toEqual(<div>Outer</div>);

ReactDOMClient.hydrateRoot(container, <App isClient={true} />, {
onRecoverableError(error) {
errors.push(error.message);
},
});
await waitForAll([]);
// Postponing should not be logged as a recoverable error since it's intentional.
expect(errors).toEqual([]);
expect(getVisibleChildren(container)).toEqual(<div>Hello</div>);
});

it(
'a transition that flows into a dehydrated boundary should not suspend ' +
'if the boundary is showing a fallback',
Expand Down Expand Up @@ -6830,4 +6883,94 @@ describe('ReactDOMFizzServer', () => {
],
);
});

// @gate enablePostpone
it('can postpone in fallback', async () => {
let prerendering = true;
function Postpone() {
if (prerendering) {
React.unstable_postpone();
}
return 'Hello';
}

let resolve;
const promise = new Promise(r => (resolve = r));

function PostponeAndDelay() {
if (prerendering) {
React.unstable_postpone();
}
return React.use(promise);
}

const Lazy = React.lazy(async () => {
await 0;
return {default: Postpone};
});

function App() {
return (
<div>
<Suspense fallback="Outer">
<Suspense fallback={<Postpone />}>
<PostponeAndDelay /> World
</Suspense>
<Suspense fallback={<Postpone />}>
<Lazy />
</Suspense>
</Suspense>
</div>
);
}

const prerendered = await ReactDOMFizzStatic.prerenderToNodeStream(<App />);
expect(prerendered.postponed).not.toBe(null);

prerendering = false;

// Create a separate stream so it doesn't close the writable. I.e. simple concat.
const preludeWritable = new Stream.PassThrough();
preludeWritable.setEncoding('utf8');
preludeWritable.on('data', chunk => {
writable.write(chunk);
});

await act(() => {
prerendered.prelude.pipe(preludeWritable);
});

const resumed = await ReactDOMFizzServer.resumeToPipeableStream(
<App />,
JSON.parse(JSON.stringify(prerendered.postponed)),
);

expect(getVisibleChildren(container)).toEqual(<div>Outer</div>);

// Read what we've completed so far
await act(() => {
resumed.pipe(writable);
});

// Should have now resolved the postponed loading state, but not the promise
expect(getVisibleChildren(container)).toEqual(
<div>
{'Hello'}
{'Hello'}
</div>,
);

// Resolve the final promise
await act(() => {
resolve('Hi');
});

expect(getVisibleChildren(container)).toEqual(
<div>
{'Hi'}
{' World'}
{'Hello'}
</div>,
);
});
});
98 changes: 96 additions & 2 deletions packages/react-dom/src/__tests__/ReactDOMFizzStaticBrowser-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -870,8 +870,6 @@ describe('ReactDOMFizzStaticBrowser', () => {

prerendering = false;

console.log(JSON.stringify(prerendered.postponed, null, 2));

const resumed = await ReactDOMFizzServer.resume(
<App />,
JSON.parse(JSON.stringify(prerendered.postponed)),
Expand All @@ -887,4 +885,100 @@ describe('ReactDOMFizzStaticBrowser', () => {
<div>{['Hello', 'Hello', 'Hello']}</div>,
);
});

// @gate enablePostpone
it('can postpone in fallback', async () => {
let prerendering = true;
function Postpone() {
if (prerendering) {
React.unstable_postpone();
}
return 'Hello';
}

const Lazy = React.lazy(async () => {
await 0;
return {default: Postpone};
});

function App() {
return (
<div>
<Suspense fallback="Outer">
<Suspense fallback={<Postpone />}>
<Postpone /> World
</Suspense>
<Suspense fallback={<Postpone />}>
<Lazy />
</Suspense>
</Suspense>
</div>
);
}

const prerendered = await ReactDOMFizzStatic.prerender(<App />);
expect(prerendered.postponed).not.toBe(null);

prerendering = false;

const resumed = await ReactDOMFizzServer.resume(
<App />,
JSON.parse(JSON.stringify(prerendered.postponed)),
);

await readIntoContainer(prerendered.prelude);

expect(getVisibleChildren(container)).toEqual(<div>Outer</div>);

await readIntoContainer(resumed);

expect(getVisibleChildren(container)).toEqual(
<div>
{'Hello'}
{' World'}
{'Hello'}
</div>,
);
});

// @gate enablePostpone
it('can postpone in fallback without postponing the tree', async () => {
function Postpone() {
React.unstable_postpone();
}

const lazyText = React.lazy(async () => {
await 0; // causes the fallback to start work
return {default: 'Hello'};
});

function App() {
return (
<div>
<Suspense fallback="Outer">
<Suspense fallback={<Postpone />}>{lazyText}</Suspense>
</Suspense>
</div>
);
}

const prerendered = await ReactDOMFizzStatic.prerender(<App />);
// TODO: This should actually be null because we should've been able to fully
// resolve the render on the server eventually, even though the fallback postponed.
// So we should not need to resume.
expect(prerendered.postponed).not.toBe(null);

await readIntoContainer(prerendered.prelude);

expect(getVisibleChildren(container)).toEqual(<div>Outer</div>);

const resumed = await ReactDOMFizzServer.resume(
<App />,
JSON.parse(JSON.stringify(prerendered.postponed)),
);

await readIntoContainer(resumed);

expect(getVisibleChildren(container)).toEqual(<div>Hello</div>);
});
});
Loading

0 comments on commit 813e43f

Please sign in to comment.