Skip to content

Latest commit

 

History

History
162 lines (145 loc) · 21.9 KB

2021-11-04.md

File metadata and controls

162 lines (145 loc) · 21.9 KB

GraphQL WG Notes - November 2021

Watch the replay: GraphQL Working Group Meetings on YouTube

Agenda

  1. Agree to Membership Agreement, Participation Guidelines and Code of Conduct (1m, Lee)
  2. Introduction of attendees (5m, Lee)
  3. Determine volunteers for note taking (1m, Lee)
  4. Review agenda (2m, Lee)
  5. Review previous meeting's action items (5m, Lee)
  6. TSC Elections for 2022, open nominations process (5m, Lee)
  7. Progress update on Client Controlled Nullability (10m, Alex Reilly)

Determine volunteers for note taking (1m, Lee)

  • Benjie
  • Stephen

Review agenda (2m, Lee)

  • Added "Guidelines for speaking on behalf of the project" topic.

Review previous meeting's action items (5m, Lee)

Guidelines for speaking on behalf of the project (5m, Lee)

  • PR
  • It's not uncommon for OSS projects to put political messages at the top of their README files, but these are often not universally agree with and can cause conflict.
  • This is written up in the TSC document because the buck stops with the TSC (though the WG should generally agree on the direction).
  • We tend to keep a firewall between the foundation board and the technical board, so the foundation doesn't really have say on this.
  • Jayden's comment was thoughtful and one of the reasons that we have not merged the statement as it currently stands; everyone should read the statement as is.
  • Robert: if someone wants to add a political message like this, how would they apply to do so? Bring it to the WG?
  • Lee: yes, bring it to this WG; we can discuss and determine if it should stay on the person's personal websites, or if we all agree with the message then we can indicate where that messaging should go. In general I don't think we'll add much of this messaging, but I imagine maybe something like net neutrality might be something we post. That's my take.
  • Robert: to summarize: someone with one of these concerns would open an issue, the TSC would call a vote on the issue, and if it couldn't be covered then it'd come up at the next TSC group meeting.
  • Lee: If it's straightforward and urgent then async. If not urgent, take it to a meeting. Neither straightforward nor urgent then bring it to WG.
  • Lee: why this is a TSC rather than WG thing is because we use the word "vote" and the TSC have the voting power; however we want to avoid the TSC having a differing opinion to WG attendees, so I'd usually bring these to the WG meetings so everyone can weigh in and we can try and achieve consensus.
  • Lee: I've framed this generally as unopinionated rules. I get Jayden's take though - it may come across as an invitation to add political speech, but we're not intending that.
  • Matt: if it becomes an issue where many people keep bringing political opinions every month then we should clarify then. Given we want it to be a long-lived policy, there may be a future world where GraphQL spec may find itself in a political situation - starting from a place of seeming neutrality offers the easiest paths. I'm not sure what topics we'd find consensus around.
  • Lee: a blanket ban would be an alternative; but if someone changed how open source works or there was a major battle between tech giants that might change the meaning of the MIT license, or something like that then we might want to add some political messaging about it.
  • Stephen: if we can change it later, why not just say we support no political messaging now?
  • Matt: that'd involve changing the fundamental agreement.
  • Lee: [agree]
  • Lee: we'll add a little extra messaging to discourage the undesired behaviours.

