Skip to content

Latest commit

 

History

History
126 lines (112 loc) · 9.61 KB

2023-09.md

File metadata and controls

126 lines (112 loc) · 9.61 KB

Client Controlled Nullability WG - September 2023

This was an informal meeting of people interested in discussing the True Nullability Schema idea.

Watch the replay: GraphQL Working Group Meetings on YouTube

Attendees:

  • Jordan Eldredge (Meta)
  • Ryan Holdren (Meta)
  • Itamar Kestenbaum (Meta)
  • Calvin Cestari (Apollo)
  • Alex Reilly (Yelp)
  • Young Min Kim (The Trade Desk)
  • Benoit Lubek (Apollo)
  • Stephen Spalding (Netflix)
  • Matt Mahoney (Meta)

Agenda:

  • Introductions
  • Jordan: Overview of True Nullability Schema
  • Discussion of unsolved problems within True Nullability Schema
    • Deprecating fields
    • Simple clientsS
    • Clients without granular reads
      • Fragments or assessors
    • Moving schema to true nullability with multiple clients
  • Discussion
    • Does this influence what we want from CCN?

Notes:

  • True Nullability idea came about from discussions between Relay team
    • Shielding the user from null or explicitly handle them
  • Being able to mark cache items as ‘errored’ is important
    • Ability to distinguish between error and null
  • Assumption that null bubbling exists to ensure type safety in the response
  • True Nullability brings the idea of combining errors and returned fields to still ensure type safety
    • Once we forgo null bubbling we can stop needing to have schemas with all fields nullable
  • Maybe moving away from JSON as the response could let us have an error state for fields that have errored
    • That might be a pretty big lift getting it into the spec so maybe separating that from this is better
  • There is a tension between existing spec which does not distinguish between server and client, and this proposal which pushes some of the responsibility to the client
  • Jordan: Maybe not. We’re just offering a mode of execution that makes a softer assertion about type safety. Clients are then free to make a different contract with their users (you will never get an error)
  • Deprecating fields - does nullability actually help?
  • Simple clients do exist
    • Often for scripts or tools that update indexes/etc
  • While making a nullable field non-nullable is technically non-breaking change, it will break existing clients since they will observe much more destruction due to null bubble
  • Alex: I wouldn’t be mad if we end up bypassing CCN and end up with True Nullability Schema
  • Jordan: A question we should ask, is would ! be an anti-pattern if we do end up in a True Nullability Schema world. Would we end up wanting to lint against !, since it’s making assertions that are unsafe? That’s the only reason not to continue pursuing CCN.
    • I believe TypeScript has a config option that makes ! (non-null assertion operator) forbidden or at least a warning.

26th September meeting

Attendees:

  • Alex Reilly
  • Stephen Spalding
  • Ernie Turner
  • Benoit Lubek
  • Benjie Gillam
  • Young Min Kim
  • Calvin Cestari (late)

