Watch the replay: GraphQL Working Group Meetings on YouTube
- Agree to Membership Agreement, Participation Guidelines and Code of Conduct (1m, Lee)
- Introduction of attendees (5m, Lee)
- Determine volunteers for note taking (1m, Lee)
- Review agenda (2m, Lee)
- Review previous meeting's action items (5m, Lee)
- Require argument uniqueness RFC (5m, Ivan)
- Moving the @deprecated directive on Input Fields RFC to Stage 2 (15m, Stephen)
- @defer/@stream (15m, Rob/Liliana)def
- GraphQL custom Scalar PR in spec: graphql/graphql-spec/pull/649
- Benjie
- Stephen
- [No changes]
- All action items
- Previous Actions (still open):
- Custom Scalar Spec Editorial Pass - Lee planning to do soon
- Introspection shortcuts RFC - now has an assignee
- Update input union RFC following subcommittee meetings - still pending
- June Actions:
- Schedule a call about GraphQL scalars - we did that, will update later (even though Andi's not here)
- Courtney assigned to advancing the namespaces
- RFC for tagged type (input unions) - work has begun, but not to a presentable state
- Ivan: validation for type system in chapter 3 requires uniqueness of type names, field names, etc; but we forgot to specify arguments were unique. It's not an editorial change, but pretty uncontroversial.
- Someone opened the PR, I think I can make the change to GraphQL.js
- Lee: on the topic of having a process; we need to have a reference implementation before calling it final stage.
- Ivan: I plan to do this.
- Ivan: suggest we move this to stage 1 without reference implementation. Want to start the ball rolling.
- Lee: adding that label now. Did we miss it when writing the spec but captured it when writing the schema?
- Ivan: because we use an object map to define the arguments it automatically doesn't allow for duplicates.
- Lee: interesting, I'm curious if other implementations do something differently than building it with a map.
- Ivan: field arguments is a map, but in directives it is an array so you can potentially have duplicated arguments. I think it's an implementation detail and I don't think anyone has an argument against it. Benjie applied some review comments and they were addressed.
- Lee: stage 1 done; Ivan: leaving to you to do a PR to GraphQL.js
- ACTION - Ivan - implement in GraphQL.js
- Ivan: we were going to try and do TypeScript conversion without breaking changes, but it's now planned to do a major version. So we're trying to group all the changes and get them in now. I have a new maintainer helping me with the TypeScript conversion
- Initial spec PR (RFC 1)
Updated spec PR (RFC 2 Candidate)- GraphQL.js PR
- Stephen: @deprecated is a great tool for evolving your schema, we use it a lot.
- There was a proposal in 2018 that expanded @deprecated to input fields and object types. Doesn't seem to be controversial. I took this and rebased this on master and cleaned it up. Are there any reasons not to advance this to Stage 2 status?
- For Draft status there needs to be a GraphQL.js PR; there is one and it needs to be tweaked before it's mergeable.
- Ivan: there's some features missing, but they're non-essential. For example we have a validation rule for tracking deprecated usages, but it's a nice-to-have rather than required.
- Stephen: For Stage 2, the GraphQL.js PR doesn't have to be merged yet. Do we have consensus that the requirements for Stage 2 are met?
- Lee: I think so. Is deprecated only used for output so far?
- Ivan: also used for enums
- Lee: trying to consider if deprecation for input fields might cause issues.
- Ivan: "What if you deprecate non-nullable type?"
- Stephen: it's allowable on input types to remove the "required" (make it nullable).
- Lee: should we go as far as requiring that deprecated input fields are non-nullable? There may be tools that only operate on the non-deprecated part of the schema that would fall foul of this.
- Matt: this would be a bad situation to be in. I shouldn't have to have a warning in my new code because of a required deprecated field.
- Lee: maybe we should add a schema validation rule that a deprecated input field cannot be required.
- Evan: counter-example; we have a use case for deprecating required inputs - it's required in v1, optional in v2, removed in v3; so if you're using v1 we still want to let you know the field is deprecated. Marking it as deprecated gives developers a better timeline.
- Lee: so you think of it as three phases: 1. make it deprecated; 2. make it optional; 3. remove it
- Evan: and yes, we have processes around this because we used versioned GraphQL schemas in a way that most people do not.
- Lee: we're talking too abstractly about tooling; we know what our tools are.
- Lee: my concern is (but may not be a real concern):
- A client-side tool that runs introspection without requesting deprecated: true in the introspection query, so it only gets the non-deprecated parts of the schema; it uses this to do typeaheads/etc, but after being deployed it may fail because a deprecated argument is required but not provided.
- Morris: when you get a warning, usually you can fix it. In many groups you want to be able to get rid of all your warnings.
- Matt: other languages do things with "future deprecation" - i.e. we won't be supporting this forever, but we don't know what our final solution is.
- Evan: if we deprecate a required input field in v1, you can solve it by moving to v2 of our API.
- Matt: but that requires me to change more code.
- Stephen: rather than making it a hard validation when creating the schema, could we handle it with tooling.
- Lee: I'm thinking it should be a SHOULD rather than a MUST when specified in the schema.
- Ivan: PR author asked "should we change the logic of
includeDeprecatedbecause old clients won't know inputs can be deprecated." - Lee: I'm more concerned with backwards compatibility rather than forward compatibility. Every version of GraphiQL should support all old schemas; but older GraphiQL does not need to support all new GraphQL schemas because they may have features it can't possibly know about.
- Benjie: I think these deprecations are serving two purposes: reducing the size of introspection for new clients who don't care about all the old deprecated fields, and indicating to developers that particular fields/features will not be supported in the next version of the schema.
- Ivan: I'm thinking we should require that non-nullable cannot be deprecated.
- Jason: I agree.
- Ivan: it's a feature that didn't exist before, so we won't be breaking anything. If by default we omit required arguments if they're deprecated, it might break some existing tooling if it doesn't bother to read deprecated stuff.
- Ivan: this would be the first directive with validation rule, all other directives don't have a validation rule. We try and push as much validation as early as possible - so if it can be validated in SDL, we prefer to validate in SDL.
- Ivan: (also note that it can be non-nullable if it has a default value; we'll need to check this in AST)
- Lee: indeed - this is about "requiredness" rather than "non-nullability" - requiredness is the combination of "non-nullability" and "has default"
- Lee: I still feel like we need this rule otherwise we're opening some pitfalls. I don't think it needs to be required, it can be a should rule, but we should definitely alert schema designers if this issue exists in their schema.
- Ivan: so we're adding this rule into spec text as SHOULD; we need to add it as a warning in some shape or form. I'm not sure how to implement this as a warning.
- Lee: in GraphQL.js I'd just make it a regular validation rule. Shopify can use a custom implementation. There are others that customise the validation rules applied.
- Ivan: okay, this should not be too complicated. Should I merge the PR with the validation rule, or do we need another discussion once implemented?
- Lee: I think it's ready for stage 2 now, having had this discussion. Go ahead and merge when it's mergeable.
- [ACTION - Lee] - Add RFC 2 tag to PR
- [ACTION] - Add SHOULD statement to spec recommending that it is generally ill-advised to add @deprecated to a non-null argument or input field unless there is a default value provided.
- [ACTION] - merge spec PR to master
- [ACTION - Ivan] - Add schema validation to GraphQL.js that enforces @deprecated on nonnull
- Matt: I'm concerned that we should have this as a MUST rather than a SHOULD - if my client is using GraphiQL, and GraphiQL hides all deprecations, I've made it impossible for my client to interact with the schema.
- Lee: that's indeed the issue. Evan's pointed out that there's a reason to not have this rule, hence the SHOULD.
- We can move from a MUST to a SHOULD but not vice versa.
- Lee: let's resolve the SHOULD/MUST concern next meeting
- [ACTION - Evan] - talk to Shopify team and others to determine SHOULD vs MUST.
- morris: the real right answer might be to implement this "future deprecated" concept.
- Evan: because we can't make that distinction, that's why we have this issue. The fact that introspection queries don't include deprecated fields by default got lost.
- Lee: the initial thinking on this was that if there was a deprecated field your client should be able to immediately move to a situation where it doesn't depend on deprecated fields.
- Benjie: do we have a champion for the custom schema metadata work? I'm very interested in it and I know Ivan's been laying the groundwork for it in GraphQL.js
- Ivan: we need two stage introspection for this, which I've been working on, but we need to move GraphQL.js to TypeScript so I've not been able to do as much as I'd like on this yet.
- Lee: I'm bumping this up to stage 2; and will make a note in the spec about this.
- Changing isFinal to hasNext
- First draft of spec edits
- GraphQL.js PR
- Rob: isFinal not being present would imply that it's true. hasNext not being there implies hasNext is false. Every query that uses defer/stream will have hasNext on it.
- We have an early first draft of spec edits. I'd love some early feedback on this.
- The GraphQL.js PR is up to date with everything in the spec, except for the ability to return async iterators from resolvers - we only allow for an array of promises so far.
- Lee: super exciting! Can you split the PR into two, one against the RFC doc and one for the spec so I can merge the RFC one.
- Lee: paraphrasing: "if you get a payload and hasNext doesn't exist, then either the server doesn't know about streaming or chose not to, so streaming is not being used and this is the final payload"
- Rob: correct
- Ivan: in Relay connection spec, "hasNextPage" is a field, so consistency is good - not a new concept. Stream is almost like pagination in some sense.
- Lee: this is great; what do you need?
- Liliana: feedback; this is our first time writing an RFC. We're working on the reference implementation, but feedback on the spec PR would be welcome.
- Lee: I'll look through the spec and provide some feedback, but it's looking detailed so I'm feeling confident
- ACTION - Lee - give feedback on @defer/@stream spec
- Ivan: I'd encourage people to experiment with this spec, e.g. by talking about it on Twitter feed, etc, so that people can start playing with it and determine if there's any issues. We need to reserve the right to do breaking changes to it though!
- Lee: this might be a good reason to be slow on the spec edits but pro-active on the reference implementation so that people can start experimenting. We'll have pretty good confidence that if this works for folks it'll be a good thing to merge into the spec (especially because Facebook are already using it). There should be an experimental flag for people to opt into it.
- Rob: that'd be great. The PR is already behind a flag.
- Liliana: the last version I published was a Relay compatible stream/defer which didn't have hasNext, but that's what we're using currently.
- Lee: I'd like to get most of this merged in and then do iterative improvements from there. Looks like it's 90% done - awesome!
- Ivan: question about server implementations; how can we use it in GraphQL.js. E.g. I don't want to have the same issue that we have with Subscriptions where it's not reachable.
- Rob: I have a PR for express-graphql that works with this.
- Ivan: great. Another great thing would be an article explaining it to the community. In GraphQL.js we have a systemic issue with documentation - it's outdated. A great accompanying thing would be an article.
- Lee: the great thing about this is that it doesn't require websockets/etc unlike subscriptions, so it should be easy for any express-graphql server to implement.
GraphQL custom Scalar PR in spec: graphql/graphql-spec/pull/649
- Evan: We're not sure what the next step is to get this merged.
- Lee: I'll move this to stage 3
- ACTION - Lee - read over this and make sure we're not missing anything, and perform the merge.
- Lee: I have 95% confidence we'll merge it as-is.
- Ivan: I'm concerned that people will read the example for UUID/etc as if it's a real thing. We should make it a URL that doesn't look official to make it clear it's an example.
- Lee: since URLs are well agreed upon and have really strong definitions (and so does UUID), it seemed like a good type to pick for the example. It would be completely fine to use this example URL.
- Ivan: if we set up the repo for common scalars, someone will add UUID and URLs to this repo even though there's already specifications.
- Lee: I don't have a problem with people pointing to the IETF specs; but I do understand your point about people cargo-culting the example. They should be real examples that demonstrate how to use it - putting example everywhere won't help people know how to use them as much.
- Benjie: I'm not super familiar with this PR, but do we need anything GraphQL-specific in these specs?
- Evan: for completeness we would probably want a document that says "reference this IETF spec, and serialize it as a string"
- Lee: for URL there's no ambiguity; string is fine. For UUID there's maybe some ambiguity because there may be a few encodings. For DateTime it's definitely an issue.
- Lee: it should be clear that you can have scalars in your own domain that are only used in your domain.
- Ivan: we should allow vendor URLs. It reduces our workload if people can use vendor-specific things - they'd be less pushy about pushing their scalars to be standard.
- Evan: I'm onboard with that.
- ACTION - Lee - make minor changes as a last patch before merging
- We'd love for GraphQL-scalars.com to evolve into something like what npm feels, where the URLs are immutable but you can easily point to the latest version of each spec. Trade off is that none of the specs would be official; it's a centralised place to store specs, to search them, and to pick one of the 13 specs for Date that best suit your needs.
- We've started a new repo for this and Andy has moved part of their initial progress into this. It's in the GraphQL org as graphql-scalars (I think).
Happy Independence Day and Canada Day!