@connection directive ignored when used in conjunction with keyArgs

[habovh]

I'm back with another caching issue @benjamn, hope you'll like this one too!

As I understand it, Apollo supports the @connection directive. I understand that keyArgs is better suited for most use-cases, however I'm finding myself in a tricky situation here.

I have an endpoint that's a relay-style pagination that's configured to use Apollo Client's exported relayStylePagination(). This endpoint is called in multiple places in my app, and I would like each place to have its own data instead of sharing the same data from the cache. So my idea was to use a unique @connection directive on each of these queries, so that the result would be stored under different keys in the cache and my lists would be independent.

However, it seems that when I'm applying the @connection directive along with the relayStylePagination(), the directive gets ignored and the data is stored according to the provided keyArgs only.

In the following screenshots, I'm using the same query with different keyArgs configuration on the typePolicy.

The query:

query {
  suggestions({ first: 3 }) @connection(key: "newsfeedSuggestions") {}
}

Here's what my cache looks like with the keyArgs defined:

Here's what my cache looks like without keyArgs defined:

Note 1: I realise I can still achieve the desired result using a key args function instead of an array of strings. If this is the intended route, then updating the docs and explicitly stating that keyArgs cannot be used in conjunction with the @connection directive would be nice.

Note 2: my need came from a weird other bug preventing me from having multiple useQuery hooks pointing on the same cache object for paginations. It seems like when I do so, each list is "fighting" the other and tries to write its own result to the cache in a loop, resulting in glitchy and unusable UI.

Intended outcome:

I'm expecting that when using @connection directive along with keyArgs both are used to generate identifiers for my endpoints.

Actual outcome:

@connection directive is ignored.

How to reproduce the issue:

• Use relayStylePagination() to define a field policy. E.g: books: relayStylePagination(['filter'])
• Query this field using @connection directive to set a custom key to store the query result. E.g: books(first: 2) @connection(key: "sideBarBooks") {
• See that it gets stored under ROOT_QUERY.books:{} instead of something like ROOT_QUERY.books:{}:sideBarBooks

Versions

  System:
    OS: macOS 11.5.2
  Binaries:
    Node: 15.14.0 - ~/.nvm/versions/node/v15.14.0/bin/node
    Yarn: 1.22.10 - /usr/local/bin/yarn
    npm: 7.7.6 - ~/.nvm/versions/node/v15.14.0/bin/npm
  Browsers:
    Chrome: 92.0.4515.159
    Firefox: 88.0
    Safari: 14.1.2
  npmPackages:
    @apollo/client: ^3.4.8 => 3.4.8 
    apollo3-cache-persist: ^0.9.1 => 0.9.1 

[habovh]

Here's the custom KeyArgsFunction I'm using as a "workaround":

const keyArgsWithConnectionDirective: KeyArgsFunction = (args, context) => {
  const { fieldName, field: { directives = [] } } = context

  const keyArgs = ['context', 'randomize'] // These could be provided through params
  const filteredArgs = keyArgs.reduce((acc, current) => {
    const res = { ...acc }
    if (context.variables[current]) {
      res[current] = context.variables[current]
    }
    return res
  }, {})

  const connectionDirective = directives.find((directive) => directive.name.value === 'connection')
  const connectionKey = connectionDirective?.arguments.find((dirArg) => dirArg.name.value === 'key')?.value.value

  return `${fieldName}:${JSON.stringify(filteredArgs)}${connectionKey ? `:${connectionKey}` : ''}`
}

The generated key for the type policy using this key args function along with relayStylePagination() looks like this:

suggestions:{}:dedicatedSuggestions
suggestions:{}:newsfeedSuggestions
suggestions:{}:requestsSuggestions

Edit: this might be over-opinionated, since I'm deciding to let some of the key being defined by the type policy's key args and supporting only the key argument of the @connection directive here. One might want to be able to use the complete @connection directive. This starts to feel more and more as a perfect use-case for a custom KeyArgsFunction. Maybe all this issue is about updating the docs? Maybe not?

[benjamn]

Thanks for surfacing this @habovh!

You're right that the @connection directive's contribution to the field key gets lost if you switch to using keyArgs: [...] without providing a fancy keyArgs function that takes field.directives into account.

When AC3 was released, I hoped the @connection directive would become obsolete thanks to keyArgs. However, @connection(key: ...) is still a good way to provide distinguishing information for query fields whose names and arguments are otherwise indistinguishable from one another, without having to add additional client-only arguments to the field (which might be rejected by the server).

I see a few different possible directions from here:

• Make it easier to write a keyArgs function that properly examines/includes field.directives, by providing more helper utilities
  • Downside: typical keyArgs: [...] won't include @connection information
• Append the @connection key automatically to the field key, when using keyArgs: [...] syntax
  • Downside: hard-coding support for the @connection directive (which we already do elsewhere)
• Support a new keyDirectives: ["@connection"] field policy for enabling specific directives, similar/adjacent to keyArgs
  • Downside: new configuration surface area, design details TBD

Do any of those ideas jump out at you as obviously the best or the worst?

Improving the @connection situation may help with these other recent issues as well:

• getStoreKeyName does not honor @connection if field is using defaults for all args (has no args) #8619
• Query silently fails with multiple paginated queries with differing child fields #8620

[benjamn]

@habovh I think I found an ergonomic and flexible way to include directives (and variables) in the keyArgs: [...] array, without having to resort to implementing a keyArgs function: #8678

The biggest drawback is that keyArgs feels like the wrong name for something that deals with directives and variables as well as arguments, but I found a clever way around that problem, too (the key synonym for keyArgs). Update: While writing initial documentation, I realized the new key configuration did not really simplify anything, so I reverted those changes, and we'll be sticking to just keyArgs for now.

These improvements will likely be released in a v3.5 beta version first, but I'm not opposed to back-porting them to v3.4.x, since they should be backwards-compatible.

Please have a look at #8678 when you have a chance!

[habovh]

@benjamn sorry for the delayed reply. The solution you've came up with is really elegant and I believe it fits perfectly this use case, plus many more.

Following your previous comment I was about to suggest that the second approach was the best of the three because it felt like the closest to the current keyArgs-less behaviour. However your solution is orders of magnitude better and far more flexible.

I see #8678 has been merged, is it part of a release already?

[hwillson]

@habovh the keys work has been merged into release-3.5 and will be released as a beta shortly (probably 3.5.0-beta.8). You can follow along in #8554 to know when the next beta is published.

[benjamn]

Alright, #8678 ended up in @apollo/[email protected] (just published).

Thanks for the kind words @habovh—really glad you see/agree this idea has many uses, though I'm sure that won't be obvious to everyone. Then again, maybe it doesn't need to be, since this trick should stay mostly hidden until you need it.

[habovh]

@benjamn glad to see that! I may not have a chance to tryout a beta prerelease anytime soon but I'll definitely be using this once 3.5 gets released.

It has many uses like you said, but this is a pretty advanced feature anyway. By the time you're looking for this kind of flexibility setting up AC, you're probably well aware of the inner workings of Apollo Client.

That said, I think this deserves proper documentation under the advanced caching section: it would be a shame to go for a fully customized keyArgs function instead of your simpler and more importantly tested approach if it can yield the desired results.

[hwillson]

This should now be resolved in @apollo/[email protected]. Let us know if you notice any issues. Thanks!