Skip to content

Latest commit

 

History

History
213 lines (193 loc) · 20.9 KB

2021-01-07.md

File metadata and controls

213 lines (193 loc) · 20.9 KB

GraphQL WG Notes – January 2021

Watch the replay: GraphQL Working Group Meetings on YouTube

  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. Update on GraphQL Specification membership documents (5m, Brian)
  7. Advancing no introspection at root of Subscription operation (10m, Benjie)
  8. Discuss Schema Coordinates Spec (10m, Mark)
  9. Default value coercion update (15m, Benjie)
    • Spec PR
    • How do we write spec language to prevent infinite recursion?
  10. Query ambiguity: replacing 'query error' with 'request error' (15m, Benjie)
  11. defer/stream updates (10m, Rob)
  12. Alternative meeting time (5m, Andi)

Determine volunteers for note taking (1m, Lee)

  • Benjie
  • Evan

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

  • Introspection shortcuts RFC - no further progress, closing as stale
  • Giving feedback on defer/stream - complete
  • Moving deprecated directive on inputs to stage 2 - we think this was done, just the record-keeping was confusing to review afterwards to make sure
  • Give feedback on schema coordinates - complete
  • Complete JS implementation of subscription introspection validation - complete
  • Convene a meeting of the GraphQL input union working group - complete
  • Add schema validation for @deprecated on non-null in JS - complete
  • If item 550 is still open next meeting, we'll do "the fast way" - so we'll remove contributors who have not contributed in over (a year?)

Update on GraphQL Specification membership documents (5m, Brian)

  • Brian provided an overview of the specification membership agreement documents (the documents everyone must sign before participating in development.)
  • He noted that GraphQL Specification project was set up on v4 of the Joint Development Foundation (JDF) documents. However, the v4 docs require a new signed document from all contributors for each release. Brian noted that this will be a substantial source of overhead for all releases.
  • Brian next noted that since JDF became part of the Linux Foundation (LF), various changes had been made to the document templates removing this requirement and only requiring the initial signature. Currently, the LF is using v5 of the documents for new projects.
  • Finally, Brian noted that the JDF documents include an upgrade provision, where the TSC is empowered to vote to adopt new versions of the specification membership agreement documents. He noted that these are being drafted by legal and will be presented to the TSC for an email vote. Once adopted, this will allow for a streamlined spec release.
  • Probably a super-majority vote required; but normally we make sure that everyone votes just to make sure everyone has read it.
  • Spec Slack TSC channel and email

Advancing no introspection at root of Subscription operation (10m, Benjie)

  • graphql/graphql-spec#776
  • graphql/graphql-js#2861
  • Decided to disallow since it already doesn’t work in the reference implementation.
  • We can revisit afterward.
  • During the graphql-js implementation some issues were discovered around fragments that are also being fixed - waiting on review.
  • The RFC is ready for the next level (stage 2).
    • General agreement
  • Ivan has some concerns about the complexity of the JS implementation, but agrees with the spec changes.
  • Outcome: advanced to stage 2
  • ACTION - Ivan: thorough review of the PR
  • ACTION - Lee: editorial review of the spec change (at first read it looks right)

