Skip to content

Latest commit

 

History

History
195 lines (164 loc) · 19.3 KB

2020-01-09.md

File metadata and controls

195 lines (164 loc) · 19.3 KB

GraphQL Spec WG January 2020

Agenda

  1. Introduction of attendees (5m, Lee)
  2. Determine volunteers for note taking (2m, Lee)
  3. Review agenda (2m, Lee)
  4. Review previous meeting's action items (5m, Lee)
    1. Currently open action items
  5. Discuss possible standardization of stream and defer. Issue (30m Keweiqu)
  6. Input Union RFC (15m, Vince)
  7. Custom Scalar Specification URIs (10m, Matt Farmer and Evan and Andi) 2. graphql-js Pull Request is ready for review 1. The spec has changed from @specified(by: String) to @specifiedBy(url: String), and needs minor update 3. graphql/graphql-spec/issues/635 4. graphql/graphql-spec/pull/649 5. Discuss next steps
  8. Defer and Stream Directives Sketch RFC (15m, Liliana)

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

  • Currently open action items
  • Lee still needs to look at the January 2020 Spec release
  • Ivan has already released GraphQL-js v15.0.0-rc.1 to npm

Discuss possible standardization of stream and defer. Issue (30m Keweiqu)

Defer and Stream Directives Sketch RFC (Liliana)

  • Facebook have been working on this for 3 years, they think they're coming to maturity for the edge-cases/etc
  • Have noticed community interest in these topics - want to come together to formalise discussion
  • How do we go forward with specification - is it official or separate?
  • Liliana / 1stdibs: we are willing to put effort into the spec, helping find commonality, etc
  • Robert: what progress has happened recently?
  • Kewei: news feed challenges our implementation, and proves that our implementation can handle busy use cases, and ensures we handle edge cases - helps us to think through. How can we continue to optimise it? Led to batched response payloads/etc
  • Michael: what are the status of the PRs? Are they in line with what you're doing at Facebook?
  • Kewei: the relay implementation is very aligned with the Facebook implementation
  • Liliana: The PR we have open is inline with the implementation at Facebook
  • Michael: our implementation is based on the Relay implementation
  • Mike: are the two implementation competing or aligned?
  • Rob: aligned
  • Vince: How does this relate to subscriptions?
  • Michael: run a standard query, and defer just parts of that query - immediately get the non-deferred part, but in subsequent updates receive "patches".
  • Kewei: defer and stream is about "one payload" that's delivered chunk by chunk; subscriptions are then used to subscribe to long-lived updates
  • Vince: so defer/stream are not meant to be long-lived?
  • Kewei: correct
  • Michael: see talk by Lee Byron relating to batching
  • Vince: Is there a mechanism for server-side to determine a field should be deferred, or is it all in the client?
  • Gabriel: although the server might know better what will perform well, it's the client who decides how they want to handle it.
  • Michael: the client is the most important part, which is why the client is the driver (not the server)
  • Mike: the client is describing the requirements - here are the highest priority things, versus lower priority that can be returned later. Particularly useful if you're fetching a lot of data.
  • Lee: A perfectly valid response from a server for a query that contains @defer / @stream is to just completely ignore them - the client places the @defer/stream hints and the server can choose whether or not to defer/stream them.
  • Matt/Erik: doesn't that let the server overrule what the client has instructed - isn't that strange?
  • Lee: it's potentially strange, but it gives you backwards compatibility
  • Lee: all production servers ignore @defer/@stream and so this has to be a valid output for backwards compatibility. Also a server may be smart about it and determine that it's actually more performant to inline something than defer it, so it may override if it knows better.
  • Gabriel: "defer" tells the server "give this to me when it's ready" and the server can just say "we're already ready" and return it immediately
  • Kewei: we have use cases at Facebook where the server does indeed override the @defer that the client indicates, and lets the client know about this via the extensions property
  • Robert: there are a number of use cases for subscriptions that would be better served with @defer - for example a long-lived invite request only has one payload - when the user ultimately accepts the invite. We've implemented it as subscriptions, but it might fit better as @defer. "I expect a single response in the future as to the status of this mutation when it's completed".
  • Kewei: we've not used this pattern at Facebook - we hold a HTTP request open for the @defer/@stream operations and it would time out. Definitely interesting to think about it as a callback in a really long-lived connection though.
  • Robert: the semantics of stating a single response will come after "some amount of time" is really useful
  • Kewei: the response format (ignoring the transports) is very reasonable and we can have it handle more use cases
  • Lee: how are these implemented in a transport at Facebook
  • Kewei: we're using the chunked encoding over HTTP transport
  • Lee: we don't talk about HTTP in the spec at all, and it's good to think about this in generic terms so I like how you've been speaking about it - a stream that is expected to close
  • Lee: what's next?
  • Kewei: we want to push this specification forward. What do people think about whether to include it in the official spec or as a supplementary spec.
  • Lee: IMO main spec, like subscriptions
  • Kewei: that's what we'd prefer. What are the challenges? Are we introducing new concepts - like HTTP chunked coding?
  • Michael: we wouldn't need to detail the transport in the spec; the transport part can be in the HTTP spec
  • Kewei: focus on request format and response format
  • Lee: that's right; there's normative and non-normative parts of the spec; we don't want to say you have to use HTTP chunked encoding in the normative part. The abstraction of the stream is already handled in subscriptions, so this is "a stream that is expected to close", and then in a non-normative comment you can state HTTP chunked encoding is what people typically use.
  • Lee: Ivan & co have been working on GraphQL HTTP spec, so contributing to that spec too would be good.
  • Dan: given Lee spoke about @defer / @stream 3 years ago there are likely implementations out there that we don't know about. How should we avoid this being a breaking change for people who already have @defer / @stream? We managed it with subscriptions by introducing a new operation type.
  • Lee: great question: we need to balance spec writing - what's the best solution, and what's the best thing we can roll out?
  • Lee: we might need to pick different words than @defer / @stream if there are big implementations that cannot migrate easily. Facebook use @fbdefer and @fbstream originally.
  • ACTION: community outreach - is anyone already using @defer / @stream that might not agree with this new spec?
  • [ACTION Jesse]: at Apollo we're excited; we've had two PRs for a long time, and there was an alpha version that was cut from that PR and a lot of people were using it. We never officially released it and it's now very out of date, but we're happy to talk to the community and see if anyone is working with it. (I think it's using chunked encoding but not fragments.)
  • Kewei: first step would be for us to come up with an initial draft, and find out what implementations out there are conforming/non-conforming and try and see what migration will be
  • Lee: I feel more confident knowing that Apollo's version never left alpha - I think that's the implementation that made it the furthest in the ecosystem.
  • Lee: it was helpful when we were writing the subscriptions spec to write the implementation and the spec at the same time, and that worked out pretty well. Maybe we can borrow some of the technical pieces from the GraphQL reference implementation.
  • Michael: the subscriptions spec is more about the "stream" which fits well with defer - we use the same interface in the .NET implementation for subscriptions and defer.
  • Robert: looking at the async iterator and how it handles errors helped us to write the relevant error handling into the subscriptions spec
  • [ACTION - Robert? Jafar?] Worth a chat with Tim and Eddy about this.
  • Robert: stateless implementation enables scaling. If we're using chunked encoding then we have no distributed state?
  • Jafar: I've always thought of stream and defer as in-process - it's still stateless (there's state governing the "how", but not the "what") - this seems the right mental model. I wouldn't typically expect interleaving mutations to interrupt the stream. "Consistent within the request." Use memoization/etc to avoid getting two different values for the same field in the same result. Stream and defer are expected to be executed as part of a single process
  • Robert: so scaling it is a case of stamping out clones?
  • Jafar: yes.
  • Kewei: this is a really important question about how a client might handle interleaving requests when requests are long-lived
  • Lee: the spec should state that if you encounter the same object twice during resolution you should get the same value back, even if you get the same
  • Dan: there's cases of name / fullName where later might be inconsistent
  • Michael: you'd use the same DataLoaders for that
  • Lee: we should have a non-normative note about what should happen here. ANd how does it interact with subscriptions?
  • Dan: non-normative makes sense
  • Benjie: how does this work with mutations? Multiple mutations would result in fields changing, and when do the defer results get added?
  • Lee: great point Benjie, if mutation A has a defer in it, does its stream of results have to come to conclusion before we start doing mutation B
  • Robert: the best solution might be to not maintain consistency. E.g. querying for the "server clock" is expected to change over time - returning the same time for later resolvers is going to cause inconsistencies anyway. I can't see how we can get out of this - we have to accept that a GraphQL response cannot guarantee consistency in all cases.
  • Jafar: there's a question about what consistency would mean - we don't have identity in the spec, so talking about consistency without identity is challenging. Memoizing every field would be a big space cost - imposing it on all users without being sure that there's sufficient benefit seems like the wrong balance
  • Lee: it's useful to dig into the weeds to discuss this stuff - but it's important to make a distinction between guarantees and expectations. Even coming up with standardised rules may be challenging. Maybe we should have a rule like "we would expect a query with defer and stream, once it's parts are put together, should be equivalent to if the query had been issued without defer/stream"
  • Jafar: do we want to make a non-normative note about single-payload responses suggesting that things should be consistent?
  • Lee: it's really easy for these things to be assumptions for single-payload responses, but when things are spread out over time you have to start second-guessing these assumptions
  • Jiyue?: we want people to be able to interact with the initial response as early as possible. Users are more likely to act upon the already-rendered screen, which may have a race-condition issue with already returned chunks. Race conditions happen more in the deferred case. Try and alleviate it, but it's hard to guarantee on the server or the client.
  • Lee: very interesting. We should have a non-normative discussion in the spec itself.
  • Gabriel: even with non-normative spec text, I don't think we can do any of this stuff - this problem is not unique to GraphQL. MSSQL server had a "READ COMMITTED" / "READ UNCOMMITED" hint; but GraphQL is relying solely on other data sources, so it cannot make any guarantees about getting data that's consistent - especially when the data is delayed.
  • Jafar: there are different scopes of consistency. Assert: never return the same value with a different value in the response. Need a defined identity for this. There are some subsets of consistency that we could achieve. My opinion is its not worth it.
  • Lee: there's definitely a lot of interest, and some great discussion here. I'm going to cut us off here because we've been talking for 45 minutes.
  • Rob: we have a PR with a implementation open against GraphQL-js - we'd like to work together with Facebook on this.
  • Kewei: we're happy to work together on this - we're looking forward to this collaboration on implementation and spec in parallel!
  • Lee: I'm super excited about this!
  • Jafar: thanks for pushing this forward.
  • Jesse: sorry your PR never landed!
  • Ivan: regarding the graphql-js PR, can I merge it?
  • Lee: the RFC document (not the implementation PR?)
  • Lee: I'm going to merge the spec PR, but the implementation we need to follow up offline
  • ACTION - Jafar - follow up with Gabriel offline.

Input Union RFC (15m, Vince)

  • Vince - topics:

    • Criteria ranking PR: graphql/graphql-spec#668
    • Determine the “worst” solution
    • Signup champions for solutions
    • Question that has arisen for me:
      • Are directives the only way to safely add new features?

  • Have added gold/silver/bronze weighting - input welcome graphql/graphql-spec#668

  • There's three tiers; solution number 2 ranks the highest ("configurable discriminator"); then solutions 1 and 5 tie, then 3 and 4.

  • 3 and 4 either fail or don't fully meet some of the important goals we've laid out, e.g. schema evolution. E.g. if the schema evolves in a particular way then previous operations might have a different result.

  • Ivan [chat]: "We promise only backward compatibility and only for queries:

    Once a query is written, it should always mean the same thing and return the same shaped result. Future changes should not change the meaning of existing schema or queries or in any other way cause an existing compliant GraphQL service to become non-compliant for prior versions of the spec.

    So we can update SDL with new keywords"

  • Vince: two important things

    • Semantics of an existing query shouldn't change as the schema evolves
    • Adding new keywords to SDL - is that a problem that it might break parsers?
      • Jesse: new keywords are okay because all existing queries will remain valid
      • Michael: agree
      • Lee: agree
      • Vince: what if the semantics of a query change (but it still succeeds) - e.g. the type that an input is determined as changes as the schema evolves. Is this acceptable? It seems bad.
      • Lee: maybe. Depends on if the result is observable. (We discussed this a couple meetings back.)
      • Lee: There are some ways you can evolve a schema that would break old queries. What kinds of queries should we allow people to write? What kind of changes should the schema be able to have?

  • Would like to determine which criterias we can cut off, and which solutions we can cut out.

  • Would also like to determine champions for solutions

  • Are directives the only way to safely add new features?

  • Vince: Looks like we’re not yet ready to eliminate any of the alternatives

  • Lee: I think that’s right. It’s not entirely binary whether a criteria is upheld. This is captured pretty well with the “warning signs”

  • Vince: Further evaluation is needed. Do we have champions for particular solutions?

    • [ACTION: everyone] Make a PR, adding yourself as champion for a solution

  • Ivan[chat]: I can take discriminator based solutions

  • Vince: Are directives the only way forward to evolve graphql?

  • Lee: We need to be able to evolve the language using more than just directives. Directives are a little smoother, but there’s a design tradeoff. If syntax change is clearly a better developer experience, we should take that into consideration.

  • Kewei: As directive proliferate, we need to consider feature composition

  • Vince: Even though directives are a mechanism for extensibility, there’s a distinction between query and schema directives. What about exposing schema directives through introspection?

  • Michael: There are discussions around extending introspection types, but it’s not finalized yet.

  • Lee: Directives can be complex. We should continue this discussion, but don’t block on this.

  • Lee: RE: directives vs syntax changes - sometimes you want a guarantee of what a server will do for you. The assumption is that directives may be ignored by implementations. In some cases this is what you want, but it can sometimes lead to surprising behavior.

  • For client queries you want to know if something serious is ignored, so we can only use directives for things that are safe to ignore. However for servers the requirements are different

  • Benjie: in the case of @oneOf if a server doesn't support the directive then it won't make that information available via introspection, so the client won't expect the server to honour that directive.

  • Lee: Next steps - champions for solutions. If a solution fails to get a champion, this is a good indicator that it should be cut.

    • Vince: I agree, but just because a solution doesn’t have a champion yet doesn’t mean that it won’t.
    • Lee: 💯
    • Vince: Timeboxed? Are champions limited to members? (e.g. tc39 requires this)
    • Lee: Anybody can be a champion. If we circle back at the next meeting, we can assess at that time.

Custom Scalar Specification URIs (10m, Matt Farmer and Evan and Andi)

  • graphql-js Pull Request is ready for review
    • The spec has changed from @specified(by: String) to @specifiedBy(url: String), and needs minor update
  • graphql/graphql-spec/issues/635
  • graphql/graphql-spec/pull/649
  • Discuss next steps
  • Lee: Let’s move this to Stage 2. This will be a DRAFT. Merge PR into graphql-js.
  • [ACTION - Lee] I’ll check with Ivan to see if this can be in next release
  • Matt: Can specifiedBy be used multiple times?
    • No, this should fail validation
    • [ACTION- Matt] ensure validation fails if specifiedBy is used multiple times

Wrapup