Skip to content

Latest commit

 

History

History
195 lines (174 loc) · 20.8 KB

2020-05-07.md

File metadata and controls

195 lines (174 loc) · 20.8 KB

GraphQL WG Notes - May 2020

Video recording: https://www.youtube.com/watch?v=xtdcaOzoacU

Agree to Membership Agreement, Participation Guidelines and Code of Conduct (1m, Lee)

  • Specification Membership Agreement
  • Participation Guidelines
  • Code of Conduct
  • Brian: We'll be adopting the CLA-style flow in GitHub to ensure that everyone is complying with the above when they add themselves to WG meetings, etc
  • Phased roll-out to prevent blocking PRs (over the next month)
  • Shouldn't affect people who've already been approved
  • Can be enabled selectively for the different GraphQL repos
  • Plan is to enable it for most, if not all
  • Signing on one repo will apply to all repos.
  • Zoom accounts: we'll be adding passwords; not to keep out meaningful contributors, just to block people guessing the Zoom ID and bombing it. Password should be kept with the invite URL.
  • Calendars: consolidating on the foundation calendar to avoid there being clashes. We'll be sending out details.

Determine volunteers for note taking (1m, Lee)

  • Benjie
  • Alan

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

  • Currently open action items
  • Made it through most of last week’s action items
  • Only 2 and they are ancient, these are the ones that sit around forever
  • All the action items that we set up in the last month have been covered:
    • [ACTION - ?] Making sure we have agenda items for the rest of the year
    • Subcommittee meeting for the input union RFC

Custom scalar specification URIs (5m, Evan Huus / Matt Farmer)

  • Anticipate the reference implementation PR being merged soon.
  • Matt: Should be pretty quick, merged this morning
  • [ACTION - Lee] Final editorial pass

Query a Query query query: "query" ambiguity (10m, Benjie)

  • The term "query" is overloaded and can be ambiguous
  • Benjie: This is to solicit opinions… We seem to have an issue in GraphQL. The term “query” is overloaded… It is confusing when people are unfamiliar with GraphQL. Hard to write documentation. Generally a problem.
  • Benjie: For example: “Query operation”, and operation of the Query type, query error, query reuse (fragments), Query keyword in the schema language, query execution (which can mean Query operation or Mutation or Subscription operation).
  • Benjie: Looking for someone who can go through the spec and try to clean up this terminology
  • Benjie: Potential useful words: “operation”, “document”
  • Lee: Perhaps create a mapping so that we can be consistent with terminology
  • Ivan: If we change the terminology in the spec, we may also need to change the implementation like in GraphQL.js
  • Gabriel: If there are many places where the term needs to be changed, then we should have a document containing all of the terminology
  • Benjie: I agree, as Lee suggested. We should have a list of spec terms.
  • Ivan: quick idea: have the term defined in the spec or reference an appendix
  • Lee: I agree, we should have a living doc to provide feedback. A source of truth in the spec would be really nice.

