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

Gql arguments are required if defined in keyArgs #6973

Closed
tadhglewis opened this issue Sep 5, 2020 · 7 comments
Closed

Gql arguments are required if defined in keyArgs #6973

tadhglewis opened this issue Sep 5, 2020 · 7 comments

Comments

@tadhglewis
Copy link

tadhglewis commented Sep 5, 2020

I'm unsure if this is intended behaviour and I can't seem to find much documentation on this feature as the documentation is being worked on (#6711).

I am using relayStylePagination / keyArgs like so

Intended outcome:

  • keyArgs should not require the argument to be defined in the gql query, if a key arg is defined in the InMemoryCache but not the specific gql, the key arg should be ignored
  • The error message (unless this is made redundant by above change) needs to be changed to an appropriate message for keyArgs

InMemoryCache

const cache = new InMemoryCache({
    Query: {
      fields: {
        contacts: relayStylePagination(["mine", "scopes", "propertyId"])
      },
    },
  },
});

gql

query GetContacts($mine: Boolean $cursor: String, $scopes: [ScopesEnum!]) { ... }

This causes issues if you have a large application with multiple gql files defined for the same field but with different arguments - my current work around is to write a function for the keyArgs and filter them depending on the arguments like so

const filterKeyArgsToArgs = (keyArgs: any) => (args: any) =>
  args ? keyArgs.filter((keyArg: any) => args.hasOwnProperty(keyArg)) : null;

const cache = new InMemoryCache({
    Query: {
      fields: {
        contacts: relayStylePagination(filterKeyArgsToArgs(["mine", "scopes", "propertyId"]))
      },
    },
  },
});

Actual outcome:

I get the following error (which is worded for key fields) when the propertyId key arg is defined in the InMemoryCache but not in the gql

Invariant Violation: Missing field 'propertyId' while computing key fields

How to reproduce the issue:

  • Define keyargs for a field in the InMemoryCache
  • Have a gql query for that field which uses those arguments - works fine
  • Have a gql query for that field which does not use those arguments - does not work (gets above error)

Let me know if you need to create a demo

Versions

  System:
    OS: Linux 5.4 Ubuntu 18.04.5 LTS (Bionic Beaver)
  Binaries:
    Node: 12.18.3 - /usr/bin/node
    Yarn: 1.22.5 - /usr/bin/yarn
    npm: 6.14.7 - /usr/bin/npm
  Browsers:
    Chrome: 85.0.4183.83
    Firefox: 80.0.1
  npmPackages:
    @apollo/client: ^3.1.4 => 3.1.4
@zsaraf
Copy link

zsaraf commented Sep 10, 2020

Having this exact same issue, let me know if you've come up with a solution! Doesnt seem like relayStylePagination is ready for production, so I'm curious as to why updateQuery was deprecated.

@tadhglewis
Copy link
Author

@zsaraf I believe relayStylePagination and all the other pagination helpers are more guides on how to guide your pagination helper?

Either way they should still be usable (and the fact that this affects all keyArgs! not just relayStylePagination).

Here is my solution:

const filterKeyArgsToArgs = (keyArgs: any) => (args: any) =>
  args ? keyArgs.filter((keyArg: any) => args.hasOwnProperty(keyArg)) : null;

const cache = new InMemoryCache({
    Query: {
      fields: {
        contacts: relayStylePagination(filterKeyArgsToArgs(["mine", "scopes", "propertyId"]))
      },
    },
  },
});

Basically you can pass a function into relayStylePagination / keyArgs which will receive the current arguments, then filter out the keyargs which aren't in the arguments. Let me know if you have any trouble with it, it's what i've been using in production for the past couple weeks after removing updateQuery.

@zsaraf
Copy link

zsaraf commented Sep 17, 2020

Thank you @tadhglewis!

@benjamn benjamn self-assigned this Oct 1, 2020
benjamn added a commit that referenced this issue Oct 1, 2020
The keyArgs:[...] configuration for field policies should be able to
include arguments that are optional, while still specifying an ordering of
all possible key arguments for serialization purposes. When an optional
argument is not provided, it will simply not be included in the serialized
storeKeyName suffix, and no exception will be thrown.

Should address #6973.
@benjamn
Copy link
Member

benjamn commented Oct 1, 2020

@tadhglewis I think allowing optional arguments in keyArgs is the right solution here! Fix incoming: #7109

@benjamn benjamn added this to the Post 3.0 milestone Oct 1, 2020
benjamn added a commit that referenced this issue Oct 2, 2020
As suggested by @tadhglewis in #6973.

The [`keyArgs: ["someArg", "anotherArg"]` configuration for field policies](https://www.apollographql.com/docs/react/caching/cache-field-behavior/#specifying-key-arguments)
should be able to include arguments that are _optional_, while still
specifying an ordering of all possible key arguments for serialization
purposes. When an optional argument is not provided, it will simply
not be included in the serialized `storeFieldName` suffix, and no
exception will be thrown.
@benjamn
Copy link
Member

benjamn commented Dec 7, 2020

@robertsmit
Copy link

robertsmit commented Dec 8, 2020 via email

@tadhglewis
Copy link
Author

Can't test whether this is or isn't working as I'm no longer working on the project I was using this on.

I'm assuming this is fine to close this issue now. @benjamn ?

@github-actions github-actions bot locked as resolved and limited conversation to collaborators Feb 15, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants