Skip to content
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

RFC: The Next.js "App Router", React Server Component & "SSR with Suspense" story #9

Open
wants to merge 11 commits into
base: main
Choose a base branch
from

Conversation

phryneas
Copy link
Member

@phryneas phryneas commented Apr 24, 2023

The RFC

You can read the rendered RFC here.

The examples

You can find some example applications in the examples folder.

  • The polls-demo is a simple poll app that showcases both Server Components and Client Components in a single app. It shows the difference between using Apollo Client in Server Components and Client Components.
  • The hack-the-supergraph-ssr example is an example exploring the "SSR and data transporting" angle more in-depth. It also uses the @defer-related Links discussed in the last part of the RFC.
    You can change the time deferred responses from the server take by adjusting it in the "Demo Settings" in the top-right corner. The link is configured so that any fragment coming in after 100ms will not be part of SSR, but be fetched on the client. Keep in mind that this uses a real network connection, so there might be some network delay on top of the number you choose here.
  • The app-dir-experiments is more of an "experimenting around" app I used for building the package

Points to look out for

Copy link

@gaearon gaearon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you so much for writing! sorry for terse tone — writing from mobile

RFC.md Show resolved Hide resolved
RFC.md Outdated Show resolved Hide resolved
RFC.md Outdated Show resolved Hide resolved
RFC.md Outdated Show resolved Hide resolved
RFC.md Outdated Show resolved Hide resolved
RFC.md Outdated Show resolved Hide resolved
RFC.md Outdated Show resolved Hide resolved
RFC.md Show resolved Hide resolved
RFC.md Outdated Show resolved Hide resolved
RFC.md Show resolved Hide resolved
Copy link
Member Author

@phryneas phryneas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you so much for the thorough review @gaearon!
I've left some more questions for clarification, and will be rewriting some parts of the document according to your explanations and suggestions on monday!

RFC.md Outdated Show resolved Hide resolved
RFC.md Show resolved Hide resolved
RFC.md Outdated Show resolved Hide resolved
RFC.md Outdated Show resolved Hide resolved
RFC.md Outdated Show resolved Hide resolved
RFC.md Outdated Show resolved Hide resolved
RFC.md Outdated Show resolved Hide resolved
RFC.md Show resolved Hide resolved
RFC.md Outdated Show resolved Hide resolved
RFC.md Show resolved Hide resolved
@gaearon
Copy link

gaearon commented May 6, 2023

awesome, did another pass to respond.

@gaearon
Copy link

gaearon commented May 6, 2023

and appreciate your weekend time — sorry i didn't get to this earlier.

RFC.md Outdated Show resolved Hide resolved
phryneas and others added 3 commits May 8, 2023 14:28
First batch of a lot of adjustments. Thanks to @gaearon for the thorough review.
@phryneas phryneas changed the title RFC: The React Server Component & "SSR with Suspense" story RFC: The Next.js "App Router", React Server Component & "SSR with Suspense" story May 8, 2023
@gaearon
Copy link

gaearon commented May 9, 2023

The thing I dislike about the diagrams is they show "SSR Component A", "SSR Component B", "Browser Component A", "Browser Component B" as vertical lines. Vertical lines in this diagram style represent things that have a lifetime – like processes that respond to messages. But components are not processes. In the RSC pass, they have no lifetime at all: a component doesn't "exist". It's just a function call. On the browser, components kinda "exist" but they also don't really "receive messages".

The vertical line that would make sense to me is "RSC renderer" and "Client-side React". And "render component A" or "receive HTML for component A" would make sense as "messages" passed to it.

RFC.md Show resolved Hide resolved
RFC.md Outdated Show resolved Hide resolved
RFC.md Show resolved Hide resolved
RFC.md Show resolved Hide resolved
Copy link

@gaearon gaearon left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

aside from parts i don't have context on, lgtm

RFC.md Outdated Show resolved Hide resolved
@phryneas
Copy link
Member Author

For now I'm not gonna merge this PR, but link to it from the README. The discussion here might prove very useful for others trying to understand even more nuance. More discussion here is very welcome!

@Ephem
Copy link

Ephem commented May 11, 2023

Absolutely phenomenal work on this! 👏 I've done an initial skim and intend to read it more in depth soon.

Getting data from RSC into the SSR pass

I've discussed the possibility of using the RSC cache to rehydrate the SSR cache with @gaearon and we agree that it mostly doesn't make sense. The base assumption was that something in the RSC cache would be valuable for the SSR pass. If this were the case, the same data could be rendered by RSC and SSR/Browser components. But while the latter could update dynamically on cache updates, the former could not update without the client manually triggering a server-rerender.

I'd like to add a case to this discussion, what about prefetching data in RSCs that is only used in Client Components? One nice thing about RSCs is that they provide a framework-agnostic central way to prefetch. This would rely on de/rehydration of Promises, but does seem like a valid use case for wanting to reuse parts of the cache (even though they haven't finished fetching yet)?

Any parts only fetched in Client Components still suffer from client-side request waterfalls unless they are explicitly prefetched, so if we can't prefetch those in RSCs, where do we prefetch them (solvable, but would be cumbersome and different per framework/app)? It would be like using RSCs + getInitialProps so to speak.

Another reason to support it could be for data that is shown both in RSCs and Client Components, but that is never refetched (infinite stale time), or only refetched as part of a RSC refetch. In this case you could pass down a SC to the point where the CC is and show the data that way instead, or pass down the raw value, but this kind of prop drilling might be cumbersome.

I'm sure I could come up with other cases (valid or not). I do agree with the original premise that it mostly doesn't make sense though, for the general case.

@phryneas
Copy link
Member Author