Discuss RFC adding introspection shortcuts to determine full typename and named type’s name (15m, Terrence)

  • #710 - Add namedType and punctuatedName to __Type
  • Terrence: Simple contribution. Proposal to add nameType and punctuatedName to the __Type introspection.
  • Benjie: I am a fan of this. The ofType ofType ofType is very verbose. It would be nice just to have a simple way to query for the type in a nice readable format. In general, I think it is a good idea. Imagine an inline tuple type, how would this change?
  • Lee: I buy that the introspection API is cumbersome. I don’t know if that’s the most important problem that we need to address right now. The thing that I’m most worried about is making assumptions about how introspection should work and baking it permanently into introspection. Punctuated name for formatted type field might be able to work with tuple/etc. Named type might get trickier in that world if there was a tuple, for example.
  • Evan: I agree with both of you and Benjie. We need a lot more depth and explanation for possible implications in the future in the RFC
  • Ivan: I am personally against using SDL inside Introspection. We have a problem with custom scalars. It's a challenge to expose default values for field arguments in SDL.
  • If we figure out how to do this properly, I'm for it - in the introspection query we have 7 or 9 levels of nesting; but I'm against treating it as a string. Maybe we can do it as an array, since for the array we don't need to write a deep query.
  • Stephen: To focus on the problem of finite queries for introspection and then go from there.
  • Lee: you're concerned about being able to write an introspection query of a [[[[....!]!]!]!]!]! which some tools won't let you introspect because their introspection query doesn't go deep enough.
  • Evan: A number of APIs have limits on the amount of nesting. This might limit the kind of introspection that you can do if you cannot go deep enough
  • Lee: I'm not as opposed to a stringified form as Ivan. How to deal with much deeper types is a problem we've discussed a lot; we've considered allowing recursive queries via fragments in the past, and the reason we don't have that currently is to prevent infinite recursion. Stephen's point helps to bring this back to an actual problem rather than an inconvenience.
  • Terrence: Not too familiar with the spec... Are there dynamic types coming in to play?
  • Lee: imagine we introduce recursive fragments as a solution for this problem; this problem goes away but when used as a general tool recursive fragments could be used to write a query that would never end, and DDoS the server. Recursive fragments make it hard to control how deep the query can go during query validation.
  • [ACTION - Terrance] - describe the problem, look at the landscape of potential solutions, and figure out which ones are appropriate or not
  • Lee: introspection should not have a recursive problem.
  • Lee: PR is marked as Strawman. Next steps: answer the questions around the concerns; tease out how this might break possible future additions to the introspection query. Compare this solution path with other solution paths - lets make sure this is the right one.
  • Terrance: don't we require multiple queries to solve this currently?
  • Lee: recursion isn't what we're concerned with - it's infinite recursion that's potentially an issue. This could happen whenever you had a cycle in the graph of your objects. If a feature is added to introspection (such as recursive fragment), that capability could also be used outside of introspection.
  • Evan: for clarity, your solution Terrance does not introduce infinite recursion.

