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

Distinguish between null and missing in args #179

Open
dwwoelfel opened this issue Oct 15, 2019 · 4 comments
Open

Distinguish between null and missing in args #179

dwwoelfel opened this issue Oct 15, 2019 · 4 comments

Comments

@dwwoelfel
Copy link
Contributor

dwwoelfel commented Oct 15, 2019

Is there some way to distinguish between the user passing in null and the user passing in nothing for an arg?

I have a use case for an update mutation that requires me to distinguish between missing and null. If the value was provided as null, then the underlying field in storage needs to be nulled out, but if the value is missing, then the underlying field should remain unchanged.

Example:

type Person {
  id: Int!
  firstName: String
  lastName: String
}

input PersonInput {
  id: Int!
  firstName: String
  lastName: String
}

type Mutation {
  updatePerson(person: PersonInput): Person
}
mutation {
  updatePerson(person: {id: 1, firstName: "Daniel", lastName: "Woelfel"}) {
    id
    firstName
    lastName
  }
}

# results in {id: 1, firstName: "Daniel", lastName: "Woelfel"}
mutation {
  updatePerson(person: {id: 1, firstName: "Dan"}) {
    id
    firstName
    lastName
  }
}

# should result in {id: 1, firstName: "Dan", lastName: "Woelfel"},
# not {id: 1, firstName: "Dan", lastName: null}
@andreas
Copy link
Owner

andreas commented Oct 20, 2019

Sorry for the slow response. I have two ideas for how to solve this:

  • Inspect at the Graphql_parser.field passed to the resolver (via resolve_info.field).
  • Make a custom scalar type that has a specific value when null is passed. This would result in a type like [`Null | `Not_null of string] option Schema.Arg.arg_typ, where None means not present and Some `Null means null was passed (you don't have to use polymorphic variants, I just used it here for simplicity).

The second approach is a bit of a hack to implement outside of the library though. I haven't thought it through, but maybe it should be a native feature of OGS.

@ozanmakes
Copy link
Contributor

I'm not very familiar with the GraphQL spec, but this discussion could be relevant: teamwalnut/graphql-ppx#26 (comment)

If this is correct, the current behavior of raising an error when nullable variables are missing from the variables dictionary does not align with the spec.

@andreas
Copy link
Owner

andreas commented Nov 26, 2019

I'm not very familiar with the GraphQL spec, but this discussion could be relevant: baransu/graphql_ppx_re#26 (comment)

If this is correct, the current behavior of raising an error when nullable variables are missing from the variables dictionary does not align with the spec.

Can you elaborate on what you mean by this? And feel free to open a separate issue -- at first glance it does not appear related to this issue (though I could be mistaken) 😄

@ozanmakes
Copy link
Contributor

ozanmakes commented Nov 26, 2019

@andreas It's more of a question I guess, I don't know what behavior is correct and I'm curious whether a possible OGS solution to this issue would be relevant to my issue.

query getUsers($first: Int, $after: Cursor) {
  users(first: $first, after: $after) {
    nodes {
      id
    }
  }
}

If I call this query with { "first": 10 } I get this error back:

{
  "errors": [
    {
      "message": "Missing variable `after`"
    }
  ],
  "data": null
}

while { "first": 10, "after": null } works.

In fact, this was the way graphql_ppx worked. It would force me to declare every variable and construct something like { "first": Some(10), "after": None }, and it would encode None with null.

After switching to graphql_ppx_re because of newer BuckleScript support I noticed my null (None) values started to get dropped, which ocaml-graphql-server did not like.

During the subsequent discussion in the issue the author said he made this change to work around an issue with Sangria, but also that this change is compliant with the spec.

Now I realize that my issue is about variables passed to the query, which is not this issue is about. But I'm still curious about your opinion on this, is this something I should attempt to solve on the client side, or at ocaml-graphql-server side?

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

No branches or pull requests

3 participants