Skip to content

Latest commit

 

History

History
241 lines (230 loc) · 12.2 KB

2023-07.md

File metadata and controls

241 lines (230 loc) · 12.2 KB

GraphQL CCN WG Notes - July 2023

Watch the replay: GraphQL Working Group Meetings on YouTube

First meeting, July 19th

Attendees:

  • Calvin Cestari
  • Young Min Kim
  • Stephen Spalding
  • Alex Reilly
  • Benjie Gillam

Notes

  • Stephen: Error handling was the main topic from the Relay folks - they're big on Fragment Modularity, and using those as error boundaries. They seem like they'd be okay with us moving forward. Just want to make sure that we don't break any future improvements in fragment modularity. We can't do much more on that front with GraphQL as it stands today.
  • Alex: So CCN errors in the standard errors array?
  • Calvin: we should ensure people are okay with this.
  • Stephen: and be explicit exactly what we are agreeing to.
  • Alex: null propagation rules. Benjie do you remember?
  • Benjie: nope… I think it was between just changing the nullability as-is or making everything between a ? and a ! non-nullable.
  • Stephen: current status is it bubbles until it hits an optional.
  • graphql/graphql-wg#1009 (reply in thread)
  • [discussion not captured; see YouTube instead]
    • ACTION - Benjie: sync the video to YouTube
  • [GraphQL.js PRs need attention from the GraphQL.js reviewers]
  • Alex: there's an issue where a second pass over the tree was required, but I've done a new implementation that doesn't require that.
  • Calvin: this was pegged against GraphQL v16, but v17 is in alpha now - should we be continuing against v16 or v17?
  • Benjie: I think you should always base it off the main branch, and if it makes sense to backport it to v16 once it is merged then that can be arranged.
  • Alex: GraphQL.js, Apollo iOS and iOS Codegen (Apollo)
  • Calvin: this seems to have got far and didn't stall for technical reasons
  • Alex: yeah, I changed jobs
  • Stephen: and I've not managed to do anything on it yet
  • Calvin: I'm working on Apollo-iOS, and adding incremental delivery, and I'd love to bring CCN with it as we upgrade to GraphQL v17.
  • Stephen: Would be great to have it late stage RFC for GraphQLConf
  • Alex: I think it's implemented in HotChocolate too?
  • Stephen: we should present it at a meeting to move it to the next stage
  • Benjie: where is the latest status
  • Alex:
  • Benjie: Not on-board with the try-catch behaviour. It can lead to unexpected outcomes
    • ACTION: Write up some examples clarifying the issues and seeking consensus.
  • Alex: a lot of that came from Relay's @required directive
  • Stephen: is there implementations of both approaches?
  • Alex: I think there is if you're willing to dig through the git history
  • ACTION - Calvin: pick a date for a WG meeting for the CCN working group before the next primary WG.
  • ACTION - Benjie: set up the WG meeting on Zoom/calendar
  • Stephen: we should invite Matt
  • Alex: are there others? Jordan?
  • Stephen: Matt said he'd represent Relay for this.
  • Alex: I intend to attend the Aug 3rd meeting.

Second meeting, July 26th

Attendees:

  • Alex Reilly
  • Calvin Cestari
  • Matt Mahoney
  • Benjie Gillam
  • Anthony Miller
  • Benoit Lubek
  • Young Min Kim
  • Stephen Spalding

