Multiple queries on one page

[k0ka]

What problem does this feature proposal attempt to solve?

My pages are generated using several GraphQL queries. Some of them can be cached and reused. But relay-nextjs allows only one query to be preloaded.

Which possible solutions should be considered?

I have implemented a proof of concept in https://github.com/k0ka/relay-nextjs/tree/multi-query But it is not backward compatible. The main idea is that instead of

WiredProps<P extends {},Q extends OperationType> 

it is using

WiredProps<P extends {},Q extends {[key: string]: OperationType}>

So the user may pass some hash of queries to preload them. The example page would look like this:

function UserProfile({ preloadedQuery }: RelayProps<{}, {preloadedQuery: profile_ProfileQuery}>) {

And one can preload several queries like this:

function UserProfile({ query1, query2 }: RelayProps<{}, {query1: profile_ProfileQuery, query2: profile_AnotherQuery}>) {

Why backward compatibility breaks?

While it looks easy to add the backward compatibility on the first glance, but after looking in-depth, there are a lot of nuances. And it might end up being a big kludge.

For example, you assume the page always has the preloadedQuery prop to extract the server environment from it:

  const relayProps = getRelayProps(pageProps, initialPreloadedQuery);
  const env = relayProps.preloadedQuery?.environment ?? clientEnv!;

We can enforce that this prop is always set up for some query (it looks like a kludge) or change it as I do in my PoC (breaking BC)

    const [relayProps, serverEnv] = getRelayProps(pageProps, initialPreloadedQueries);
    const env = serverEnv ?? clientEnv!;

Also it is not easy to distinguish OperationType and [key: string]: OperationType in javascript as they both might be objects. We have to check some of the OperationType key type that's always defined and is not an object.

What's next?

I implemented the PoC in a more elegant but BC Breaking way. The next question is whether you really need this feature in the package. It might be too complex and specific for my case. In this case I'll just maintain it in my repo.

If you do need it, we have to decide on the way how to implement it.

[FINDarkside]

Could you give some example use case? In what case would you need multiple queries instead of using fragments to make them into one query? Using multiple queries for a page seems like an anti-pattern unless I'm missing something.

[k0ka]

We have the "Top users" block on the sidebar of each page. It is changed infrequently and might be cached. We can't do it using one query as we can't cache query parts in Relay.

[FINDarkside]

We can't do it using one query as we can't cache query parts in Relay.

You can cache query parts if you want and relay will only fetch the data that is not in the cache. By default this library uses store-and-network fetch strategy meaning that it'll first use the cached data and then re-fetch it. If you want to avoid fetching data that is in the cache you should change the fetchPolicy to store-or-network.

[k0ka]

fetchPolicy is applied to the whole query.

if any piece of data for that query is missing or stale, it will fetch the entire query from the network.

So in my case it will refetch entire query including user top on each page.

[FINDarkside]

Hey you're absolutely correct, I had the wrong impression of how relay uses the cache! I now see the use case for this and might use this myself if it gets implemented. In perfect world I think this should be react-relay feature though (fragment/field specific caching). That's probably not going to happen though. :(

E: Apparently relay classic did only fetch the data not in cache, but that feature was discarded in relay modern for some other benefits.

[rrdelaney]

This seems like a worthwhile feature to implement. I've thought about the API a bit and would definitely like to do it in a backwards compatible way. What do you think about something like this:

function MyPageComponent({
  preloadedQuery,
  additionalQueries,
}: RelayProps<{}, page_Query>) {
  const pageQuery = usePreloadedQuery(MyPageQuery, preloadedQuery);
  const secondQuery = usePreloadedQuery(SecondQuery, additionalQueries[0]);
  const thirdQuery = usePreloadedQuery(ThirdQuery, additionalQueries[1]);

  // ...
}

export default withRelay(MyPageComponent, MyPageQuery, {
  // ...
  additionalQueries: [SecondQuery, ThirdQuery],
});

This should preserve backwards compatibility, maintain the simple API for most pages, but allow the flexibility of multiple queries.

[FINDarkside]

Personally I think it would be more clean to add new prop preloadedQueries which includes all queries (including the first one) in array. preloadedQuery could be kept just for backwards compatibility, but wouldn't be the "best practice" anymore, at least not when using multiple queries. Second argument to withRelay could accept array of queries and also support non-array for backwards compatibility.

[k0ka]

We should also think of variablesFromContext and fetchPolicy as they must return values per-query. I think it's a good feature that variablesFromContext is fully typed, so TypeScript users will have compile-time errors if they forgot anything.

In my PoC it is implemented as:

export interface WiredOptions<Queries extends {[key: string]: OperationType}, ServerSideProps> {
  variablesFromContext?: (ctx: NextPageContext | NextRouter) => { [K in keyof Queries]: Queries[K]['variables'] };

So the variableFromContext function must return an object like

{
  query1: {
    variable_for_query1: "value",
  },
  query2: {
    variable_for_query2: "value",
  },
}

and it tracks variable types.

We might provide two HoCs: leave the current one and create a new withRelayMulti

[rrdelaney]

@k0ka That's a good point about variablesFromContext not working the same anymore.

@FINDarkside Using an array or just changing the type of the second argument can get weird. For example, if we modify relay-nextjs to detect arrays in the second argument, but then a later change to Relay switches the compiled output of queries to also be arrays we will never have the correct behavior.

[FINDarkside]

Was just thinking about this and realized this would also solve the underlying issue in #73. Or at least be a workaround for it.