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

[RFC] Null value #83

Merged
merged 1 commit into from
Oct 26, 2016
Merged

[RFC] Null value #83

merged 1 commit into from
Oct 26, 2016

Conversation

leebyron
Copy link
Collaborator

This proposal adds a null literal to the GraphQL language and allows it to be provided to nullable typed arguments and input object fields.

This presents an opportunity for a GraphQL service to interpret the explicitly provided null differently from the implicitly not-provided value, which may be especially useful when performing mutations using a per-field set API.

For example, this query may represent the removal/clearing of the "bar" field from thing with id: 4.

mutation editThing {
  editThing(id: 4, edits: { foo: "added", bar: null }) {
    # ...
  }
}

In addition to allowing null as a literal value, this also proposes interpretting the variables JSON to distinguish between explicit null and implicit not provided:

mutation editThing($edits: EditObj) {
  editThing(id: 4, edits: $edits) {
    # ...
  }
}

This variables results in the unsetting of bar

{ "edits": { "foo": "added", "bar": null } }

Finally, this proposes following the not-provided-ness of variables to their positions in arguments and input-obj fields

mutation editThing($editBaz: String) {
  editThing(id: 4, edits: { foo: "added", bar: null, baz: $editBaz }) {
    # ...
  }
}

Such that the three variables are semantically different:

  • {} The "baz" input field is "not provided"
  • {"editBaz": null} The "baz" input field is null
  • {"editBaz": "added"} The "baz" input field is "added"

@leebyron
Copy link
Collaborator Author

cc @dschafer @schrockn

@dschafer
Copy link
Contributor

This looks good to me!

@dylanahsmith
Copy link
Contributor

👍 this will definitely simplify the transition from REST to GraphQL for mutations, otherwise, a boolean field would need to be provided to distinguish between setting a field to be null and not changing the field.

@oguzbilgic
Copy link

👍
Seems like it has been some time. Is this still on roadmap?

@lprhodes
Copy link

I created a fork a short while ago that allows for this. I've not merged in the most recent commits yet but it's working in production.

https://www.npmjs.com/package/graphql-nullable

@OlegIlyenko
Copy link
Contributor

👍 Looks like this PR was open for quite a while now. Just wanted to give my option on this change.

I'm coming from scala background. I don't see an addition of null as a problem. It opens up quite a few interesting possibilities. In general statically-typed languages still make it possible to distinguish between undefined (property is not defined on object) and null (for example all scala JSON libraries have JNull or JsNull AST nodes). I tried to implement this proposal in the sangria and it looks good so far. If one does not care about distinction between null and undefined then they can be just treated the same (it happens pretty often). But on the other hand, if you need to distinguish between them, then this addition is very helpful.

Note: there is a potential semantic difference between the input value
explicitly declaring an input field's value as the value null vs having not
declared the input field at all. GraphQL services may optionally interpret these
two cases differently if it is advantageous.

I like this part and would keep it like this. So that it's up to the server implementation to decide whether to treat them differently or not. This would also mean that standard protocols (like introspection API or relay connection) can't relay on this mechanism, if I understand it correctly.

@joewood
Copy link

joewood commented Jan 19, 2016

Any reason why this RFC is restricted to mutations? Could it be applied to explicit input field arguments? E.g. a query being interpreted to a SQL clause where table.field is NULL.

@dylanahsmith
Copy link
Contributor

It doesn't look like this RFC is specific to mutations. Mutations are just the prominent example. The only change to the spec that mentions mutations is this added line:

For example, to represent deleting a field vs not altering a field during a mutation, respectively.

@dbackeus
Copy link

LGTM 👍

Coming from a ruby background not being able to send null to set an attribute to a nil value is certainly surprising.

@taion
Copy link

taion commented Mar 21, 2016

I'd be very happy to see this change land. We have a number of places where our GraphQL layer bridges to REST services that have special handling for explicit nulls in e.g. PATCH operations, and it would be much easier to bridge to those services if we have the option of using a null literal.

@akre54
Copy link

akre54 commented Apr 21, 2016

We often have NULLs in our database represent default values (i.e. a localizer with a NULL locale is the default translation). We've had to hack around this, but would be great for GraphQL to support natively.

@skosch
Copy link

skosch commented May 4, 2016