TSC Elections for 2022, open nominations process (5m, Lee)

  • TSC Elections issue
  • Nomination form
  • TSC Election Process
  • TSC is there to provide formal votes. Also a set of people we can give admin rights to - maintenance control over GitHub, etc.
  • We set this up last November; I picked everybody (because I was the solo BDFL before that!). We set up 2 year staggered terms, so half the seats expire each year. I did a random selection of who the first 5 would be, 3 of them here today: Benjie, Ivan and Matt.
  • First we collect nominations - there's a google form linked in the issue, from my twitter, etc. If you're interested please fill out the form, at the end of the month the remaining 5 TSC members will do the "ranked choice" style vote, and at the end of the year we'll find out who the new people will be - some seats may stay the same, some may be new faces.
  • This topic is here in case anyone has any questions.
  • Hat tip to Brian for helping craft the excellent TSC documentation.
  • Matt: I believe even people who are up for re-election must also submit their own self-nomination to be up for re-election.
  • [Benjie lost connection again. Zoom crashed twice.]
  • Lee: minimum couple hours a month. Good to try and keep on top of spec PRs/etc. Hopefully everyone can help with that, but I'm likely to ask TSC members for help first, then WG after that. Probably 2-5 hours a month I guess.
  • Hugh: it's really great to see TSC members attend the meetings - it's good to see they care. It's great you're calling that out as encouraged.
  • Lee: it's a delicate line to walk because we could only invite people from the WG which would create a bit of an insular community. Some of the TSC members who don't attend the WGs do offer other perspectives which is valuable.
  • Brian suggested a monthly TSC call separate from the WG; I pushed back against that because I don't want a wall between the TSC agenda and the WG agenda.
  • Alex: you'd also be expected to read and review the proposals that are up for a vote, right? How much time does that take?
  • Lee: the general idea is that we go for broad consensus rather than TSC consensus - that's why things tend to take a while, but it helps avoid divergence so we don't often need TSC votes. Keeping abreast of what's going on in the technical environment is valuable, but you can just read the PRs as they come up if you prefer. Just attending these meetings is sufficient to stay up to date.