phryneas commented May 11, 2023

I'd like to add a case to this discussion, what about prefetching data in RSCs that is only used in Client Components? One nice thing about RSCs is that they provide a framework-agnostic central way to prefetch. This would rely on de/rehydration of Promises, but does seem like a valid use case for wanting to reuse parts of the cache (even though they haven't finished fetching yet)?

This is something that I want to keep exploring, and yes, passing a Promise from the RSC into the Client Component could be very promising here.
The only immediate problem that I can see here is that we couldn't pass that over from everywhere in the code - it would have to be a JSX prop, so we would probably end up with a <PrefetchQuery> component that would have to be rendered out. (passing it into the Provider doesn't make sense, since that is in the Layout and you probably want to prefetch at least on a per-page level.)

Any parts only fetched in Client Components still suffer from client-side request waterfalls unless they are explicitly prefetched, so if we can't prefetch those in RSCs, where do we prefetch them (solvable, but would be cumbersome and different per framework/app)? It would be like using RSCs + getInitialProps so to speak.

Waterfalls are less of a problem if you use fragment composition, so that's generally a pattern that we want to make more popular with Apollo Client.
As for the alternative to getInitialProps - that would be the SSR render of your client components. Of course, that would only apply for the first page that a user is accessing - but on the other hand, it probably doesn't play too much of a difference if these requests happen from the server or the client, after the page has been hydrated once and is "live".

Another reason to support it could be for data that is shown both in RSCs and Client Components, but that is never refetched (infinite stale time), or only refetched as part of a RSC refetch. In this case you could pass down a SC to the point where the CC is and show the data that way instead, or pass down the raw value, but this kind of prop drilling might be cumbersome.

This one could lead to a bit of a slippery slope, and in the long run probably end up with inconsistencies if developers are not very careful with it - but at the same time I do see some use cases.
Maybe we could add something like a repository-aware eslint plugin that could make sure that data prefetched in RSC is only ever queried from other server components, and client components make sure to always use the cache-only fetchPolicy 🤔

Absolutely phenomenal work on this! 👏 I've done an initial skim and intend to read it more in depth soon.

Thanks, and please let us know if you have any further thoughts!

@Ephem
Copy link

Ephem commented May 11, 2023

so we would probably end up with a component that would have to be rendered out. (passing it into the Provider doesn't make sense, since that is in the Layout and you probably want to prefetch at least on a per-page level.)

It's nice to hear that this is something you are thinking about too. For React Query we are renaming <Hydrate> to <HydrationBoundary> in v5 precisely because of this, the idea being that you might want several throughout your tree, even further down in nested layouts. I hadn't thought about an explicit <Prefetch> component, that's worth considering!

Waterfalls are less of a problem if you use fragment composition

That's true! I was stuck thinking about the general case.

Of course, that would only apply for the first page that a user is accessing - but on the other hand, it probably doesn't play too much of a difference if these requests happen from the server or the client, after the page has been hydrated once and is "live".

Yeah, for the SSR pass this is no more a problem than for RSCs (depending on where the RSC and the SSR servers are located compared to your backend/DB, if RSC is co-located with those and SSR lives on the edge, doing it in SSR would be worse). For client side page transitions though this would lead to the same bad performance as request waterfalls have today. Part of the motivation for RSCs is to "solve" request waterfalls, but they can't do that for data used in Client Components unless we also prefetch that data there, if that makes sense? (Though fragment composition works around that!)

This one could lead to a bit of a slippery slope, and in the long run probably end up with inconsistencies if developers are not very careful with it - but at the same time I do see some use cases.

Definitely! My thinking though is that when you consider all of the above things together, and think about how complex this area is to reason about, it would be really nice to provide developers with a single coherent API for prefetching stuff.

If we could say "If you are using RSCs, just make sure you prefetch everything in those" and make that work well, that would be golden. Again, I do think fragments are a good alternative solution for this for Apollo (especially considering de/rehydrating promises is pretty alpha), so I'm talking more high level/generally here. 😃

Edit:

Maybe we could add something like a repository-aware eslint plugin that could make sure that data prefetched in RSC is only ever queried from other server components, and client components make sure to always use the cache-only fetchPolicy 🤔

Yes, I think there are several ways to support this and help developers avoid ending up with tearing everywhere. It might even be possible to enforce. If you want to prefetch data for the client, pass down a Promise. If you hydrate actual data from an RSC to a Client Component, force "cache-only"/Infinite stale time (possibly with per query opt-out as escape hatch).

@phryneas
Copy link
Member Author

Notable things that have happened since this RFC:

Support for useBackgroundQuery and useReadQuery #38 (in SSR)

Usage for these is

function Child({ queryRef }) {
  const { data } = useReadQuery(queryRef);

  return <div>{data.foo.bar}</div>;
}


function Parent() {
  const [queryRef] = useBackgroundQuery(query);

  return (
    <Suspense fallback={<SuspenseFallback />}>
      <Child queryRef={queryRef} />
    </Suspense>
  );
}

The difficulty here is that child components suspense, while the parent starts the request. To get this to work, we simulate an ongoing request in the browser while the real request happens on the server.

restart simulated streamed request on stream close #62 (in SSR)

With these simulated queries, there is always the risk that for some reason, they are started on the server, but not actually consumed (the useReadQuery might be behind a conditional) - which means that the stream closes before we can actually transport the data over.
We have no way of detecting a stream close on the server at this moment (hello, feature request!), so that request will just keep going on the server. But on the browser, we can wait for the .ready event which means that definitely no more HTML will be streamed in. In that case, we have to restart the request in the browser so we at least get a result eventually.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants