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

Lookup / Key relationship clarification. #164

Open
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

michaelstaib
Copy link
Member

@michaelstaib michaelstaib commented Feb 6, 2025

This PR clarifies that lookup and keys are separate concepts that may overlap but do not have to.

Copy link

netlify bot commented Feb 6, 2025

Deploy Preview for composite-schemas ready!

Name Link
🔨 Latest commit 0299962
🔍 Latest deploy log https://app.netlify.com/sites/composite-schemas/deploys/67a4ea6dc1341b00098fc6e5
😎 Deploy Preview https://deploy-preview-164--composite-schemas.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

package.json Outdated Show resolved Hide resolved
decision to serve a field from more than one source schema is intentional and
coordinated.

```graphql counter-example

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If Product starts out without keys (and is shared between subgraphs) and we want it to become an entity, is there a path to do that without an atomic multi-subgraph change?

@smyrick
Copy link
Contributor

smyrick commented Feb 6, 2025

I made a graph to try to help define the relationships of Entities, lookup, and nested look up vs direct vs root.

Key and Lookup Diagrams

Apollo Federation

In Apollo Federation, you can actually do a look up directly to a type ie not through the query node and so your graph can have abitrary entry points coming from the outside that are accessed by the Router

Screenshot 2025-02-06 at 12 02 37 PM

Product Subgraph

type Query {
  listAllProducts: [Product]
}

type Product @key(fields: "id") {
  id: ID!
  title: String
  description: String
  variants: [Variant]
}

type Variant @key(fields: "id product { id }") {
  id: ID!
  product: Product
  colorway: String
  price: Float!
}

Reviews Subgraph

type Review @key(fields: "id") {
  id: ID!
  title: String
  body: String
  product: Product
}

type Product @key(fields: "id") {
  id: ID!
  reviews: [Review!]
}

type User @key(fields: "id", resolvable: false) {
  id: ID!
}

Users Subgraph

type Query {
  user: User
}

type User @key(fields: "id") {
  id: ID!
  username: String!
}

GraphQL Federation

In the new spec, there is actually no longer arbitrary entry points as far as the subgraph schema goes. Everything must come from a root field somehow (if hidden) and defined with lookup, even if nested.

Screenshot 2025-02-06 at 12 04 22 PM

Product Subgraph

type Query {
  listAllProducts: [Product]
  productById(id: ID!): Product @lookup @inaccessible
  variantById(id: ID!, productId: ID!): Variant @lookup @inaccessible
}

type Product @key(fields: "id") {
  id: ID!
  title: String
  description: String
  variants: [Variant]
}

type Variant @key(fields: "id product { id }") {
  id: ID!
  product: Product
  colorway: String
  price: Float!
}

Reviews Subgraph

type Query {
  reviewById(id: ID!): Review @lookup @inaccessible
  productById(id: ID!): Product @lookup @inaccessible
}

type Review @key(fields: "id") {
  id: ID!
  title: String
  body: String
  product: Product
}

type Product @key(fields: "id") {
  id: ID!
  reviews: [Review!]
}

type User @key(fields: "id", resolvable: false) {
  id: ID!
}

Users Subgraph

type Query {
  userById(id: ID!): User @lookup
}

"An user account in our system"
type User @key(fields: "id") {
  id: ID!
  username: String!
}

@smyrick
Copy link
Contributor

smyrick commented Feb 6, 2025

With that in mind lets look at a more detailed example of just the users subgraph. Let's say we want to add some Cart type that is unique per user so it is nested but an user can have many saved carts/lists. The only way to resolve the current checkout cart for the user is to first fetch the user type

type Query {
  userById(id: ID!): User @lookup
}

type User @key(fields: "id") {
  id: ID!
  username: String!
  activeCart: UserCart
  savedCarts: [UserCart]
}

type UserCart {
  id: ID! # Not unique by itself, must combine with user id
  items: [Variant]
}

In it's current state I don't need to make the UserCart an entity, I am not adding the @key directive, and I don't need a way to access it from other subgraphs to add fields. So this schema is fine

However if I want to start referencing the UserCart type in other subgraphs either just returning an id or adding additional fields to it then the router will need some way to access just the UserCart type

Add a discountCode field

Say we want to add this subgraph for a discount code

type UserCart {
  currentDiscount: Float
}

And resolve this query

query {
  userById(1) {
    username
    activeCart {
      currentDiscount
      items { title price }
    }
  }
}

Then do that that, you must some how access an entity so that you can agree on what the cart you are modifying is.

We could do that a few ways

Make UserCart a direct root entity

type User @key(fields: "id", resolvable: false) {
  id: ID!
}

type UserCart @key(fields: "id user { id }") {
  id: ID!
  user: User
  currentDiscount: Float
}

type Query {
  userCart(id, userId): UserCart @lookup @inaccessible
}

Adding a reference through the User

This is what we would want to do, but this fails. This can't work because then User.activeCart would have different fields depending on what subgraph you hit and not be fully satisfiable. Also how would you resolve the User.savedCarts?

type User @key(fields: "id") {
  id: ID!
  activeCart: UserCart @shareable
  savedCarts: [UserCart] @shareable
}

type UserCart {
  currentDiscount: Float
}

Nested lookup by only 1 id

So then you are still forced to make UserCart an entity to declare that it has different fields, but you can't do this either because you need to have the user id to get the full type

type Query {
  useById(id): User @lookup
}

type User @key(fields: "id") {
  id: ID!
  cartById(id): UserCart @lookup @inaccessible
}


type UserCart @key(fields: "id") {
    id: ID! # Not unique by itself, must combine with user id
  currentDiscount: Float
}

Lookup returns non-entity

This is what we talked about today at the meeting. I think based on composition rules alone this is technically ok, but it does seem extra confusing to me. How am i supposed to know just looking at this subgraph that UserCart is actually a type that has additional fields elsewhere? This does give a path for the Router to make two distinct request to resolve this query but I think it could be better

query {
  userById(1) {
    username
    activeCart {
      currentDiscount
      items { title }
    }
  }
}

⚠️

type Query {
  useById(id): User @lookup
}

type User @key(fields: "id") {
  id: ID!
  cartById(id): UserCart @lookup @inaccessible
}

# How do I know this is a special type and will possibly conflict with other subgraphs?
type UserCart {
  currentDiscount: Float
}

Lookup must return a type with @key aka entity

By forcing the use of @key this means users know that the type can differ across subgraphs and it a place that could cause composition errors. This allows us to still be nested too it also leaves it open for a root resolver in the future if you want.

It forces users to think about how can I truly make my types unique and have proper keys and you have to think about that if you are using @lookup

type Query {
  useById(id): User @lookup
}

type User @key(fields: "id", resolvable: false) {
  id: ID!
  cartByID(id): UserCart @lookup @innaccessible
}

type UserCart @key(fields: "id user { id }") {
  id: ID!
  user: User
  currentDiscount: Float
}

@martijnwalraven
Copy link

As promised, I wrote down some thoughts about the relationship between @lookup and @key. A gist seemed like the most readable option: https://gist.github.com/martijnwalraven/c5ab26ecd7db905633938e0df3a85846

@martijnwalraven
Copy link

martijnwalraven commented Feb 10, 2025

@smyrick I'm not sure I follow your examples completely, but it makes sense to me that UserCart would have to be an entity if we wanted multiple subgraphs to contribute fields to it. I don't think that means every @lookup necessarily needs to return an entity though. If nested lookups are needed to get to an entity, not every lookup field along the way needs to return an entity.

Sensible examples are difficult to come up with, but something like this should be valid I would think:

type Query {
  language(code: String!): Language @lookup
}

type Language {
  code: String!
  text(id: ID!): InternationalizedText @lookup
}

type InternationalizedText @key(fields: "id language { code }") {
  id: ID!
  language: Language!
  body: String
}

@smyrick
Copy link
Contributor

smyrick commented Feb 11, 2025

I guess what I am trying to devise is what is the need for @lookup on a field that does not return an entity. Your example is a simplified one so let's use that.

type Query {
  language(code: String!): Language @lookup # What if this directive was gone?
}

type Language {
  code: String!
  text(id: ID!): InternationalizedText @lookup
}

type InternationalizedText @key(fields: "id language { code }") {
  id: ID!
  language: Language!
  body: String
}
  • Language is not an entity and merely serves as a name spacing or grouping type

  • Given Language is not an entity, the router should never use just the Query->language(code:) path so to resolve data it will always be the path that ends entity Query->language(code:)->text(id:).

    • What this the root level @lookup is doing is specifying a path to how to get to the InternationalizedText finder though
  • The router query planner is going to have to do some work to figure out what entity resolver to use. It will have some info like "I know I need to fetch the InternationalizedText.body field from this subgraph and I have the key @key(fields: "id language { code }"). So I will start from Query and figure out what possible path leads me to a Lookup that has all those keys. Does that mean if we change the schema to this planners are going to have still tie look up args to keys and know that have all the right ones?

type Query {
  languageByCode(code: String!): Language @lookup
  languageByShortCode(shortCode: String!): Language @lookup
}

type Language {
  code: String!
  shortCode: String
  text(id: ID!): InternationalizedText @lookup
}

type InternationalizedText @key(fields: "id language { code }") {
  id: ID!
  language: Language!
  body: String
}

I also agree that the spec should be flexible to allow nesting and maybe I am conflicting the best practices that I would want to encourage which would to be to have clear matching root level looks ups for entities to make planning easiest as possible for planners, but we can't enforce that convention up everyone either

So in that case would it be better to:

  • Require that Language is an entity
    • This makes less sense now thinking about it as other services might not have a unique way to idenfity it
  • Be explicit that Router entry points should only use paths that are marked with @lookup even when traversing nested types, allowing us to annotate as you describe
  • Not require that you have a full path annotated with @lookup and Instead planners just "find" one resolver that does have a @lookup for a given entity type, and then figure it out from there.

Now that I am typing this all out I see the benefit of being explicit in requiring @lookup full defined paths, even on non-entities.

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

Successfully merging this pull request may close these issues.

5 participants