Progress update on Client Controlled Nullability (10m, Alex Reilly)

  • Action Item. Relevant links within
  • The "human readable written proposal" was merged earlier. PRs exist for spec changes and GraphQL.js changes - looking for reviews on these.
  • Young has been experimenting with an integration with The Guild's code generator, so hopefully this will be working soon.
  • Is the volume of reviews important? Is inviting people to review helpful?
  • Lee: I think that is helpful. Somewhere you can keep a running list of open questions and their state would be really helpful so people know what things are still being discussed versus settled.
  • e.g. when the type of the field is more complex than null/non-null (e.g. lists) - do we want to rule that out, etc?
  • e.g. the set of unknowns - the codegen one will be interesting to see - will it make codegen significantly more complicated?
  • Jordan:
  • Matt: we need to figure out for Relay if this is the same as @required for relay (in which case the server will never see it because it gets compiled away as a client-only consideration) or is it something that relay passes to the server and gets fed back
  • Jordan: we're going to continue to present the facade that they're scoped to the fragment, which requires us to never send conflicting info to the server.
  • Lee: Relay's had experimental non-null/@required for ages. Does it have the opposite? You can't make a field nullable client-side, you have to do it on the server.
  • Michael: schema stitching is an example use case of this - e.g. making the root fields nullable where schemas stitch together so you don't lose information on error.
  • Alex: codegen seems to work. We want to ensure that combining ! and @skip doesn't cause issues, but other than that it should be quite smooth. Clients may have to do some work to make their caches work nicely.
  • Lee: would be great to list out all the question, even if they're easy to answer - even building out an FAQ would be valuable. skip/include and ! shouldn't interact I think, but noting that explicitly and having test cases for it would be wise.
  • Any other major open questions?
  • Jordan: from the Relay side we have three flavours of this, one that does the spec proposal, one that logs for something suspicious, and one that actively throws. Our usage is 97% the throw behaviour. How we've seen it adopted has diverged quite a lot from how it's written in the spec currently.
  • Matt: because we're including fails when the server hits an error, there's an error path and an error message in the fails response so it feels like a throw.
  • Jordan: 90% of the use cases we have will not be able to adjust to the new syntax. It's not a major issue though.
  • Alex:
  • Alex: The upgrade path for marking list items required is doable (not straightforward but doable). The other consideration is caching; I'd like to give people some idea of how that's going to work (even though it's out of scope for the proposal).
  • Lee: I think it'd work in a similar way to Relay.
  • Matt: any time there's errors, nothing gets written to the cache. This is a reasonable behaviour if a little heavy-handed. At facebook our native handlers throw on error - we do not give the client the ability to access that data, even if there is a partial response; not that that's what you should be doing but it works for us at Facebook.
  • Lee: seems reasonable - you're interpreting this as a throw variant.
  • Lee: maybe the entire document explodes if a ! is failed? Or maybe you explicitly have to have a ? higher up?
  • Jordan: we've thought about: what if you have rust/TS style syntax where ! is an assert and ? is nullable, and you use them together. The throw variant doesn't make sense on the server because it's so destructive; it only makes sense on the client because of React's error boundaries.
  • Lee: that maps well with other conversations I've had around this. At Robinhood the discussion has been around error boundaries: what happens if something blows up and part of the response is gone - I'd like to say that's okay, stop blowing things up at this point even if the client has to handle the null case. Kind of the opposite intent to this PR.
  • Stephen: before we implemented this at netflix we considered different options; one of them signifies that you're indicating that something semantically should be nullable or not - if it's returned null then there will be an error in the errors array. It allows clients to bubble up the errors, but it also allows clients to handle how that works. Hard to make it backwards compatible - maybe you need to add a strict mode, or add a maybe. Would we ever consider adding a maybe type?
  • Matt: generic types 😬
  • Lee: had this proposal been raised long-long ago before the type system was solidified (like 2013/14) then it might make sense that everything on the server could not include nulls unless explicitly indicated. We decided not to do that because it was too big of a footgun to have something keep blowing the entire request up all the time. We are where we are with nullable by default though; seeing how well "strict mode" went for every other language that attempted it makes me shudder. Adding a ! should add an error into the errors list. You take the field type off the field and then you modify it, and then that's the type you use for value completion/execution/etc.
  • Jordan: might clients choose like Relay would the throw semantic. Is there a larger population of clients who may want that behaviour.
  • Lee: Facebook's clients have the stop process if errors contains anything. Defining errors in your domain (like Sacha has done) can draw a nice line between expected errors and unexpected errors which go the throw route.
  • Jordan: if the spec talks about bubbling null, but my client throws, that would be confusing to me.
  • Lee: isn't that already the case for non-null fields though? I can see the difference between a schema saying thai field will never be null versus the schema saying it's nullable and then you adding the non-null assertion.
  • Jordan: we have three ways: it's not an exception, it is an exception, or it is an exception but we can recover.
  • Ivan: rather than just replacing the type with non-nullable, we could change the way we do errors - e.g. return a partial result inside the error. Gives us a way to indicate something outside the ordinary but without losing the data.
  • Lee: that feels orthogonal - an ! might make it explicitly the case that the client is indicating that they are okay with losing data if the field is null.
  • Lee: sounds like you're suggesting the answer is "more information" - and throwing out the data because of the assertion failing is undesirable. A question mark would explicitly indicate that you want to catch the error here and trim the response to that point. Would we want to interpret the use of a questionmark as "please don't blow up my response - I actually do want a partial result" - indicating that the error was "caught" at the given location so your client can decide to allow partial results if you opted in with a question mark - e.g. if all errors were caught errors then I can proceed, but if any were not caught then I'll throw them out. This would imbue new behaviour, but just adds an annotation to the error object.
  • Alex: Definitely interesting, but is it useful?
  • Lee: it's additional information, right? This question comes up - if a field is null is it null because the value is null or because an error happened and it was the lowest nullable field? You can figure it out by seeing if an error was thrown below that field, but its a slightly confusing thing to do. It might be interesting to give us this way to indicate "client intent to handle an error" versus a nullable field in the schema.
  • Matt: if we had a first class error type, would the proposal change? Would ! and ? change? If there was a way to encode "this field could be an error" or if we said for any field if there's an error thrown, rather than putting it in the errors array we return an error object at that point in the response and that error "interface" could be extended. Would this first class error type mean that a null coming up in the response where we didn't want it to be null - would that result in an error object with more data inside? Would having first class errors radically change how we approach this problem? Should we spin up what first class errors might look like?
  • Lee: I think an error object in the response rather than sideloaded would meaningfully change how clients deal with the response. Could lead to confusing potential results, and it would occlude fields with a particular name e.g. message might be a field to avoid so that it doesn't conflict with Error.message. What a client wants is that if I say it's a string then it's a string (not a null) so I don't have to refine the maybe type so I can start doing work with it. I'd expect you to either have explicit throw/catch points - maybe first class errors would bubble until they hit one of these explicit points.
  • Brian: JS / JSON seem to have an oversized influence on how nulls are thought of in GraphQL. What if instead of null we excluded stuff if it was missing. Similar argument for errors; JS has its own semantics for errors and it's wildly different in other languages, so I worry about adding this stuff to the spec.
  • Stephen: Matts suggestion has two parts
    • a little more structure on different types of errors
    • where do they go - in the graph or sideloaded?
  • we could consider these separately. I don't tend to see the error path used much; is there a reason for that? Using the path in the client to attach errors to where they happened in the graph (via clients/tooling) - I tend to only see the path used by humans debugging it.
  • Lee: you're right - all the information exists to merge data/errors using the path information - it's a doable client-side transform.
  • Matt: clients not doing that could be a good argument that it's not needed.
  • Lee: for the developer ergonomics every field is now a left/right type and you'd have to unpack all the way along the path which would be really annoying. An explicit "error boundary" might be what is desired here.
  • The major question is: should this meaningfully change server behaviour, or is it so the client can do post-processing?
  • Jordan: throw versus bubble semantics; if we decide that client-side nullability were somewhat different, ! is an assert that throw which is caught at the client-defined error boundary.
  • Matt: Relay's interesting since the error boundaries are fragments, and fragments don't exist.
  • [additional discussion about possibilities]
  • Matt: maybe "?" as an error boundary might want to be introduced separately from "!".
  • Lee: I think we have to think about them at the same time.
  • Benjie: I agree that (!) and (?) need to be solved at the same time. Initially (?) made me uncomfortable, and now I understand better why. It could cause issues with your data model to come up that wouldn’t otherwise come up. If the underlying thing would have thrown an error anyway, it probably should still throw an error. These are slightly different behaviors.
  • Lee: hesitant to have two different things - server-defined vs client-defined error. To take this to first principles - how will people want to use this?
  • ACTION - add to our list of unanswered questions - it's not necessarily null, more an error boundary?
  • Jordan: it's hard for me to untangle what this 90% means when we're in React with error boundaries; does this map well to other situations where we don't have this kind of primitive.
  • Lee: a sophisticated client will want maximal information because it does post-processing anyway. A shell script calling curl probably doesn't want additional information - you just want a final payload. This is a trade off when the final payload is destructive.
  • Brian: I like the framing in terms of "what do developers want". I think it's the ability to stop making breaking schema changes - both making a field nullable or non-nullable is breaking due to the way code generation works.
  • Lee: the value there being that rather than the server changing it the client can change it but they take responsibility for it.
  • Jordan: the user problem we're solving is different - we have pervasive nullability; but our developers don't want to write 10,000 null checks - half the code is null checks and that's the problem that we want to go away.
  • Stephen: same motivation for us.
  • Alex: us too. We used to have loads of wrapper types and all the codegen types get throw away - not useful because we have to null check every field.
  • Lee: we have kind of the same, but we want to be able to balance good data (financial data we have to be super careful with), we've considered having two schemas - one with non-nulls everywhere and another where you can opt in to handling the nullability. The "?" helps us solve this problem, which is why I found this compelling. If a client cares about a partial response they should be able to describe that intent. If a client want to blow up when there's nulls then they should be able to describe that intent.
  • Hugh: are you considering advancing this without considering lists, or still looking at this?
  • Alex: talking to Netflix and Relay they've not ran into any problems with this yet; we could add it but I've been erring to the side of me doing nothing.
  • Jordan: we've not encountered a need for this due to the throw semantic. If you want to bubble you might want to make sure that there's an indication at every level leading to that.
  • Alex: if you or apollo have a good usecase for this I'd love to hear it.
  • Stephen: soft signal from Netflix on that - having it exist is better than nothing.
  • Lee: we might decide that we don't dig into lists, but we should still explore the syntax so that we can look into adding this in the future if the need arises.
  • Lee: I consider Stage 2 the point where there's code/implementation so we can actually start to use this. It's not strictly required though.

AOB:

  • Benjie: we got the Oct 2021 spec release out! Lee!
  • Lee: yeah! I wrote a blog post about it. Big props to Brian for chasing down every contributor and making sure they'd signed all the legalwork that was necessary.
  • This is the first cut under the GraphQL Foundation, so all of the legal text we've written is enforceable and solid.
  • Hopefully back to an annual process after this.