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

Relay and Next13 #4107

Open
jantimon opened this issue Oct 27, 2022 · 22 comments
Open

Relay and Next13 #4107

jantimon opened this issue Oct 27, 2022 · 22 comments

Comments

@jantimon
Copy link

Hello 👋

This is a follow up on #3889

Next13 ships with a new way to load data on the server and client:

Could you please share your opinions and visions if/how relay might fit in here?

Does it make sense to use Relay at all for Next13 Server Components?

Does it make sense to use Relay for Next13 app layout and app pages?

Will reacts fetch cache work well with relays cache?

@tinleym
Copy link

tinleym commented Nov 1, 2022

Here are some observations I thought I'd share, as I've been trying to wrap my mind around this. Please correct me where I'm wrong :).

Traditional Query / Fragment composition seems like it holds. Per https://beta.nextjs.org/docs/rendering/static-and-dynamic-rendering#dynamic-rendering, a single "dynamic" fetch anywhere in the route causes it to switch from static to dynamic. If a component/fragment will just go the way of its route's query, I can't see any drawbacks to just having a query per route and composing fragments as usual.

Populating the client store with server data doesn’t seem like it should be much different than it was pre Next 13. The question I have is, how do you use the client store as a cache when route requests happen on the server?

The server would need to know what the client already has. But you shouldn't need to send your entire store on every navigation. It seems like you’d do fine with some kind of pre-navigation hook that takes what it can from the client store, and then sends that payload to the server with the navigation request. The server can then extract that payload from its request and finally stitch everything back together when the request completes. Then you’re back on trodden ground.

I don’t think there’s a way to do this with the current APIs. However, this thread makes me think support is on its way:
https://twitter.com/sebmarkbage/status/1585813880097607680

In there Seb mentions “Server Context”, which entails sending a “small” amount data back and forth on each request. The thread also contains a link to a merged PR, Flight Support for ServerContext. If I’m not mistaken, Flight is part of Meta’s undocumented React/Relay/SSR story.

UPDATE: Server Context does not appear to be the answer: https://twitter.com/sebmarkbage/status/1587615489605369861

It would be nice to get some official word!

@alunyov
Copy link
Contributor

alunyov commented Nov 8, 2022

I've been experimenting with the Next.js v13 a little bit.

Here's my thoughts so far...

In general, yes, it is very much possible to use Relay and Next.js v13. There are few missing (or not fully complete/compatible) APIs on the Relay side to make this integration natural and easy to set up. Specifically, this is related to the Preloading API (we cannot use loadQuery directly in the RSC now, for example).

But, these APIs (or simplified version) possible to implement outside of Relay, I think.

Here's an example PR that adds integration for Relay and Next.js v13. With RSC doing the data-fetching/data-preloading, serialized payloads are sent to the client, where we transform them into preloaded query references for usePreloadedQuery: relayjs/relay-examples#241

Please note: the preloading in this example probably won't fully work with @defer an @steam directives. Also, I want to hear more from @voideanvalue on the overall approach.

Does it make sense to use Relay at all for Next13 Server Components?

I think, yes. The Server Components is a generalized implementation of Relay EntryPoints. Again, we may need to polish some of the Relay APIs to make this integration seamless.

Does it make sense to use Relay for Next13 app layout and app pages?

I think, yes. There is nothing specific in Relay that should prevent using this. You may need to structure you app in a way, that will put the query fetching into the top-level/page-level server-components.

Will reacts fetch cache work well with relays cache?

There maybe some edge-cases related to mutations and cache invalidation. So you may need to play around with cache settings.


Re @tinleym comments:

The server would need to know what the client already has

I don't think the server needs to know what the client has, I think it just need to be responsible for fetching.
However, this may change in the future. But for for now, I think, just data-fetching/preloading and server-rendering.

@tinleym
Copy link

tinleym commented Nov 8, 2022

Thank you for this, @alunyov!

Having experimented with this integration more myself, my main outstanding question is how to best get data into RSCs, especially if they’re deeply nested.

The base case would be to server render stateless components with data. Root query data can be directly passed around, but it just feels like a regression to forgo fragments for prop-drilling. Next 13 showcases RSCs as a solution to prop-drilling for REST API clients via co-located fetching; it also feels like a regression to similarly make a bunch of little queries per request with Relay after being spoiled with fragment hoisting.