Input Union RFC (??m, Vince)

  • Input Union RFC

  • Vince: A few of us met a week ago to talk about the various solutions proposed for the input union. Very lively and interesting discussion; entire RFC reviewed; we have some rough notes...

  • Solution 4 didn't have a champion, so for the most part is off the table.

    • Structural uniqueness
    • Whole host of complexities
    • Undue burden on schema authors, restrictions on what you could model
  • All other solutions look potentially viable.

  • Potential new solution - solution 6 - merging of solutions 1, 2 and 3. A frankenstein algorithm that'd do a set of checks to determine what the result is. We don't have a final form of this solution yet, it's still being thought through, but there are aspects of all of solutions 1, 2 and 3 that people like.

  • There's a fair amount of talk about @oneOf and how that might be appropriate for both input and output, and could be a valid solution.

  • Follow up that may or may not have been completed.

    • Lee’s going to write up a concept for this merged solution 6.
    • Vince is going to do some research on what literal value looks like (as required by solution 6).
    • Benjie’s going to do some thinking about @oneOf and how it can be used to express input unions and also usage in output types.
  • Evan: (was there for most of the meeting) there's broadly two solutions: a tagged union approach (@oneOf) and a discriminated union approach (solutions 1-3).

  • Vince: Yes, one of the comments in a recent PR from someone studying programming languages commented that all of these solutions are essentially the same under the hood. The task is to discriminate between types.

  • There are a few mechanisms we can use (vehicles for discriminants):

    • type name
    • literal value
    • nothing
  • Vehicles to resolve ambiguity: * having a default * using an order * enforcing uniqueness

  • How can we merge these vehicles together whilst maintaining simplicity.

  • Another aspect we went over was: where do we put the power to discriminate:

    • The spec - strict rules
    • The server - the schema author decides
    • The client - indicates what type to use
  • Lee: One thing that was very valuable is that we read through the solution criteria; this was super useful because we could add colour (added objections, removed objections, adjusted the criteria score, eliminated some of the criteria). There's a followup action to make sure that we go back through and make sure all these changes are fully written up.

  • Lee - solution 6 sub-discussion:

    • Eliminates 3 in favour of 6, but may also eliminate solutions 1 and 2.
    • Solution 1 and 2 require an explicit discriminant
    • 1 requires __typename
    • 2 introduces literal types; when specifying a union you have to indicate which field contains a literal type with the discriminator (and this would be validated at schema build time)
    • We like both; but the problem was with adoption strategy - converting an input object to an input union couldn't easily be done because they needed a discriminator added.
    • If there's already structural uniqueness then it seemed too burdensome to require schema authors to add a redundant discriminator.
    • Solution 3 is the opposite. It looks only at the order. It just walks through all the different types and selects the first one that fits.
    • If they're structurally unique, it works fine. If not, you need a discriminant. Authors need a tool to specify the discriminant - and Option 3 doesn't supply this.
    • A realization of option 6, option 1 and 2 provide tools for selection, but to not require them
      • __typename
      • literal type
    • Vince: The rough algorithm is check type name, then use some of these discriminant techniques
    • Lee: the "first match" approach works as an unoptimised algorithm for solutions 1 and 2 too - at most one type would match, so we can still do structural comparison. (Of course, we can add optimisations on top of this for types that have discriminators.) Importantly the algorithm can remain simple
  • Ivan: I’m worried about error messages. Complex algorithms create confusing error messages.

  • [ACTION - Ivan] We should add a solution criteria that requires the error messages are helpful.

  • Vince: pick a field to be the literal discriminant; e.g. input CatInput { species: "CAT", numberOfLives: Int }. How do you structure this so that the structure doesn't get complex. What if the input union definition contained all of the structure needed to describe it; so the inputs don't need that. It might look like a type. A quick sketch (via chat):

    inputunion Animal @discriminator(field: "species") {
    cat: CatInput
    dog: DogInput
    wolf: DogInput
    }
    
    inputunion Animal @discriminator(field: "__typename") {
    CatInput: CatInput
    DogInput: DogInput
    }
  • Ivan: Benjie came up with a problem with this solution. You cannot add a field with the same name. In many applications, input unions will be generated from a database and when you choose a name for the discriminator field, you’re limiting all other member fields. When you read a schema, it is not obvious that you cannot use this field.

  • Lee: Good point from Ivan. I’m not sure if it’s a dealbreaker. It might need something more to limit that error path.

  • Benjie: We were discussing how the @oneOf models external sources. One of the main issues is now it gives two ways of doing the same thing. I will research this topic more.

  • Lee: When we were first looking at unions, we had a similar discussion.

  • We decided the form of the output was more important than the shape of the query. We know that there are people using the structural union approach, even though they know that there's no guarantee of this type safety. There's also a handful of use-cases of this inside of Facebook even. We think there may be value in adding this in in addition to any other Input Union solution.

  • Vince: we should do another focus working group.

  • Lee: one between now and the next Spec WG would be great. It's a good forcing function to get us through the actions we have.