Notes:

  • Previous action items:
    • Add CCN to next WG meeting: Done
    • Do spec changes for the stripped down proposal: Young completed this, Alex owes a review
  • Benjie: Review of GraphQL Conf, there were a few discussions of CCN at the conf, Stephen and Ernie gave talks.
  • There was a slide in Stephen’s talk that inspired me.
  • Benjie put together a writeup, interested in the interrobang
  • CCN is needed because everything is nullable by default, but Jordan pointed out that we have no way to know the true nullability of fields in the schema
  • Null represents errors, not necessarily that a value is legitimately null
  • Jordan proposed that we could remedy that. Interesting shenanigans like having multiple schemas
  • Benjie is suggesting something like an error boundary that indicates that this field can be null, but only if there is an error, and not under normal execution
  • Syntax is still up in the air, interrobang isn’t great
  • If we were starting again, then we might have fields be error boundaries by default, but that ship has sailed
  • Is CCN still necessary if the error boundary syntax is in the spec?
  • It seems like no
  • On the need to not bubble errors: You could fix that by adding question marks to every field in the spec
  • Relay needs this to preserve their normalized store
  • Could be something like the @no_bubbles_please that Stephen described in his talk
  • Young: I’m concerned that CCN took a long time to get to this point, and the new thing will take the same amount of time to get merged
  • If we have true nullability, then the usefulness of CCN decreases, but we don’t have it yet, so I hesitate to pause CCN
  • Stephen: Earlier this summer I was interested in trimming back CCN to get something in incrementally
  • Those incremental steps are fine as long as they’re getting us where we want to go
  • These new ideas will take some time, but if we hand users CCN now, is there a chance that CCN becomes eventually harmful once we have true nullability, or do we end up in a place where CCN becomes a feature that we suggest against using
  • One thing we could do is take CCN back to being a directive, implemented in clients only and get it implemented in different clients
  • Ernie: If it’s only a directive then it isn’t useful for simple clients.
  • CCN is also still useful where you’re a third party user of a service and you can’t change the schema you’re using.
  • Benjie: One of the great things about the exclamation point is it doesn’t need server support. Clients can simulate bubbling or use different behavior if they like, and strip out exclamation points.
  • If we can avoid adding unnecessary features to the spec, we should do that.
  • In response to Young’s concerns, the new proposal isn’t very complicated. Collecting errors is a new concept, but otherwise not that complicated.
  • Ernie: Getting into your proposal Benjie, if I say something below this level is an error, but want to stop it at some point I can, but if I turned off null bubbling then it wouldn’t anymore
  • Benjie: Interrobangs and @do_not_bubble are complementary features, they don’t conflict
  • Ernie: If the interrobang was a feature, I think I’d put it everywhere in the schema today. What are the use cases for selectively using it vs spamming it everywhere?
  • Benjie: Matt Mahonie said at the conference that defaults matter. Ideally the interrobang would be the default behavior. We could propose a change in default behavior, but it would be difficult.
  • Effectively what do_not_bubble would do is treat every field like an error boundary by default.
  • Alex: Is there any chance that they accept a change in defaults in a major version bump?
  • Benjie: Probably not.
  • Stephen: It feels like it’s worth thinking about what the perfect future version of GraphQL is so we can work in that direction.
  • Ernie: It feels like there should be a way to opt into do_not_bubble for the entire service as the owner of the service.
  • Benjie: I think a number of the relay folks would agree with you. There’s the concern about old clients not functioning with newer servers.
  • Young:
  • Benjie: If you put the directive on the introspection, it could respond with a different schema that shows clients the true nullability fields. New clients can opt into receiving this different set of information. It would be a different flow through the execution algorithm.
  • But to be clear, if we do a no_bubbles, then we’ll have to have a different execution path anyway.
  • Ernie: You could have the server execute everything as if it had no_bubbles
  • Benjie: Doing that reduces your compatibility, but breaks spec. You could still do it though.
  • Stephen: Do you think it’s possible to achieve this where the indicator for the client is only on introspection? If we can limit it to introspection queries, then it’s basically only a development time thing, and not a runtime thing. It seems like it should be possible to have a mode switch.
  • There’s a separation between the SDL and the way that’s interpreted by introspection.
  • Benjie: We could make it so the schema has an interrobang on it.
  • Yes we can do introspection only for interrobang, but not for null bubbling.
  • Stephen: This feels like it’s really close. The next step could be a tweak on Benjie’s RFC to include what we’ve discussed here.
  • Changes:
    1. In the SDL, we add a question mark, this has the behavior of the field being expressly nullable
    2. Anything that does not have a question mark is not supposed to be nullable ie if there is no value, then there MUST be an error in the errors array.
    3. Introspection part: A flag that would indicate that a client is aware of this behavior and gets introspection that’s aware of this nullability
  • Stephen: For code gen, interrobang types would generate to non-null types
  • Benoit: Apollo clients would make interrobanged fields non-nullable, and if there were errors we could maybe find a way to handle those gracefully, but responses with errors would most of the time be unusable.
  • Benjie: Relay would be capable of more sophisticated error handling with their component error boundaries.
  • This provides more ways for clients to handle these issues.
  • Stephen: In your code gen, interrobang fields could be nullable, and then there’s info in the schema that doesn’t get used by code genned, or you could have a mode in codegen that makes those types non-nullable. Developers might want the choice.