Referencing what Seb says here regarding server context alternatives: https://twitter.com/sebmarkbage/status/1587615487336251393, I’m wondering about the viability of a store that lasts the lifetime the request, and if that might enable on the server the fragment co-location Relay users are accustomed to on the client.

The other case I’m wondering about is (I think) aligned with the goals of Data-Driven Dependencies. Say I have a guest and user version of my app. Ideally, I’d want to only send the code for the current authz. It would be nice to be able to load a union into a server component, and then use the resulting object type to render along the appropriate path. Is a preloaded fragment a silly idea?

@alunyov
Copy link
Contributor

alunyov commented Nov 8, 2022

Thank you @tinleym!

This is just my opinions, and this may change over time. But this how I feel at this moment about your questions.

my main outstanding question is how to best get data into RSCs, especially if they’re deeply nested.

That's where the main difference (and it is a question of trade-offs) with Relay approach of combining all data requirements into a single query vs. Next.js individual fetches in components.

In both cases you have to collocate the data requirements next to the component that uses them. In Relay this is done via fragments, in Next.js - via fetch calls inside the component.

Once you have all fragments spread in your query you can start the fetch in the the top-level RSC. It won't be possible to use deeply nested RSC in that tree, because after you pass the query reference to the Relay root, you'll have only client components after that, they are using context.

It is still possible to lift the fetch for a deeply nested query to the top-level RSC, and pass it's preloaded query reference to it's usePreloadedQuery via context. If you have multiple of these queries, you can start fetching them in paralell.

Referencing what Seb says here regarding server context alternatives

I don't think we currently have plans to refactor/change this part. It's pretty fundamental change to Relay.

The other case I’m wondering about is (I think) aligned with the goals of Data-Driven Dependencies.

I'll take a look at the 3D Example, I think it still should be possible to use it with the new version.

@litewarp
Copy link

litewarp commented Nov 8, 2022

I put together this implementation based on the 3D example. It works but would appreciate feedback from others.

relayjs/relay-examples#242

@tinleym
Copy link

tinleym commented Nov 12, 2022

@alunyov I've been using your example repo as a reference; it's been very helpful, thank you!

One thing I'm curious about: in environment.ts, why is QueryResponseCache instantiated conditionally on IS_SERVER?

I understand that IS_SERVER narrows a client component environment where client APIs aren't available. I've found that this environment also doesn't attach credentials to fetches, which can cause a fetch to be fired without cookies it might need.

Removing the IS_SERVER condition to instantiating QueryResponseCache seems to solve this issue, as the cache will preclude the credentialless fetch. Is there a reason I'm missing not to do this?

@tinleym
Copy link

tinleym commented Nov 17, 2022

I'm having an issue with stale data coming out of usePreloadedQuery (the triggering condition in my case is deleting cookies and reloading). The current (and correct) data is coming through on the fetch and is being cached in the network, but I'm getting stale data in the environment, which is forwarded on to the preloadedQuery on the first render. The current data comes through on subsequent renders, causing a hydration error.

It seems like this could be fixed by injecting the query directly into the environment store. What's the best way to do this?

@hanford
Copy link
Contributor

hanford commented Dec 28, 2022

Very excited about all developments happening in Next.js 13 and Relay.

I've built out a POC closely following https://github.com/relayjs/relay-examples/tree/main/issue-tracker-next-v13 for an internal product where I work and wow is it interesting

This is a big improvement over the previous Next.js <> Relay integrations I had looked at, and I was hesitant to reach for something like this.

@pankali
Copy link

pankali commented Jan 5, 2023

If someone came here looking for a tutorial on using relay with Nextjs 13, I found this step by step guide:
https://github.com/Roshanjossey/nextjs-13-relay

@hrasoa
Copy link

hrasoa commented Jan 22, 2023

