Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
docs: react 18, streaming SSR, rsc with new apis #33986
docs: react 18, streaming SSR, rsc with new apis #33986
Changes from 16 commits
4eeca93
a334326
2392013
d17ef8c
26c8747
3c8e770
3c1a31d
4e002ee
59abdbb
0e28d10
b34441f
d914e36
b7997f8
a3be20d
24f2e2b
5dc8762
f5c87e7
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
This file was deleted.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not strictly identical, as
React.lazy
doesn't do any preloading. Might be worth suggesting that you continue usingnext/dynamic
for now for performance?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
When we enable
{ suspense: true }
it's usingReact.lazy
and doesn't do preloading I think?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh, I see the issue -- we just haven't implemented the preloading yet (since it happens after the shell is flushed, where all of the existing preloading is done today), but we should. Otherwise there's no reason to have the
{ suspense: true }
option instead of just recommending that people useReact.lazy
instead.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The need for
{suspense: true}
makes it feel second-class. Do we plan to flip the default at some point in the future?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The issue is that currently that would be a breaking change without an incremental upgrade path: your application using
next/dynamic
would break when upgrading to React 18 unless you add suspense boundaries first, but you can't add suspense boundaries until you upgrade to React 18. It also changes the semantics a bit, since pre-Suspense, the loading state is associated with thenext/dynamic
component, rather than the parent where the component is rendered.Changing the default in the future is a possibility, but it's likely a major version away. Some shorter term options might be:
<Suspense>
boundary, we could assume you want the suspense behaviorReact.lazy
, we could push users toward that instead, when using React>=18Both of these options would probably require some support in React though.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It occurred to me that with facebook/react#23304 and facebook/react#23267 this might not actually be a breaking change anymore, if the transition semantics apply to
root.render()
as well... Would need to investigate further though.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think what we could theoretically do is use suspense by default during SSR and initial hydration if you're using React>=18, since that behavior shouldn't change (after the next RC).
For client-side navigations that would still be at risk of suspending, we could synchronously render the legacy non-Suspense based fallback that you provide with
next/dynamic
, and then also warn that you should add a suspense boundary and move your fallback there instead.Let me think about it some more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Just noticed that facebook/react#23304 does in fact change the behavior for
root.render(...)
as long as it's not wrapped influshSync
. So yeah, it would probably make sense to make this change.Thanks for flagging @gaearon
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think it might be worth adding a caveat to the main React 18 doc about using any existing (i.e. pre-React 18) suspense libraries, especially if you're relying on a library like
react-ssr-prepass
. This might not apply to many Next.js users though, unsure.