Not to be a bother, but what's the status on this? Are there still open questions?

@leebyron
Copy link
Collaborator Author

leebyron commented May 4, 2016

There are still open questions on how to resolve "nullability" with "optionality" once a value can be explicitly provided and be the value null. That also has implications on whether the value null should be replaced with a default value or not.

This is still open, but it's been lower priority relative to some of the other proposals we've been working the kinks out of.

@skosch
Copy link

skosch commented May 4, 2016

Thanks for the quick response @leebyron, that's good to hear.

Default values mean well, but my current practical problem, unless I'm doing it wrong, is that database updates happen based on changesets, and so right now it's impossible to know whether the client wanted a field nulled or simply left unchanged. (I'm probably beating one rotten carcass of a horse here, but since this is marked RFC I wanted to throw another vote in the hat for bona fide nulls.)

@taion
Copy link

taion commented May 4, 2016

BTW, I think in most other contexts that I've seen, "value is required" (may not be undefined) and "null is permitted" are treated as orthogonal concepts.

One place this comes up the most strongly is for e.g. a PATCH update (for a partial update of an object). In such a case, certain non-nullable fields may still not be required (i.e. to have their values not be changed).

@leebyron
Copy link
Collaborator Author

leebyron commented May 5, 2016

Yeah, to be clear here the question isn't "is it a good idea to introduce a null value" the question is "is it worth the additional introduced complexity"

Splitting the concepts of optionality and nullability actually adds a lot of cascading changes to GraphQL, so while in isolation it's perfectly easy to make a case for why having them separate is fine, what we actually want to be doing here is striving to maintain GraphQL's simplicity whenever possible.

The costs of maintaining simplicity is that there are often corner-cases where GraphQL feels awkward, but the value is that the primary cases feel simple. I understand that may feel like a slight - it certainly doesn't feel like a corner-case when you're the one experiencing it, but it is the reality that most mutations in GraphQL use the action pattern rather than the CRUD pattern, so "setting a field to null" is actually a case we haven't had to deal with in production yet.

And also to be clear, I do think having a null literal is a good idea - I do want to make enabling CRUD style operations more reasonable, I would just like to do so with as little collateral damage to simplicity as possible.

@skosch
Copy link

skosch commented May 5, 2016

That was thoughtfully written. Thanks. :)

Not sure if it's been discussed elsewhere – could you sidestep those issues if you defined some Relay.NULL constant for us to use, instead of messing with the JS null literal?

@taion
Copy link

taion commented May 5, 2016

I don't think Relay.NULL can be the answer, since input types can't be union types.

The CRUD stuff just comes up when migrating existing systems to GraphQL, I guess.

@skosch
Copy link

skosch commented May 5, 2016

Ah, forgot about that. Never mind then.

IvanGoncharov pushed a commit to IvanGoncharov/graphql that referenced this pull request Jun 17, 2017
This proposal adds a null literal to the GraphQL language and allows it to be provided to nullable typed arguments and input object fields.

This presents an opportunity for a GraphQL service to interpret the explicitly provided null differently from the implicitly not-provided value, which may be especially useful when performing mutations using a per-field set API.

For example, this query may represent the removal/clearing of the "bar" field from thing with id: 4.

```
mutation editThing {
  editThing(id: 4, edits: { foo: "added", bar: null }) {
    # ...
  }
}
```

In addition to allowing `null` as a literal value, this also proposes interpretting the variables JSON to distinguish between explicit null and implicit not provided:

```
mutation editThing($edits: EditObj) {
  editThing(id: 4, edits: $edits) {
    # ...
  }
}
```

This variables results in the unsetting of `bar`

```
{ "edits": { "foo": "added", "bar": null } }
```

Finally, this proposes following the not-provided-ness of variables to their positions:

```
mutation editThing($editBaz: String) {
  editThing(id: 4, edits: { foo: "added", bar: null, baz: $editBaz }) {
    # ...
  }
}
```

Such that the three variables are semantically different:

 * `{}` The "baz" input field is "not provided"
 * `{"editBaz": null}` The "baz" input field is `null`
 * `{"editBaz": "added"}` The "baz" input field is `"added"`
theorygeek added a commit to goco-inc/graphql that referenced this pull request Sep 11, 2018
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.