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

[WIP] Possible evolution of the Relay integration with Next.js v13 #270

Draft
wants to merge 5 commits into
base: main
Choose a base branch
from

Conversation

alunyov
Copy link
Contributor

@alunyov alunyov commented Jan 11, 2023

Note: This is WIP and not ready to be merged. I’m sharing for feedback and adjustments on the direction.

Summary:

  • Data-fetching for routes is happening in the Relay Server Components, using fetchQuery this version is different from what we export from relay-runtime - (maybe we’ll need a different name). Internally, it fetches the query, publish to the relay store. This fetchQuery can only be called on the server.

  • RelayRoot - RSC, simple wrapper, that passes serialized list of fetched queries to the RelayClientRoot

  • RelayClientRoot - client component that wraps the children with Relay context provided, and publishes the fetched server queries to the client store, so the state of the local store is matching the one on the server.

    • Everything under this RelayClientRoot is normal client-first relay with hooks and things... But there is no root query component (with usePreloadedQuery or similar...).
    • Added server-only version of useFragment (thanks @sibelius for the question: [WIP] Possible evolution of the Relay integration with Next.js #269 (comment)) - this will allow for individual server components define own data requirements, and the view will be fully server-rendered, with possible client leafs. The fragments composed into a query and fetched in the top RSC with fetchQuery.

Note on the API naming: we may want to name these useFragment and fetchQuery differently, to avoid possible confusion with similar APIs for react-relay and relay-runtime.


So, this is a “sketch” of the possible integration, so many things may not work:

  • Preserving the state of the store, when navigating between the list and the issue view done with keeping track of seenQueries but may not work in general case.
  • Multiple leafs with RelayClientRoot.

Not sure if it’s working:

  • useLazyLoadQuery - under RelayClientRoot. (should work, I think)
  • 3D/queryPreloading
  • stream/defer (most likely not working)
  • GC for the leaf client components that don't have known owners on the client

}
const data = record.response.data ?? null;
if (data != null) {
environment.commitPayload(record.operationDescriptor, data);
Copy link
Contributor

Choose a reason for hiding this comment

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

Just a thought about alternative, this could happen more granularly at useFragment if we change Relay implementation. If the fragment can't be resolved from the store, we could check if the fragment owner exist in a global fetchedQueries and commit if not.

@alvarlagerlof
Copy link
Contributor

alvarlagerlof commented Jan 29, 2023

I wonder if this could work with fetch-multipart-graphql. Part of the benefit of server components without GraphQL (right now) is being able to load different things at different rates with different suspense boundaries.

If we can have a fragment await until it get's the data that it needs, and then have them nested, and some of them using @defer, that would make the Server components experience for this very very nice. You could have skeletons at different levels of data fetched while still only using client components at the leafs.

I have been trying to look into how Relay works internally to understand if it's possible. I think, if the hooks right now are able to suspend until deferred data is loaded, it should "in theory" be a matter of instead being able to await the data for a fragment instead.

} from "relay-runtime";
import { networkFetch } from "./environment";

const fetchedQueries: FetchedQuery[] = [];

Choose a reason for hiding this comment

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

Wouldn't this leak data between requests and cause a memory leak?

Choose a reason for hiding this comment

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

maybe we can use AsyncLocalStorage/async_hooks for this.

@tbezman
Copy link
Contributor

tbezman commented May 15, 2023

@alunyov any updates to share?

@alunyov
Copy link
Contributor Author

alunyov commented May 15, 2023

@alunyov any updates to share?

Sorry, no real updates here. I'm on parental leave, maybe I will get back to this in a few month.

@tinleym
Copy link

tinleym commented Oct 3, 2023

This might run into garbage collection issues without usePreloadedQuery/useQuery or environment.retain()... I ran into this with a similar setup when I was running multiple queries on input events after the page fetch.

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

Successfully merging this pull request may close these issues.

7 participants