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

Configuring keyArgs for types rather than fields? #8021

Open
LeonardDrs opened this issue Apr 22, 2021 · 7 comments
Open

Configuring keyArgs for types rather than fields? #8021

LeonardDrs opened this issue Apr 22, 2021 · 7 comments

Comments

@LeonardDrs
Copy link

LeonardDrs commented Apr 22, 2021

Hey,

I've seen that we can specify merge function for types rather than fields. But since typePolicies.Type.keyArgs does not exist - which seems normal since we are talking about a type, not a query field - am I forced to use Query.fields for pagination?

To be more precise:
I have an album containing multiple tracks with this schema:

type Album {
  id: ID!
  tracks(first: Int = 20, after: String): AlbumTracksConnection!
}

type AlbumTracksConnection {
  edges: [AlbumTracksEdge!]!
  pageInfo: PageInfo!
}

type AlbumTracksEdge {
  cursor: String!
  node: Track
}

When I want to paginate, I could use the following query:

query Album($albumId: String!, $nb: Int, $after: String) {
  album(albumId: $albumId) {
    id
    tracks(first: $nb, after: $after) {
      pageInfo {
        hasNextPage
        endCursor
      }
      edges {
        cursor
        node {
          id
          title
        }
      }
    }
  }
}

with this fetchMore:

fetchMore({
  variables: {
    albumId: "13",
    nb: 3,
    after: data.album.tracks.pageInfo.endCursor,
  },
})

and this typePolicies:

typePolicies: {
  Album: {
    fields: {
      tracks: relayStylePagination(),
    },
  },
}

Everything works perfectly.

But then, if I want to configure the typePolicies for type over fields, since AlbumTracksConnection is always paginated, I'd try:

typePolicies: {
  AlbumTracksConnection: relayStylePagination(),
}

which does not work as the Album.fields.tracks policy keeps its original keyArgs value.

I would then need to specify Album.fields.tracks.keyArgs to false in the typePolicies for the pagination to work. But then I would loose the benefit of being able to target types over fields name.

So the question is:
Do we want to be able to handle pagination/keyArgs via type, like we are able to handle merge? It would ease the pagination for types.
Or am I doing something wrong with the pagination?

Probably related: #7070 (comment)

PS: In real life, I'd use a Paginable type like so:

new InMemoryCache({
  typePolicies: {
    Paginable: relayStylePagination(),
  },
  possibleTypes: {
    Paginable: ["AlbumTracksConnection", "AnythingConnection", ...],
  },
});
@dylanwulf
Copy link
Contributor

I'm not sure this would make sense outside of your use case. GraphQL allows you to return the same type from multiple different places, each with its own unique set of arguments. It would be impossible to guarantee that the type will always have the same arguments.

@LeonardDrs
Copy link
Author

LeonardDrs commented Apr 23, 2021

It would be impossible to guarantee that the type will always have the same arguments.

Sure but we could always still specify a specific keyArgs in a different place where the type has different arguments.

I'm not sure this would make sense outside of your use case.

I thought pagination was pretty common, what is specific from my use case?

@LeonardDrs
Copy link
Author

@benjamn Hello, sorry to bother you with this ping.
I'd be grateful if you could provide me your insight on this situation.

For now I've developed a graphql-codegen plugin to create the typePolicies object so I can manage, but I'm still wondering if the pagination could be handled via types instead of fields.

@sebastienbarre
Copy link

Having worked on pagination in the past few days, I'm not sure how your scenario would work.
relayStylePagination() needs access to args.after and args.before to work.
These are not defined at the AlbumTracksConnection level, they are defined at the field level, in this case, tracks(first: $nb, after: $after), that's why relayStylePagination() is associated to the tracks field.
Case in point, you could have two different fields returning an AlbumTracksConnection, but with different arguments for each fields.

@benjamn
Copy link
Member

benjamn commented Oct 20, 2021

@LeonardDrs I see where you're coming from, but I agree with @dylanwulf and @sebastienbarre that the details don't quite work out.

Another problematic case (simpler than pagination): what if you're replacing the value of an Animal-typed field that used to be Dog with a Cat object, but Dog and Cat types define different keyArgs? Which one should we use?

The merge function in type policies (#7070) is a special case where I was able to convince myself all the details would work. On the reading side, we often need to compute the field key (using keyArgs) in order to find the right field value, so we don't have the luxury of using the field's value to determine the field's key.

I'd love to see that GCG plugin if you can share it!

@LeonardDrs
Copy link
Author

Hey, thanks all for your replies.

I had dream that we could have specify keyArgs on each fields, if needs be, like so:

new InMemoryCache({
  typePolicies: {
    Paginable: relayStylePagination(),
    Cat: {
      keyArgs: [...]
    }
  },
  possibleTypes: {
    Paginable: ["Cat", "Dog"],
  },
});

Keeping the default keyArgs for Dog, but yeah this might get weird.

The GCG plugin was published under our private repo and fit our needs/api.
It creates a basic typePolicies object. One could use annotation on query argument to keep them as keyArgs but we do not have this need, at least yet.
Here is what it looks like https://gist.github.com/LeonardDrs/4b976b1303b538fc389141fd9b737bef

@Azerothian
Copy link

Azerothian commented May 12, 2022

type WhereQuery {
  active: Boolean!
}
type Item {
  id: ID!
  children(first: Int = 20, after: String, where: WhereQuery): ItemConnection!
}

type ItemConnection {
  edges: [ItemEdge!]!
  pageInfo: PageInfo!
}

type ItemEdge {
  cursor: String!
  node: Item
}

Type Policy

const itemConnectionTypePolices = {
  ...relayStylePagination(() => ["where"]), // without normalisation
  fields: {
    edges: {
      fields: {
        node: {
          fields: {
            children: itemConnectionTypePolices,
          },
        },
      },
    },
  },
};


const memoryCacheOptions = {
  typePolicies: {
    Item: {
      fields: {
        children: itemConnectionTypePolices,
      },
    },
  },
};

https://www.apollographql.com/docs/react/caching/cache-configuration/#disabling-normalization

  1. Without normalisation you cant run two queries on the same object and one will override the other. But it is the only way data is store in the parent object.
  • This following will override each other without normalisation
    • Item.children.edges[0].node.children(where: {active: false}).edges
    • Item.children.edges[0].node.children(where: {active: true}).edges
  1. With normalisation (ie. supplying keyArgs) the following active: false queries will interfere with each other
  • Item.children(where: {active: false}).edges
  • Item.children(where: {active: true}).edges.node.items(where: {active: false}).edges
  1. Writing the code above is way funky and can be easily cleaned up if we could do it by a Type instead of path.

Ideas

  • Allow for a function to define the actual key instead of providing fields and variable names.
    • We will need the parent object and child obj plus return values.
    • if we could get the full path to the queried object would be a bonus win
  • Allow for the capability to define whether the type will be normalised or not.
  • Allow for type based definitions of policies as a function or an object

@sebastienbarre it should be the type of the field that would be the referring type, which at that point you could supply the args for normalisation / manipulation.

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

No branches or pull requests

6 participants