Default value coercion update (15m, Benjie)

  • graphql/graphql-spec#793
  • Updated and addressed feedback from last month.
  • Currently a case where infinite loops can occur, need to figure out how to avoid this in the spec (graphql/graphql-spec#793 (comment)).
  • Benjie would like help on this ^.
  • Lee: the original intent was that default values would be fully formed and not need expanding/coercion, but this is tricky.
    • Benjie: there is a conflict with user expectations.
  • Lee: is the right thing to treat defaults like user-provided values with coercion and recursive default application, or is the right thing to just validate default values at schema time
  • Matt: anything we can push to build time is a win
    • Lee: this is technically an implementation detail
  • Ivan: we have infinite-recursion-prevention in a few places already but it’s a lot of code; the implementation makes me nervous, but I agree with the spec change
  • Lee: somewhat controversial idea: maybe this is two problems?
    • The very first motivating example of 793 where those queries give different results. Definitely should be solved, this is a problem.
    • Using default values of partial input objects. Is this even reasonable to have a default value of an incomplete object?
  • Benjie: the real motivating case from PostGraphile does need partial input object default values, or else requires a lot of expensive duplication in the schema.
  • Lee: the schema should always give you fully-formed information, as with e.g. objects which “duplicate” the fields on any interfaces they implement; there’s no navigation needed.
    • The naive developer looks at default = {} and sees this as an empty object, which is maybe equally surprising if there are other defaults deeper in the schema.
  • Evan: could we fully-specify in the SDL but let implementations generate that automatically like ruby does with interfaces?
  • Benjie: full-specified does make specific differences much harder to spot in large default inputs
    • Ultimately “if you see {} as a default value, it should be the same as the client passing {}”.
  • Matt: this defaults-should-mean-the-same-thing-as-the-client-passing-it makes sense, but isn’t technically violated by the schema requiring them to be fully-specified. We already have the “differences harder to spot” issue with interfaces.
    • Leaning to requiring fully-specified now.
  • Lee: would like some more concrete examples to motivate cost/complexity trade-off.
  • Benjie: we already have all this complexity and code for handling client-specified variables; if we don’t solve in the spec, we’re just punting it to every other implementation and schema developer
  • Lee: we solve for readers of schemas more than for writers of schemas
  • Benjie: can we just expand for introspection and not for SDL?
    • Ivan: the ability to repeatedly cycle between SDL and introspection without changes is an important property
  • Lee: Just to be confusing, there is going to some complexity in requiring fully-formed-ness right now since that isn’t actually a thing in the spec right now and would need definition as well; so both solutions have some hidden complexity
  • Benjie: so, next steps are not clear
  • Lee: as a next step, let’s gather some concrete examples from real schemas, as comments on the pull request
  • ACTION - everyone, post your examples
  • Benjie: it'd be nice to have a repository with a load of .graphql files containing loads of schemas that we can use for this research
  • ACTION - Benjie - Ping James about Apollo schema store - are there a collection of public GraphQL schemas that they maintain?
  • See also https://github.com/APIs-guru/graphql-apis
  • Alan Cha: we collected 8600 unique GraphQL schema from OSS projects for a research paper, but we cannot publicly republish this due to usage rights; maybe I can think of a way to let you use these schemas.
  • Lee: even just searching across that for = { would be valuable!
  • Alan: we even combined schemas that were defined in separate files into one schema.
  • Andi: in GraphQL Java we recently updated a schema anonymizer function
  • Benjie: that'd be useful because Alan can share the anonymised schemas and we can see if any are relevant
  • Alan: we maybe can share the anonymised schemas, and then link them back to the originals via an identifier
  • Lee: that's an amazing dataset to have. Even just the anonymous schemas is beneficial to see the patterns being used even if we can't determine why (due to naming)

defer/stream updates (10m, Rob)

  • Mostly blocked on the JS side by the TS migration.
  • Official experimental branch of graphql-js/express-graphql
  • Lilliana and Rob wrote a blog post which <...> tweeted
  • Got feedback; couple things to discuss:
  • Item 1:
    • In @stream we have the initialCount argument (required)
    • Benjie suggests that we make it optional with default 0
    • I don't have a strong opinion on it
    • What's everyone's opinion?
    • About initial count: { myList @stream(initialCount: 5) { ... } } would include the first 5 results, and the rest would be streamed. E.g. you might specify how many posts fit "above the fold" and have the rest come in async.
    • Lee: I have less context than others, but Benjie's opinion makes sense to me - people will want to write this without arguments; I'd expect it to default to zero without arguments.
    • Rob: We talked about complex batching (fetching groups at a time) but we decided against this in the end.
    • Rob: it's pretty nice to use @stream without arguments, so I can make this change
    • Ivan: is this a minimum or maximum?
    • Rob: server should try and satisfy exactly that many (though it can of course return early if there's insufficient data)
    • Lee: I'd expect the server to know more about this than the user - if you request 3 but the server gets 5 in the initial batch, why not just supply 5?
    • Rob: could be that the resolvers below are more expensive, so just resolve the necessary 3 first.
    • Ivan: I agree with Rob. If we're allowing it to be optional, maybe change the name from initialCount to a name that indicates that the items will be inlined (i.e. reverse the meaning).
    • Benjie: I think there's distinction between the initialCount (which could be for the first follow-up payload) versus inline (which implies injecting it into the main GraphQL response)
    • Lee: probably need to do some bikeshedding around the name; it's worth making sure the term we use is the best one we can think of.
    • Lee: I want to challenge requiring the initial set to be a particular size. Maybe it's the right decision, but it seems unintuitive to me. How should the client and the server talk to each other: when the client says I want X, is that a requirement/constraint or a hint? Performance issue versus UI constraint.
    • Benjie: if the server doesn't support @stream then you'd get all the results, so it's defacto a minimum
    • Rob: current plan is that if the server does not support @stream then it should reject the query.
    • Rob: we don't want servers to say they support defer but then ignore it; it might be better to split things up into separate queries rather than using a defer that doesn't actually happen.
    • Matt: Facebook has had a huge amount of usage of @stream/@defer internally; we require that initialCount says "this is what you will get" but we also have an escape hatch where the server is allowed to define what to do if a particular boolean is passed.
    • ACTION - Kewei: explain exactly why this case is, so that we know why the decision was made.
    • Lee: we want the spec to describe the constraints, and non-normatively the desired outcomes/best behaviours; but we don't want the spec to add requirements that people will need to break to get the behaviour they want. We should use SHOULD not MUST for some of these behaviours.
  • Item 2:
  • In the directive location in the schema there's a number of options; for @stream it's field; but we need it to statically say that it's a list field - would adding this be a breaking change? Is a runtime validation rule that you can't @stream unless it's a list good enough?
  • Lee: the purpose behind directive locations is that arbitrary directives would have rules that tooling could rely upon even if they don't understand the directive itself. Given we're putting this in the spec, we can add additional validation logic, so we don't need to rely on the location for arbitrarily defined directives. Adding a different location is undesirable (conflates AST with type system).
  • It should be checked in query validation.
  • ACTION - Rob - update for initialCount; validation rule in GraphQL.js and the spec.
  • ACTION - everyone - eyes on the RFC please!
  • Lee: I'm happy with the speed of this; it's a credit to the consistent work put in - thank you!

Alternative meeting time (5m, Andi)

  • GraphQL things are currently scheduled for 4am for Australian/New Zealand people
  • Shifting 2 hours backwards (to 6am) would be desirable; but it makes it late for the Indian/Asian area, but maybe we can alternate it sometimes?
  • For Pacific Coast US, this is first thing in the morning so pushing it back 2 hours is very palatable. I know it makes it later for Europe.
  • Andi: for Europe it'll be after working hours, but still during "I'm awake" hours :D
    • Finland 9pm
    • Central Europe (Germany) 8pm
    • London 7pm
  • Ivan: 9pm for me, if we keep within 2 hours, it's normal. Finishing 11pm is not super cool, but it's possible.
  • Benjie: what's the effect on the TSC?
  • Matt: we could split the meeting; but it'd require a lot of coordination up front.
  • Ivan: I'm for predictability; easier to have it at night rather than at random times. Maybe we can use a weighting system to find the best time for everyone (not preferred hours, but possible hours) using the TSC and attendees of this call.
  • Lee: I'm okay with alternating between two times if we can get them predictable and locked into the calendar in advance. I don't want to have a selection bias of people who are regularly on this call - perhaps if the time wasn't so inconvenient more people in your time zone might join. We've had good attendance from Europe, but if we push it back 2 hours it might reduce that - I don't want to "rebalance" to find a new set of people who can attend.
  • Andi: I agree we should see the bigger picture; maybe we need three different meeting times we alternate between. We want to make it as accessible as possible for everybody; it's 1am in asia right now which is probably why we don't have anyone from asia here right now.
  • ACTION - Andi - open a GitHub issue and discuss the options there.
  • ACTION - Lee - find out if there's a great way to collect this input (worst case, google form); use this to inform choice.
  • Lee: seems like moving it 2 hours back might be okay with most people.

Discuss Schema Coordinates Spec (10m, Mark)

  • RFC and spec edit have been out for a few months; thanks to everyone who has reviewed those.
  • Last time the field arguments were the discussion; two main contenders: Query.business(id:) versus Query.business.id; we wanted consensus and after much discussion on the GitHub threads... when/how/where do we make the next steps?
  • Lee: I've added the appropriate GitHub labels on the RFC. Merging that as the discussion seems to be settled out; you can still change it via additional PRs.
  • Determining if we need to move from RFC 1 to 2. Benjie seems happy with it. Are there JS changes required?
  • Mark: we concluded no need for JS changes. We're in the same kind of state as last month, just with the question over arguments being more clear. I was 50/50, but am leaning more towards the Question.business(id:) syntax for arguments.
  • Lee: regarding reference implementation changes, we should add at least parsing functions to allow parsing into an AST and printing an AST back out again. That would give people the tools they need to use this; and I think that's needed for RFC2.
  • Lee: another (not required to move it to RFC2) would be to have a function that you pass a schema reference to and get a schema coordinate out.
  • Lee: another: given a schema and a schema coordinate, return out the thing that that schema coordinate refers to. Also not required for RFC2.
  • Lee: the parse/print will help us write tests and ensure we've dotted the i's and crossed the t's.
  • ACTION - Mark - implement the print/parse functions.
  • Lee: we encourage the spec champions to write a blog post about it and folks share it out - you get the credit and explain why it's useful, etc.
  • Lee: <Brian talked earlier about how to get things into the next release of GraphQL>
  • Mark: are there any other open questions?
  • Ivan: returning the enum value or whatever based on schema/coordinate pair may not be useful, we should probably indicate what the resulting thing is. You can distinguish type, field, argument, directive, but Foo.bar does not indicate what type the result is going to be. I need to think about this.
  • Lee: this is part of the reason why I like writing the reference implementation at the same time as the spec text - it helps us to test assumptions with the real code, write unit tests against it, etc.
  • Benjie: explains discussion with Danielle and summarises: graphql/graphql-spec#746 (comment)
  • Benjie: if there are other use cases for this, now would be the time to say
  • Mike: where should we discuss that
  • Mark: in the RFC
  • Lee: it's fine for that to happen there; it's hard to follow discussions on closed PRs. Maybe we should open an new issue about how Schema Coordinates can be evolved.
  • Mike: I'd like to surface how we might use this; we track usage of deprecations and this would have been fantastic for that.
  • Lee: that's great; I think Evan has opinions on this too.
  • Evan: yeah we use paths internally and as part of our public deprecated API usage docs.
  • ACTION - Benjie - open this issue?
  • Ivan: when you open the GraphQL.js PR can you also open a Spec RFC PR?
  • Mark: for the utility functions specifically?
  • Ivan: if you do the development publicly then I can help you earlier.
  • Mark: ah, stream of commits - I'll take your suggestion so we can work as we go.
  • Ivan: if you need help, reach out on the GraphQL Slack via @i1g. We can even schedule a call if we need to. I'm not sure how to name the function even.

Query ambiguity: replacing 'query error' with 'request error' (15m, Benjie)

  • Query errors can come from queries, mutations, subscriptions or even just the document that describes them.
  • Lee: makes sense to me. Since we're opening the can of worms... we should maybe look at errors in general. Validation errors, field errors, they all have slightly different behaviour. If there's a place we can describe the errors; that would be beneficial too.
  • Lee: I'm okay with it being a follow-on RFC, but if we can do a small amount of work to improve the editorial around errors that could be a quick win.
  • Evan (chat): if we’re clarifying errors, we should leave a name for “in-schema” error types that get returned from mutations in a lot of schemas
  • Evan: for example if you model errors in your GraphQL schema type system itself, we might want to add a non-normative note about these.
  • Ivan: this was one of the questions from GSoD via Carolyn - how to do validation errors? Definitely not part of Benjie's effort to clarify the word "query", but encouragement if someone is interested in clarifying this, a non-normative note about unexpected errors versus defining errors in the schema.
  • Lee: sharing things to this audience can be done in Slack.
  • ACTION - Benjie - investigate adding what a response error is the first time we mention it.
  • Advanced to stage 1.