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

Defaulting to semantic nullability #17

Open
flux627 opened this issue Jun 8, 2020 · 3 comments
Open

Defaulting to semantic nullability #17

flux627 opened this issue Jun 8, 2020 · 3 comments

Comments

@flux627
Copy link

flux627 commented Jun 8, 2020

This guide has been a breath of fresh air with regard to nullability handing, in stark contrast from the official GraphQL standpoint on the matter:

https://graphql.org/learn/best-practices/#nullability
https://medium.com/@calebmer/when-to-use-graphql-non-null-fields-4059337f6fc8

And countless articles like this one, in agreement:
https://medium.com/expedia-group-tech/nullability-in-graphql-b8d06fbd8a3c

But I prefer the practice of catching errors at the resolver level, and either allowing the query to fail, or replacing the affected fields with "blank" or error-informing ("This comment failed to load") data. For me, this is far more manageable than dealing with non-semantic nulls everywhere.

Would it make sense for this tutorial talk about this, since it is a deviation from the GraphQL standard?

@eapache
Copy link
Contributor

eapache commented Jun 9, 2020

I have lots of thoughts on this, but none of them are confident - while I still agree with you in theory, we've run into so many problems with non-nulls in practice that I don't feel comfortable recommending their use. The official recommendations have a bunch of trade-offs, but they're likely the right choice for the majority of use cases.

@flux627
Copy link
Author

flux627 commented Jun 9, 2020

There seems to be some dissonance here- this sentiment is not reflected in the tutorial. Would it make sense, then, to update the tutorial? Looks like @swalkinshaw is the author to the semantic null bits.

@eapache, I would love to hear your non-confident thoughts on the matter. Can you share what specific problems in practice you've run into?

@eapache
Copy link
Contributor

eapache commented Jun 9, 2020

Would it make sense, then, to update the tutorial?

Our thoughts on this are evolving quite a bit, and we prefer not to change the tutorial too drastically on a regular basis. We'll definitely update it if we come to a confident conclusion.

I would love to hear your non-confident thoughts on the matter. Can you share what specific problems in practice you've run into?

Given a field which our internal data model declares must always be present, you’d think it would be a safe bet to make the relevant GraphQL field non-null. We’ve collected a significant number of exceptions to this rule:

  • When the user using the app does not have permission to access the object. Apps are expected to know their own access scopes and not request fields they don’t have access to, but they cannot easily track the permissions of the active user.
  • When a race condition between resolving a query and mutating the data results in an inconsistent view of the world within the execution of a single query.
  • When the database has invalid data. We sometimes have model-level validations without database-level validations, so there are some fields which ought to have no null values but in practice have a bunch.
  • When the backing data is moved from our core system into a transparent micro-service which can fail or time out independently.

In a mostly-nullable schema, any of these problems would be isolated to the single failing field which would return null, permitting the rest of the query to succeed. This is a pretty important property for overall system resiliency.

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

2 participants