Notes:

  • Introductions done (12:05pm)
  • Agenda
    • Benoit: Potentially adding list syntax to end of agenda
      • Shortcut syntax to lists
    • Calvin: Reducing the scope of CCN spec
  • Review Action Items
  • Error Handling (GitHub Discussion)
    • Anthony: Room for other options to eb considered
      • Exposing errors inline. Could make it easier for clients to parse the errors related to the data they care about
    • Young: Keeping Error Handling in this RFC could lead to further delays. Could we postpone this decision and progress to the next stage
    • Benjie: Now might be the right time to introduce it since it is a syntactic change.
    • Anthony: Separating the ‘larger’ discussions from the CCN spec
    • Calvin: These conversations are valuable but affect the whole spec so are maybe better suited to discussion outside of CCN
    • Alex: (to Matt) Is it correct that Relay can handle the existing error handling with CCN?
    • Matt: One of the worries is the Relay won’t use CCN
      • Relay crates a result type at the fragment/component boundary
      • @required which is either bubble to nearest nullable parent or throw an error
      • Recent proposal by Jordan for @result type - transform from get-or-null to ok of possibly null or value or error
      • If you want field-level error handling you need some mechanism for that but if it’s errors array that’s probably fine
      • With CCN, if we didn’t have (?), then when you’re accessing a field that is nullable and it’s null, how do you semantically tell the client that it was null because of an error vs it’s null because it happened to be.
    • Alex: Understanding of Relay error handling is whether it’s not written to the cache (couldn’t be confirmed)
    • Anthony: I make a similar point to this here
      • ‘Simple nullability’ seems to satisfy Relays ability to deal with CCN errors via the errors array
    • Stephen: Interesting how CCN is leading us to other parts of the spec that has room for improvement
      • Another piece of the spec (conflate null with error)
        • No good way to communicate the actual intent to the user
      • Might be better suited to another RFC but worth considering
      • Should we step back and consider what is the minimal scope in order to achieve the stated goal of CCN
    • Alex: Future of CCN could be to pare it back, evaluate the goal, and bring in complication as it’s needed.
      • Thank you to all that have taken notes and discussion we have plenty of reference material
    • Anthony: Introducing ? with CCN as an error catch makes it unintuitive and difficult to reason about
    • Stephen: In summary: Include ! and ? in simplest defined behavior
    • Matt: Simplest defined behavior takes on a different meaning depending on the context
      • If all fields in schema are nullable and ! is used on a field it’s unclear whether you want to handle errors at a particular level or “I don’t care”.
    • Anthony: Benjie’s summary of three cases/options highlights the inconsistencies in the current spec.
    • Matt: If we bubble to nearest ? it will be confusing
      • Placing ? above ! is unclear how everything between should be treated
    • Anthony: The distance between ? and ! makes it very unclear
    • Benjie: When writing fragments/components they probably don’t want to render something if it’s not present
      • But they may not be the one adding the ? in a parent field
    • Alex: The reason ! propagating to nearest ? is trying to replicate Relay’s fragment boundaries
      • FragmentBoundaries proposal may do a better job of solving this problem
    • Stephen: Keeping null propagation inline with the current behavior keeps the door open for better proposals to solve in the future
    • Matt: If I have a field with a ! how do I know the difference between the field or the parent being null
      • If we cannot know that difference any client than wants to do component nullability handling cannot
      • This means Relay will never be able to use CCN
    • Anthony: Doesn’t the error array solve that?
    • Matt: Relay’s lazy rendering could prohibit use of the errors array because it may simply never be rendered
    • Anthony: Is it fair to say that CCN would be off the table until try/catch or FragmentIsolation is in place
    • Matt: Sticking with current null bubbling means it could never be used
    • Alex: What if CCN was simply Relay’s directives?
    • Matt: If CCN matched @required(throw) then it would be able to be handled
    • Stephen: If Relay were not to do special processing for CCN but simply pass it on as if the server did it- could that work?
    • Matt: If it’s treated as an error then it’s infectious across fragments
    • Alex: (missed at 12:44pm)
    • Matt: null field would blow up other fragments because it wouldn’t exist
    • Anthony: Fragment Isolation would be able to mark the fragment as optional
    • Stephen: This isn’t really different to the current behaviour, just CCN isn't solving for it.
    • Matt: The server stating via it’s schema it saying a particular state could not exist
      • Server side errors are handled as unrecoverable
    • Alex: Error handling and null propagation are inherently linked because GraphQL uses nullability for error handling
      • Relay inconsistency with CCN is how to differentiate between the server-side irrecoverable errors vs. the client opting in and saying it could handle the error
    • Benjie: Would syntax stating the client could handle it a way forward?
    • Matt: Any error introduced by the ! without fragment isolation makes it untenable to use CCN
      • This is about keeping the integrity of the store
    • Alex: Could there be a particular carve out for CCN where errors for ! are separate and able to be identified
    • Matt: How about instead of nulling out the parent to ‘unset’ it
      • ‘Unset’ = the server stated the field is not present
    • Benjie: That tried to leverage the fact that nil and null are not the same where many programming languages do not make that distinction
      • Input values went down this path and provide for a sample reference
      • Probably a very high bar to get that kind of change into the spec
    • Young: If the client is using the ! then they’re really stating that they can handle it
    • Matt: If it were unresolvable then the CCN markers would be compiled out of what gets sent to the server
      • Relay has a store on the client which must remain consistent with responses
      • CCN breaks consistency with other responses
    • Matt: Relay does have mechanisms related to this such as @required
    • Young: Early on in CCN the idea was floated to say whether clients could simply handle this
    • Matt: With fragment isolation, if it makes this easier to handle, then possibly CCN could be used
    • Stephen: Timecheck.
      • Is there a way forward, conclusion?
    • Anthony: Maybe we shouldn’t aim for perfect
      • Many clients with have significant work to do
      • If the current blockers have a path forward with other proposals then maybe we should step past them
    • Matt: Big issue in getting to stage 2 is getting to this being the preferred solution
    • Alex: Needs to be see consensus on error handling and null propagation before things can be merged
    • Stephen: Foundational enough questions that indicates more discussion
    • Anthony: Should the resolution be that these aren’t the concern of CCN to move forward
    • Benjie: It seems that many of these concerns are related to fragment isolation
      • We should make sure that what is done with CCN does not block that proposal
      • **Action: (Matt): **Could fragment isolation resolve this in Relay
    • Matt: One option. Strip this way back and introduce only the !
      • Then sovle for problems as clients implement and face problems
      • Gives us concrete data to work with
    • Stephen: Action?: Pare down the spec
    • Benjie: Do we even have consensus on what ! just does
      • Alex: Day 1 proposal - normal propagation rules
    • Benjie: Lee wanted ? in for the work being done at Robinhood where they need to be able to make fields nullable where the schema stated they werent
    • Young: Happy to take on the PR work for implementation
    • Benjie: Modification on the error to state that it originated from a !
      • Message and Path exist but we can add an additional key
    • Calvin: Action: Update on the state of CCN
      • Problems we’ve faced
      • Paring down the spec
      • Are there any objections?
  • Nullability Assertions (GitHub discussion)
    • (Conversation inextricably linked with error handling)