Split default value loop from field-type resolver loop #6119
Conversation
|
This pull request is being automatically deployed with Vercel (learn more). 🔍 Inspect: https://vercel.com/keystonejs/keystone-next-docs/2NvuW3t2RUkwNs8XLxmUHZzfzynr |
🦋 Changeset detectedLatest commit: 26e3606 The changes in this PR will be included in the next version bump. This PR includes changesets to release 1 package
Not sure what this means? Click here to learn what changesets are. Click here if you're a maintainer who wants to add another changeset to this PR |
|
This pull request is automatically built and testable in CodeSandbox. To see build info of the built libraries, click here or the icon next to each commit SHA. |
timleslie
changed the title
timleslie
force-pushed
the
resolve-data-updates
branch
from
de45bca to
26e3606
Compare
timleslie
changed the title
| if (input === undefined) { | ||
| // No-op: This is what we want | ||
| return () => undefined; | ||
| } | ||
| if (input === null) { | ||
| // No-op: Should this be UserInputError? | ||
| return () => undefined; | ||
| } |
There was a problem hiding this comment.
Why is this being here better?
There was a problem hiding this comment.
It's just about visibility. I want to have a single place to point at for this discussion, and it frees up the four other resolvers to be simpler.
There was a problem hiding this comment.
Could you not do this or write it in such a way where the value used to decide what to return is based on the value passed to the function rather than the input from GraphQL? The resolve function passed to relationship field input resolvers currently accepts some input that the field's input resolver calls it with and decides what to do based on that rather than using the GraphQL input directly and this breaks that.
There was a problem hiding this comment.
I don't want to bike shed this too much because what I really want is for all this code to evaporate even further, so as long as this works I want to move forward and have these specific lines go away as part of broader changes I'm working on.
Decouples these two loops so that we can provide better error messages moving forward.
This also lifts the
nullinput case out of the individual relationship resolver functions so we can have the discussion around what the correct behaviour for this input should be (currently a no-op).