Skip to content

Latest commit

 

History

History
156 lines (145 loc) · 8.74 KB

2023-08.md

File metadata and controls

156 lines (145 loc) · 8.74 KB

GraphQL WG Notes - August 2023

Watch the replays: GraphQL Working Group Meetings on YouTube

Primary

Agenda: https://github.com/graphql/graphql-wg/blob/main/agendas/2023/08-Aug/03-wg-primary.md

Determine volunteers for note taking (1m)

  • Benjie

Action items:

  • Will be closing a number of them in September, so please comment on them if they're still relevant

CCN:

  • Error handling and null propagation are the main concerns, but they seem like they're not specific to CCN alone - they have overlap with other proposals such as the fragment modularity proposals.
  • Our proposal is to strip the CCN proposal right back to just the ! syntax and simple enforcing of non-nullability.
  • Reason for being here: gather feedback on the idea of stripping CCN back to the beginnings.
  • Anthony: there was a lot of discussion around the open issues, and came to a realization that a lot of this isn't tide to CCN and can be addressed in other ways. We're proposing keeping ! but skipping the ? for now, but I'm not sure there's consensus.
  • What we seemed to have consensus on was around the behaviors: removing the discussion around bubbling/catching/etc by just using the existing logic.
  • Calvin: RFC is just the ! and the spec will follow. Lee's original reason for raising the ? was to mirror the !, which we believe has value. We don't want to keep the "catching" behavior right now.
  • In my opinion ? is best as a nullability modifier, not null propagation boundary.
  • Martin: I'm excited about ? because it means we can add more ! to the schema which is more pleasant by default; but I'm happy so long as we're not preventing it from a future update.
  • Anthony: we're now past the experimentation phase and we've learned a lot. Discussions are around the behaviors of edge cases, specifically when ? just changes the nullability or whether it's as a catch for an error raised by a ! (like a try/catch). The fact we couldn't come to a determination on this is what's encouraged us to drop the ? for now and just keep the ! modifier. My thought is that we could add an @catch directive or some other approach in future and just have ? and ! be simple modifiers leveraging the existing algorithms.
  • Ivan: I like the simplest approach with ! and ?, but one problem was that Relay will strip them and send the query to the server, the same will happen for a lot of proxies. If the proxy removes ? before sending the query upstream that would mean the proxy would have to implement the semantics (and it can't?). ! is stricter, but ? cannot be stripped because the error may already have been thrown.
  • Anthony: so are you arguing for adding ? from the start to help address this feature as early as possible?
  • Ivan: yeah, I think it's needed.
  • Someone talked about Relay stripping ! and ?
  • Benjie: I think Relay need to strip ! because of the lack of fragment boundaries, but they wouldn't need to strip ? I think?
  • Ivan: clients don't know if server supports the syntax
  • Benjie: yes, the SDL and introspection must indicate that the syntax is supported.
  • Martin: can we handle this in the upstream schemas by making everything nullable?
  • Ivan: I don't think that's feasible.
  • Ivan: I think feature detection could be a solution. Proxy wouldn't support ? until every downstream schema supported ?.
  • Anthony: I'm concerned about large stacks of feature flags. Is it better to version the schema? That way Oct2023+ with CCN would always support the syntax
  • Benjie: there's a few reasons not to do this, but the biggest is that it would prevent MAYs being indicated. I think these feature flags would only be required where things cannot be detected in other ways: types, fields or directives on the introspection schema.
  • Michael: I agree, we also have to deal with servers that don't allow aliasing!
  • Benjie: I think we MUST have it indicated in SDL and introspection in order for clients to support code generation, or editors like VSCode to know whether the ! or ? is specifically allowed or not.
  • Alex: I disagree. If I need a new thing in GraphQL, I ask the guy next to me and then I implement it on my client. It's not that complicated - I just ask the guy if CCN is supported before I try and use it. It seems unnecessary to block all clients/servers from using CCN just to support these big complex clients/servers.
  • Ivan: Proxies can strip ! before sending to upstream, so no need for feature probing. Not the same for ? though. So ? is blocked until we can indicate feature flags, but I don't think ! is.
  • Anthony: whether or not clients can work with specific servers in your company; the spec states "this is valid legal syntax" and having a client use that and the server reject it (because it doesn't support it) seems like a violation of the spec.
  • Benjie: this would also be needed for fragment arguments.
  • Matt: I don't mind if the server doesn't support it - it can just fail.
  • Ivan: GraphQL is like TypeScript: if you write something you get immediate feedback if it's valid or not, but you don't get that in JS or Python. You shouldn't need to run the query to know if it will work, but it's a developer experience thing.
  • Matt: it's not a blocker for getting it into GraphQL.js; it's only a blocker for getting it into the spec.
  • Ivan: strippable features and non-strippable features are different.
  • Matt: a client could compile (maybe) the query to remove all the fragment arguments.

SDL Omissions (Martin)

  • Working on Apollo Kotlin. Widely used.
  • User writes queries in .graphql files, and puts the schema into the codebase (.graphqls). When we do so, we get errors: 'include' type tried to redefine existing directive 'include' type.
  • This is because IntelliJ assumes that the SDL will come without the built in directives, without the introspection types, etc.
  • We need this information at Apollo for good reasons, but some tooling requires that it's not present.
  • The spec states that the built-in scalars MUST be omitted for brevity from the SDL.
  • We have the same but MAY for directives.
  • But we don't have the same for introspection types.
  • I propose a small edit so that tools like IntelliJ will know to handle this situation.
  • Matt: either you include none, or you include ALL. This would allow you to have a GraphQL server that doesn't support Floats.
  • Matt: this would help with the @specifiedDirective
  • Benjie: String has to be supported (for the introspection schema to work), so we could say that if String was present then you've opted into the verbose SDL format. Currently the introspection types don't appear because __schema and __type don't get returned from introspection, and thus there's no entrypoint to them in the SDL.
  • Ivan: do we have to print __typename everywhere though?
  • Ivan: we could have explicit fields, implicit printable fields, and implicit non-printable fields.
  • Matt: Relay has added __id field which normally aliases to id but not for special things. The __id is also implicit. We have a cacheKey type field and a fetchKey field which aren't necessarily the same (e.g. fetchKey might require hundreds of bytes).
  • Ivan: can we merge this as is? Otherwise everything is snowballing!
  • Benjie: I think it's a MUST that the introspection types are not included in this proposal, but we should have a separate RFC that allows printing the full schema including introspection fields, built-in scalars, etc. I think there'll be a lot of prose in this latter one but not a lot of algorithm changes.
  • Ivan: GraphQL.js buildSchema ignores the introspection types and just adds our own, but really there should be two functions, one for building a client schema that has everything the SDL indicates and nothing more.
  • Even if we change the spec and validation, the implementations would still need to have additional changes.
  • Martin: the reason we need the full SDL is so we can tell if the server supports certain introspection fields.
  • Matt: path 1 is to have a full description of the schema that we can validate against. Path 2 is "for every new feature in the spec we add something to SDL/introspection to indicate this".
  • Ivan: feature flag for syntax is easy because there's no interdependency.
  • Michael: can you not solve this by using a config file based on the features detected. We use a heavily annotated schema where we add schema directives to indicate which features were supported.
  • Martin: every implementation would then have it's own directives potentially, whereas SDL is standard and well defined.