-
Notifications
You must be signed in to change notification settings - Fork 12
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
base: main
Are you sure you want to change the base?
Conversation
✅ Deploy Preview for composite-schemas ready!
To edit notification comments on pull requests, go to your Netlify site configuration. |
decision to serve a field from more than one source schema is intentional and | ||
coordinated. | ||
|
||
```graphql counter-example |
There was a problem hiding this comment.
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?
I made a graph to try to help define the relationships of Entities, lookup, and nested look up vs direct vs root. Apollo FederationIn 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 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 FederationIn 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 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!
} |
With that in mind lets look at a more detailed example of just the users subgraph. Let's say we want to add some 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 However if I want to start referencing the Add a
|
As promised, I wrote down some thoughts about the relationship between |
@smyrick I'm not sure I follow your examples completely, but it makes sense to me that 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
} |
I guess what I am trying to devise is what is the need for 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
}
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:
Now that I am typing this all out I see the benefit of being explicit in requiring |
That has been my assumption all along, but it might be good to make that explicit. (We may want to allow no-argument fields that return singletons purely for namespacing purposes.)
The reason that won't work is because without |
On edge cases - In this prior example:
Is not Language essentially an entity? It can be retrieved by We could theoretically remove code from Language:
In this case Language doesn't contain its lookup key so it's perhaps more clearly not an "entity". Should this even be supported? Or would it be conceptually simpler to require that every lookup must leads to an entity? |
Yeah, I'm starting to come around to the idea that lookup fields should always return entities. And we may want to go even further and require every lookup field to match a specific key. That does make sense from a semantic point of view, because lookup fields are meant to always return the same object. And the only notion of sameness we have is based on keys. The main reason for relaxing this requirement was to support nested lookups. If we leave those out for now, the example above would turn into something like this: type Query {
text(languageCode: String! @is(field: "language.code"), id: ID!): InternationalizedText @lookup
}
type InternationalizedText @key(fields: "id language { code }") {
id: ID!
language: Language!
body: String
} If we do want to support nested lookups in the future, I think we could still enforce this requirement at every lookup level. So in the example below, type Query {
language(code: String!): Language @lookup
}
type Language @key(fields: "code") {
code: String!
text(id: ID!): InternationalizedText @lookup(with: "code: language.code")
}
type InternationalizedText @key(fields: "id language { code }") {
id: ID!
language: Language!
body: String
} |
Or maybe even a combination of type Query {
language(code: String!): Language @lookup
}
type Language @key(fields: "code") {
code: String!
text(languageCode: String! @is(field: "language.code") @require(field: "code"), id: ID!): InternationalizedText @lookup
}
type InternationalizedText @key(fields: "id language { code }") {
id: ID!
language: Language!
body: String
} |
There's some relevant discussion here as well: https://gist.github.com/martijnwalraven/c5ab26ecd7db905633938e0df3a85846?permalink_comment_id=5446733#gistcomment-5446733 |
This PR clarifies that lookup and keys are separate concepts that may overlap but do not have to.