Allow passing a function to keys in Graphcache config

[dios-david]

I'm currently experimenting with Graphcache in an existing application. I find it really cool but there is something which bugs me a lot :) I see tons of warnings like this one:

Invalid key: The GraphQL query at the field at `Query.posts({"limit":10})` has a selection set, but no key could be generated for the data at this field.
You have to request `id` or `_id` fields for all selection sets or create a custom `keys` config for `PaginatedPosts`.
Entities without keys will be embedded directly on the parent entity. If this is intentional, create a `keys` config for `PaginatedPosts` that always returns null.

This suggests to add PaginatedPosts: () => null to the keys config if I'm okay with embedding the entity to the parent entity, which is exactly what I want in most of the cases.

My problem is that the GraphQL API I'm using have hundreds of types and I would like to avoid adding all of them one-by-one to the keys config if that's possible. I know this is just a warning in dev mode only, but it causes lot of noise.

What do you think about adding support for passing a function to keys which would receive the typeName as param so we could programmatically generate the keys config? Like this:

Config with object (existing solution):

cacheExchange({
  keys: {
    Todo: data => data.name,
    Product: data => data.productId,
    Post: () => null,
    Author: () => null,
  },
});

Config with function (proposal v1):

cacheExchange({
  keys: (typeName) => {
    switch(typeName) {
      case 'Todo': return data => data.name;
      case 'Product': return data => data.productId;
      default: return () => null;
    }
  },
});

Config with function (proposal v2):

cacheExchange({
  keys: (typeName, data) => {
    switch(typeName) {
      case 'Todo': return data.name;
      case 'Product': return data.productId;
      default: return null;
    }
  },
});

Apollo has (had?) a similar solution as well, I think that's quite useful for devs.

[kitten]

We chose the less intrusive option here as we don't want every code path in every application to be disturbed by this, basically 😅

We could add a global modifier but seeing as this is related to pagination, we do have exceptions for these warnings in place for type names that comply with the Relay pagination spec https://github.com/FormidableLabs/urql/blob/8224ed0c5dbc31d62102254d50d9c1584c84c601/exchanges/graphcache/src/operations/write.ts#L308

I guess I can look into this though, but we consider these changes carefully as most of the time we don't decide on features by edge cases per user, sorry 😅✌️

[dios-david]

I just copied this example warning from another discussion as my app was not running at that point, so let's forget the actual type name in my example :)

But it's good to know there is a regexp in place for keyless types 👍 What about making that regexp configurable? That would help me as well.

most of the time we don't decide on features by edge cases per user

That perfectly makes sense, but I'm not sure this is an edge case here. The case here is we need to add these return-null functions for each keyless types if we want to silence these warnings. But in my understanding it's totally valid to have keyless types in a GraphQL schema, actually in my case the schema contains 175 object types (so excluding scalars/enums/inputs) and only 21 of them contains a field with an ID type. In worst case scenario (if all the types are actually used by the application) I would need to add 154 return-null functions to the keys object (and maintain it) if I wanted to silence the warnings. Anyway, this is just an example but I'm sure you get the point here.

[eden]

Sorry to necro this discussion, but I stumbled upon it via google. Here's an interim hack that seems to work for this case without any change to urql itself:

      cacheExchange({
        keys: new Proxy(
          {},
          {
            get(_target, prop) {
              switch (prop) {
                case 'Todo': return data => data.name;
                case 'Product': return data => data.productId;
                default: return () => null;
              }
            },
          }
        ),
      }),

[kjr-lh]

Note that if you do this proxy solution, you will have to include any types that you try to use in cache.invalidate calls.

The cache invalidate code will try to use the keys config to get the id, and will error when the response is null

[eden]

Yes, we hit this. I should note I stopped using this in my code and opted for explicitly listing out the types here.

[JoviDeCroock]

You can bypass that with

      cacheExchange({
        keys: new Proxy(
          {},
          {
            get(_target, prop) {
              switch (prop) {
                case 'Todo': return data => data.name;
                case 'Product': return data => data.productId;
                default: return () => x.id || x._id;
              }
            },
          }
        ),
      }),