Client render Suspense content if there's no boundary match #16945
Conversation
Without the enableSuspenseServerRenderer flag there will be no boundary match. Also when it is enabled, it will client-render the Suspense content and treat it as a mismatch if there's no boundary in the HTML.
| element, | ||
| ), | ||
| ).toWarnDev( | ||
| 'Warning: Did not expect server HTML to contain a <div> in <div>.', |
There was a problem hiding this comment.
This warns because of the superfluous div but before that it should warn about missing a Suspense boundary but it's currently missing that warning:
|
FYI, Next.js' specific The behavior Next.js relied on, represented as a test: it('Suspense + hydration in legacy mode with fallback', () => {
const element = document.createElement('div');
element.innerHTML = '<div>Hello World</div>';
let div = element.firstChild;
let ref = React.createRef();
ReactDOM.hydrate(
<React.Suspense fallback={<div>Loading...</div>}>
<div ref={ref}>Hello World</div>
</React.Suspense>,
element,
);
// Suspense component did not suspend during hydration, so
// it was hydrated as if it was absent.
expect(ref.current).toBe(div);
expect(element.innerHTML).toBe('<div>Hello World</div>');
});☝️ this worked because Next.js guaranteed nothing in the tree would suspend during hydration / initial render. |
|
We released Meaning, We don't necessarily need this old behavior to be supported -- the Suspense boundary was not used for any stable Next.js feature (it was flagged behind an experimental feature). However, this has been released in Next.js for ~6 months (April 2nd). Time (and cross-team coordination) could probably heal this, the End-users and other companies in the ecosystem may have been relying on this behavior though (see comment above). We've seen companies/apps rely on this behavior in the wild. Areas (apps built with or inspired from) we should probably look into: |
Yeah we realized when discussing this incident that we really should be running smoke tests on the major React frameworks as part of our release plan.
That might be wise. We'll wait to land this change until after we figure out what to do in Next and the other projects that might be relying on this accidental behavior. In the meantime, I marked |
|
I'm not opposed to releasing a |
Without the
enableSuspenseServerRendererflag there will never be a boundary match. Also when it is enabled, there might not be a boundary match if something was conditionally rendered by mistake.With this PR it will now client render the content of a Suspense boundary in that case and issue a DEV only hydration warning. This is the only sound semantics for this case.
Unfortunately, landing this will once again break #16938. It will be less bad though because at least it'll just work by client rendering the content instead of hydrating and issue a DEV only warning.
However, we must land this before enabling the
enableSuspenseServerRendererflag since it does this anyway.I did notice that we special case
fallback={undefined}due to our unfortunate semantics for that. So technically a workaround that works is actually setting the fallback to undefined on the server and during hydration. Then flip it on only after hydration. That could be a workaround if you want to be able to have a Suspense boundary work only after hydration for some reason.It's kind of unfortunate but at least those semantics are internally consistent. So I added a test for that.
cc @Timer @timneutkens