@defer/@stream arguments and payload format (20m, Rob / Liliana)

  • @defer/@stream arguments and payload format
  • (Overview of above link)
  • The spec will have @defer/@stream as a SHOULD not a MUST to allow the server to override the client if it knows it will be more efficient to return the data directly. The client may need to handle this if it is expecting the result to be deferred.
  • Matt: high-level comment - this RFC matches how our [Facebook's] client worked as of 2 weeks ago. We discovered at Facebook that our client works perfectly with servers that always indicate if they're defer/streamed or not. But when we pointed the client at an external spec-compliant server we errored because we never got a final payload, because the initial payload is the final payload, and there's no indication.
  • Lee: if we have a default for this, it should default such that it works for existing spec servers.
  • Facebook is always sending is_final whether true/false so that it can support older clients, but they don't want the open source community fixed with that.
  • Rob: we'll update the spec to specify that is_final defaults to true
  • Lee: use camelCase rather than underscore_case
  • Rob: I was going to ask about that - I'll make the adjustments.
  • Lee: "label" is required. It seems useful, can you detail more about why that's needed in addition to path?
  • Rob: there could be two deferred fragments on the same object - they'd have identical paths, but the fragments would be different - it's a way for clients to understand this. It doesn't necessarily have to be a required argument; marking it as non-required is a possibility.
  • Matt: It might have to be required especially with clients that can send multiple requests. Multiple queries open at the same time on the same path and you do not know which one… Differentiate multiple requests
  • Lee: we always need to be able to differentiate results streamed back from different operations - your network interface should be able to know the difference about payloads from one request vs another, it's just stream de-duplexing.
  • Kewei: It seems that Rob’s first use case, I think for clients who are more fragment-oriented, this may be required but I’m not sure
  • Lee: clients could auto-generate labels based on the location within the document/fragment/etc.
  • Benjie: I am confused about the results coming from different fragments. If you allow two different streamed fragments, this seems like quite a divergence from what we do currently.
  • Benjie: Say you have non-nullable fields, in this solution, you would need to generate
  • Matt: This is a problem for type model based clients. Operation-level type model based clients and again there, you would have to update it to be knowledgeable about deferred fields. It gets messy. For clients that are truly fragment based, it is actually very clean. As long as you have a label, you can ask if it has been fulfilled or not. When we do get the deferred field back, we know that the types match. You do need to add special handling at the fragment-level boundaries.
  • Benjie: Is there a problem with too much branching going on? Could it be used for some kind of denial of service attack?
  • Matt: We’ve always allowed you to create queries that branch a lot. Get friends of friends of friends. It does potentially exacerbate the problem but because you now define these kinds of fragment boundaries, you can stop the query as it is being fulfilled.
  • Lee: We should have a clear articulation of all these problems in the RFC.
  • [ACTION - Rob] I will lay them out in the RFC
  • Lee: I don’t think all servers should be required to support @defer and @stream
  • Matt: we're currently proposing that servers SHOULD (not MUST) implement @defer and @stream - should we bump this down to CAN [MAY]?
  • Matt: What should servers do if they receive queries that use @defer and @stream but do not support them.
  • Rob/Lee: if the server accepts the directives, it's reasonable to assume it's treating them correctly even if we give the ability for the server to ignore them when it sees fit.
  • Lee: also if you include @defer/@stream it likely indicates what the network response is, e.g. maybe using chunked encoding. We should expose via introspection whether the server supports these capabilities or not.
  • Evan: I am curious if @defer/@stream will work better for a client and if a client would get faster responses
  • Lee: in cases where parallelism is possible, we'd hope that well-designed GraphQL servers should be able to exploit that.
  • Evan: for languages without global interpreter locks this should be possible
  • Benjie: do we need to indicate both via the server AND via the schema which features are available
  • Lee: we should make it easy for clients to understand what is/isn't available
  • Benjie: example GraphQL reference implementation schema with subscriptions served over express-graphql does not really support subscriptions.
  • Lee: the assumption at the moment is that if the GraphQL schema says it supports subscriptions then the server also supports subscriptions
  • Benjie: a single schema might be exposed over multiple transports, so we need to know how to differentiate what features are supported both from the network transport and from the schema.
  • Lee: point well made, we this may be outside of the defer/stream spec (which can use chunked encoding, so can be served over regular HTTP); but it may be something we can use the GraphQL over HTTP Spec to help us with.
  • Kewei: a fundamental issue is interleaving mutations and queries. Instead of thinking of spreading execution over a long period of time, think of it as allowing the server to reorder the execution but still trying to complete it in as little time as possible.
  • jhusain: when we defragment on the server it's to help us avoid multiple executions of the same field. Because there's no "identity" of a node in GraphQL, it's hard to resolve. It may be valuable to standardize how we identify a unique entity within the GraphQL schema because it may affect how we resolve issues like this. Identity can help us to relatively inexpensively guarantee consistency within a single operation.
  • Lee: I support doing these in parallel. Using @defer effectively lets you pull results down earlier but doesn't necessarily mean that queries take longer to run (previously you'd build the entire result and then send it all at once).
  • morris: if you can determine before the query if @defer/@stream are supported...
  • Rob: I was assuming that an old server that didn't understand it would error out with an unknown directive
  • morris: is that normal?
  • Lee: that's true for other directives that servers may/may not implement - if the directive doesn't exist then the query is invalid. Failing loudly if the directive is not supported is important.
  • Gabriel: naming the argument if may be confusing. foo @defer(if: false) may be interpreted as defer this if the value is false.
  • Kewei: this is the same issue with @include and @skip
  • jhusain: consistency here is the strong argument
  • Evan: I've not seen people make this mistake with @skip/@include
  • Gabriel: I'm in favour of consistency as well.