Is relay still worth it with server components ? I mean context does not work on server (all the relay hooks won't work) so have to send everything on the client.

@Albert-Gao
Copy link

Albert-Gao commented Jan 25, 2023

Relay (SPA), SSR, and RSC are 3 different ways to solve the waterfall, and once you have done the Relay way, I do not see the point of doing the SSR/RSC way, SSR is janky than SPA, RSC is way better. But with a SPA, once you lifted the data request out of the component lifecycle, it is way faster than any kind of SSR/RSC, since the whole static assets could be retrieved from CDN and cached locally.

@arcticmatt
Copy link

@alunyov curious if you've tried relay-nextjs?

@fveracoechea
Copy link

@Albert-Gao that is a good point, on the other hand, there are other reasons why you would want to use NextJs v13 besides RSC:

  • Build speed
  • Code Splitting
  • Image optimizations
  • File routing
  • Etc...

There is a scenario in which you can leverage RSC for static/presentational components that do not ship JS to the browser, and Client site dynamism with Relay.

@hassan-zaheer
Copy link

Hey team,

Hope it's okay to drop in a relevant question, I've followed Relay's official example to integrate with Next13, but I need to pass in cookies that I get from the ingress request to any egress request to downstream dependencies

From my understanding, the way to do that in latest version is to use nextjs/headers which provides cookies but I get a warning from NextJS if I import this into https://github.com/relayjs/relay-examples/blob/main/issue-tracker-next-v13/src/relay/environment.ts - as this is being provided to Relay environment context which gets set up in client component and this is a server only component module.

Is there a workaround to this!?

@vinceprofeta
Copy link

@hassan-zaheer Were you able to come up with a good solution?

@hassan-zaheer
Copy link

hassan-zaheer commented Aug 4, 2023

@vinceprofeta yes, basically I moved <RelayEnvironmentProvider> to a client component, which takes in cookie as a prop, and this is rendered by a Server component which has access to cookies from { cookies } from "next/headers". Then I changed the create environment implementation from the Relay-NextJS 13 example to take cookie as an optional parameter and set it to egress requests when it's provided

@vinceprofeta
Copy link

vinceprofeta commented Aug 5, 2023

@hassan-zaheer Nice that works. I was able to forward the browser cookie to the gql server, but do you have a good pattern for forwarding the session cookie set by the graphQL server and setting it on the browser? Im thinking about using the app dir api/route as the way to talk to the GQL server. That way i have access to the Req/Res, but seems silly that I would do that from a server component. Thoughts?

@mizdra
Copy link

mizdra commented Apr 21, 2024

Hello everyone!

I have recently been working on improving the official example (https://github.com/relayjs/relay-examples/tree/b6f9b199d0b8027b5a76a11f1821631b216f4df4/issue-tracker-next-v13). The motivation is to reduce the Client Component.

The official example uses a lot of usePreloadedQuery and useFragment, so many components are converted to Client Component. However, that would destroy the advantages of the Server Component.

Therefore, we have created a PoC that allows you to keep more components as Server Component.

This PoC uses fetchQuery for data fetch and readInlineData for data masking. Because they are not React Hooks, they can also be used from Server Component. This allows many components to be converted to Server Component.

I believe this PoC is a big step forward over the current official example. I would love to hear from maintainers about this PoC.

PS: I noticed that there is a problem with mutation not updating the display of the part rendered by the server component. Perhaps we need to request the server to re-render the Server Component after running the mutation. I am working on that now.

@shadiramadan
Copy link

@mizdra I'm going to give this a shot!

@shadiramadan
Copy link

@mizdra I was able to run your example and incorporate that pattern into a sample app- but I was unable to get nested @inline fragment spreads working (nested readInlineData).

The second @inline (fragment in fragment) caused:

**TypeError: createContext only works in Client Components. Add the "use client" directive at the top of the file to use it.**

no matter what I did (I need to avoid use client).

Not sure where I'm going wrong but your example did not have such a scenario (your SideBar inline has a usePaginationFragment inside it and not another one from readInlineData). Curious if you can reproduce this issue...

@mizdra
Copy link

mizdra commented Sep 4, 2024

@shadiramadan Thanks for trying! But we should not talk about it in this issue. Can you report a bug via https://github.com/mizdra/poc-nextjs-app-router-relay/issues? It would be helpful if you also provide reproduction code.

@shadiramadan
Copy link

@mizdra I came back to this and thought I'd update by saying I found an unrelated react-relay import instead of relay-runtime. I ended up adding an eslint rule to ban react-relay for readInlineData. All good thank you for the nice POC!

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

No branches or pull requests