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

Introduce Lookup Directive. #30

Draft
wants to merge 10 commits into
base: main
Choose a base branch
from
Draft

Introduce Lookup Directive. #30

wants to merge 10 commits into from

Conversation

michaelstaib
Copy link
Member

@michaelstaib michaelstaib commented Apr 11, 2024

This PR renames the @entityReslolver to @lookup and introduces the agreed characteristics to the specification.

TODOs:

  • Semantics of lookups vs patches RFC document (Michael).

When an entity resolver returns an interface all implementing types are inferred
as entities.
Lookup fields may return object, interface or union types. In case a lookup
field returns an interface or union type all possible object types are

Choose a reason for hiding this comment

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

Since unions don't have to share any fields I am unsure whether we should support it (as it definitely complicates things).

Copy link
Member Author

Choose a reason for hiding this comment

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

We do ... in the composition we make sure that all union members are entities.

version: Int # NOT an entity resolver.
personById(id: ID!): Person @entityResolver
version: Int # NOT a lookup field.
personById(id: ID!): Person @lookup
Copy link

Choose a reason for hiding this comment

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

another use case would be for batching but we would then need some semantics to match the plural arg to a field (using @is?)

type Query {
  person(ids: [ID!]!): [Person] @lookup
}

... but this would really mess up with @require arguments so for simplicity maybe we should not allow it?

Copy link
Member Author

Choose a reason for hiding this comment

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

This is being solved transparently ... so there should be no batching fields.

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.

None yet

5 participants