2020-09-03.md

GraphQL WG Notes – September 2020

Watch the replay: GraphQL Working Group Meetings on YouTube

Agenda

1. Introduction of attendees (5m, Lee)
2. Determine volunteers for note taking (1m, Lee)
3. Review agenda (2m, Lee)
4. Review previous meeting's action items (5m, Lee)
5. Allowing deprecation of inputs (10m, Evan)
6. TypeScript Migration Progress in graphql-js (5m, Dotan)
7. How to bootstrap graphql-js working group? (5m, Dotan)
8. GitHub infrastructure for managing Working groups (10m, Ivan)
9. @defer/@stream (15m, Rob/Liliana)

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

• Specification Membership Agreement
• Participation Guidelines
• Code of Conduct

Determine volunteers for note taking (1m, Lee)

• Benjie
• Antoine

Review agenda (2m, Lee)

• Benjie: Tagged type: thanks for feedback, please keep it coming. More work required from me.
• Ivan: GraphQL Foundation:
  • attempt to unify GraphQL JS and GraphQL parser
  • Graphql.org subscription is open - 3month effort
  • Attempting to answer all the FAQs - both technical and non-technical
  • Contributing on issues and PRs very welcome
• Lee: this is great, documentation is super important; it would be great to have someone leading this effort.
• Ivan: Orta is helping to maintain graphql.org

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

• All action items
• Lee: Would love a volunteer to incorporate the input union RFC, for the next steps. Essentially ownership.
• Benjie: I've not got time to do that right now due to work on the Tagged type/etc.
• Evan: if it doesn't need doing soon, I could look into it mid September
• Lee: Would like to get it by the end of the year, no expectation on time though.
• Still finalizing the membership agreements; Brian is chasing these down. This is the first cut within the GraphQL Foundation and we need to ensure it's by the books.
• Benjie: We talked about having a review bot? What is the status?
• Lee: it's coming, Brian knows what tech to use, but he needs to front-load that tool with a number of already signed membership agreements so that all the existing contributors don't get another point of friction - particularly important when they're contributing as part of a larger organization. Most existing contributors won't even need to know the tool is in place because it'll automatically know them. The aim is to make it so future specs can be cut without having to chase down agreements.
• Ivan: I've been merging stuff into the spec/etc; if you need me to stop let me know.
• Lee: If you see that contributions come from new contributors, you can flag and let Brian know.
• Ivan: There are a lot of one-time contributors, which makes the problem a little challenging.
• Lee: It’s a good problem to have, if you could do a quick check before merging and give them a link to the contribution agreement.
• Ivan: if someone wants to help, adding a checklist in the issue template would be a great contribution
• ACTION - Lee: mention this issue template idea to Brian

Allowing deprecation of inputs (10m, Evan)

• graphql/graphql-spec/pull/525#issue comment-685934302
• Evan: summarized comment: only question left is whether the spec should allow deprecating an input that is required. Two concerns: deprecations are intended to be actionable; introspection does not include deprecated things by default - could break tooling that's no longer receiving deprecated inputs as part of their introspection results.
• Regarding case 1: not much of a blocker; the action from deprecation doesn't need to come from a schema (e.g. it could be switch to a new version/endpoint)
• Regarding case 2: much more of an issue because we could be breaking clients. My suggestion is to return required arguments even if they're deprecated - introspection should always give you what's necessary even if you say you don't want it.
• Benjie: Since this would be a new argument added to the input, I don't see a major issue with adding ?. Maybe this can be solved with naming, using something else than includeDeprecated.
• Evan: includeDeprecated where is that argument placed in the introspection query? Benjie was suggesting we would have to add it to the input arguments. It’s added within the arguments of fields fields(includeDeprecated: Boolean = false): [__Field!]
• Benjie in chat: https://spec.graphql.org/June2018/#sec-Schema-Introspection
• Benjie: the fields field and the enumValues field on the __Type type get passed an includeDeprecated argument; we can use a different name when we add it.
• Evan: The other option beside renaming would be to change the default value, defaulting to true.
• Benjie: but would the name do something other than what it says?
• Evan: No; because it defaults true we can make it do what it says in the tin.
• Ivan: one issue is consistency; general question about breaking changes: if you just update graphql.js or other libraries, everything should work as before unless you start using a new feature. For example deprecating arguments or input fields, it's okay if it will break clients if the owner knows about it - they can choose to not use this feature. Should we care if we break something if the owner has to explicitly choose to use the new feature?
• Evan: it's not something where clients can change their behaviour; deprecating an input could have unknown effects across the ecosystem forever.
• Lee: What I would like to know is whether this is a use case outside of Shopify use case. Is this a general problem that people would run into, I’m worried we are rotating around this specific use case.
• Evan: I don't have examples of other schemas right now. Here's an example though; you've written your schema and now you want to rewrite in a different backend language, so you add it to a new endpoint and you want to get clients to move from myschema.com/v1 to myschema.com/v2 and you want to notify users of the old schema which fields are now deprecated.
• Lee: this is a legitimate hypothetical use-case; I suspect that most who found themselves in this situation would do a two phase migration - first remove everything that's not supported, then point to a new URL, then add the new features. Whatever we encode in the spec becomes the default way of doing things, which sets precedent for the way people build things. What elements do we want to evangelize and encourage, and which things are Shopify specific and we want to limit the impact of that on the GraphQL ecosystem. It's just this one change that concerns me.
• Evan: long term prediction is that more people will version their schemas like we do at Shopify; I think people will run into the problems that we have and solve them in the way that we have too.
• Lee: I don’t mean to say the entire approach you are taking is wrong. There are details of it we can take with a critical eye.
• Different URLs for different versions makes sense; but keeping the original ones immutable is an over-constraint. Maybe better to alter the old schema and give the field you want to remove a default value.
• Evan: making the argument default without anything else causes a lot of knock-on effects.
• Evan: I see the value in having a clearer path and a stronger forcing function for making people migrate their schemas better.
• Lee: For what is worth I’m open to have this as a SHOULD instead of a MUST. If there is a switch that checks whether it’s deprecated or not. I’d rather loosen what’s required in the spec rather than include all deprecated values by default because this will hurt in the long run.
• Evan: if we have a SHOULD then we might not use it anyway because it could still break clients.
• Lee: maybe we need to revisit the exposed metadata proposals so that we can indicate "shopifyDeprecated" via this method?
• Ivan: I agree we're mixing two things: deprecated in this version and you can migrate inside this version; whereas Evan's is more of a "you should migrate to the new version" in a different schema. It would be useful to show these schema extension tags inside GraphiQL with text messages.
• Evan: GraphQL has a very explicit vision for how schemas should evolve, and that's not the vision that we went with; I'm okay working around this at Shopify. I think GraphQL's vision does and will work for many companies, we're a bit of an edge case.
• Evan: we mark fields as deprecated even if there's no replacement in the same schema graph - you have to move to a different schema version.
• Evan: we wanted to give ourselves the option to do hard and fast migrations when needed whilst giving 3rd party developers a way of migrating when they're ready.
• Lee: it's worth a conference talk!
• Evan; TL;DR: sounds like we want to go with MUST rather than SHOULD. Happy to get it out of the way.
• Ivan: action point; can we gather statistics on how people are using introspection - are people using introspection in production clients? Or is it just GraphiQL/Postman/etc? Maybe you can gather this information from Shopify?
• Evan: we don't have Development vs Production distinguished from clients
• Lee: I'm worried we'd get stats that we don't know how to interpret. We should think about what we're enabling/disabling in design rather than breaking/not breaking in usage.
• Lee: if you still wouldn't use it with a SHOULD then it sounds reasonable to keep it a MUST.
• Ivan: GraphQL.js PR was up to date a couple weeks ago; can I merge it?
• Lee: I think it's safe to merge. Make sure it's inclusive of these decisions.
• Ivan: yes, it has a validation rule for this - I updated it after last WG
• Lee: do we have the ability to change the spec proposal?
• Ivan: yes, you have commit rights to push to his repo.
• Lee: I’d like to only increase this from stage 1 to stage 2 once there is content for the spec proposal.
• Ivan: we should create a branch on spec repo, merge to branch, create PR to branch, so whoever wants to update the language can send a PR there.
• Lee: Since you’ve been working on graphql.js change do you want to champion this spec change proposal?
• Ivan: I need a native language speaker to review changes; I've pinged Benjie before and he's been really helpful with it.
• Benjie: 👍
• Lee: ACTION - Ivan - get the GraphQL.js change in and organize updating the spec texts.

TypeScript Migration Progress in graphql-js (5m, Dotan); How to bootstrap graphql-js working group? (5m, Dotan); GitHub infrastructure for managing Working groups (10m, Ivan)

• Dotan: Ivan - can you please share an update? next steps, blockers, and how can we help
• Ivan: One blocker I wanted to bring to the working group, I tried to move the work into [??].
• We need a feature freeze for a couple weeks. Working on merging things and migrating at the same time is too hard.
• Dotan: we can do the TypeScript work on a branch and keep rebasing every day.
• Ivan: TypeScript migration will be a breaking change. Let's discuss it on a call since not everyone's interested in JavaScript implementation. I think it's important to allow new contributors; Flow is a barrier for many people to contribute. TypeScript is easier, I've heard from many contributors.
• Ivan: I should also contact Matt about Flow support because we need to figure out how to migrate to TypeScript without breaking Flow clients (at Facebook specifically)
• Dotan: any issues, if you need help, we can put people to work on it with you. If you need help, we are here.
• Ivan: I need to learn to enable people to work on smaller chunks, "help-needed", etc. Summer of Code/Summer of Docs is helping me to develop these skills.
• ACTION - Ivan - organize a call with Dotan to figure out a strategy.
• Ivan: plus Lee, I see you're trying to do the same with ImmutableJS, so would be great to discuss.
• Lee: I'm not doing a great job on that right now, but yes.
• Lee: one thing that might help is giving Dotan/Uri committer rights, so long as you're in tight communication. Do you need help scheduling the call? Brian might be able to help with the mechanics of setting up a GraphQL.js specific meeting.
• Ivan: we have a number of people with commit rights and no clear criteria on when we can remove/add them, so that's something we need to discuss with Brian. Particularly there are still a lot of people from Facebook.
• Lee: I agree that we need that eventually, but it doesn't need to be a blocker. I'm more interested in setting up a core consortium of committers that are code reviewers/committers. Next step is to set up a meeting.
• Ivan: I suggested 2 weeks. We've started a newsletter for GraphQL foundation; I think it would be good to put this in there to help us be open to new contributors.
• Ivan: we have GraphiQL WG and GraphQL-over-HTTP WG (now under GraphQL Foundation) and [....] WG, we have Input Union WG, and we'll be getting the GraphQL.js WG. That's 5 WGs at the same time. If we create a repo for each WG...
• Input Union is an example of temporary group with clear end goal; if it has its own repo it will be dead repo on GraphQL GitHub organization. I suggest we create a different GitHub organization for this, and this means we can script things, be more liberal with commit rights, etc. What do you think about this idea.
• Dotan: I think this sounds fine. One repo or multiple depends on features. I suggest ZenHub; a board ontop of GitHub. Everything's managed with GitHub issues; you can have your own internal status and an option to share it to the public. Might help to track progress on tasks.
• Lee: good point. I support more tooling to help us get contributing, but I'm hesitant to add a new org because of discoverability.
• Lee: I want to make a distinction between WGs and sub-committees; we should have broken it out and kept notes and agendas as a folder rather than one massive file. I'm happy to consider a lot of these things sub-committees; I could go either way with GraphQL.js being a subcommittee.
• Lee: hopefully this solves the issue; subcommittees meet for a while and leave behind a folder full of notes.
• Ivan: with GraphQL.js a lot of the issues come from the JS ecosystem. It's taken me a long time to review @stream/@defer, but there's not normally times when GraphQL.js blocks [??]. A lot of the issues are about GraphQL.js as client for JS rather than GraphQL.js as reference implementation for GraphQL.
• Lee: for anything that's an independent WG, a separate repo in the org makes sense. One org helps with discoverability. Otherwise we have to help people navigate the multiple GitHub orgs. Also folder structure helps; if it's tied to a project or spec then it makes sense to have a folder in the same repo that tracks the agendas/notes/etc.
• Ivan: problem with that is rebasing, CI/CD, etc. We have 11 separate flows in CI right now, it's a bit overkill for adding an agenda item. We also want to generate changelogs; Babel, etc all have separate repos.
• Lee: true
• Ivan: even for GraphQL-over-HTTP we have way more commits for WG folder than for rest of the spec. It dilutes progress, and makes a noisy git history which is hard to browse for relevant changes.
• Lee: sounds like separate repos are the right thing to do; I support that.
• Dotan (Chat): GitHub Actions has ignore-paths feature
. You can ignore the entire directory of meetings.
• Ivan: that's a partial solution; generating release notes from changelog, and other things we'll constantly be solving other issues. I think it's two separate processes.
• Dotan: we can help with that, we're doing similar things in our repos.
• ACTION - Ivan; Ivan: I can create a repo and add you there; copy structure of GraphQL WG.

@defer/@stream (15m, Rob/Liliana)

• RFC FAQ
• GraphQL-JS support AsyncIterable resolvers
• GraphQL-JS support for @defer/@stream
  • Updated to export new functionality from experimental directory
• Spec edit PR
• express-graphql PR
• GraphQL-over-HTTP RFC PR
• Rbo: Link in the agenda RFC FAQ, trying to answer the “why we don’t support Defer on field question”. The reasoning for that is the original Apollo implementation was only on fields. We ran into problems in that it’s difficult to coordinate. Ivan brought up the argument that you could make the same argument using Skip and include.
• Michael: Going down with the FB solution is a much cleaner way to implement defer and stream.
• Matt: from a Facebook perspective, I believe we tried to add @defer/@stream on fields to start with; we found 1. it wasn't needed; 2. most people wanted to add defer/stream at the fragment level. Not doing @defer/@stream on fields made it much easier to write the spec since it reduced the changes needed. For now we're deferring field deferral until it's needed.
• Rob: That’s what we came to as well. We wanted to discuss this with the group.
• Ivan: you can force any field directive to be on an inline fragment. You can apply @skip/@include on a group of fields. Important in the spec is consistency. If it makes it hard for client libraries to handle it, we know we need to remove it during the experimentation phase. If we never add it, we will get an inconsistent API with the other directives. It seems like a pretty simple change in GraphQL.js; not sure about Relay or other client libraries. We need to support multiple inline fragments anyway; it's probably only 20 lines in GraphQL.js to achieve this. (Maybe 50-100 with tests.)
• Rob: I agree it's not hard to implement on the server side. It's consistency with other directives vs a more minimal spec.
• Lee: I'm less worried about consistency here because there's very few directives in the spec. It's not my intention that each new directive we add to the spec has to match the previous ones. Your point about only applying to fragments and not fields makes sense (I was confused initially). With @skip you could add to multiple fields and know that they'll all either be there or not. The same is not true for @defer; this has a different impact on your UX. Designing around the pitfall is wise. Why do I have to wrap a fragment? It's to make the path from deferring one field to two fields easier.
• Ivan: we should add a note in the spec because I'm not the only one who will have this question; especially language implementers will have these questions. I agree with the arguments, but it needs to be addressed in the spec. The spec is good at giving intention, not only specifying but explaining why.
• Michael: Apollo tried this with a field and also outlined all the problems they faced, so you can use this as a base
• Lee: I think the issue with returning null for a non-nullable field here is something they could have worked around, it sounds like a tooling issue.
• ACTION - Rob: Rob: I'll update the RFC PR with a more thorough explanation and then we'll get that into the spec as well.
• Lee: I agree with Ivan that we should have a non-normative note; the more places we explain it the better.
• Ivan: one question - when reviewing the PR, I noticed there's an implementation-level similarity with subscribe. People question why we have execute vs subscribe function. I want to look into can we unify subscribe/execute/stream/defer to reduce the number of return values. Not sure if it's outside the scope of this RFC, but maybe we can think about if we're returning async stuff in execute, maybe we can unify algorithms for execute/subscribe.
• Rob: we can do that; I'd like to do that after support is merged into execute.
• Ivan: yes, sure, it's an experiment for a reason: we're not 100% sure on the result. An end result of this RFC is that async results will be part of execute. We don't have to figure it out at this point. Maybe hasNext makes sense in a subscription context?
• Lee: what other help do you need for defer/stream.
• Rob: we're working on getting async iterator merged. Ivan suggested there's a memory leak, so we're looking at getting things merged into iterall. And generally we need more eyes on the spec.
• Andreas: do we have specific criteria for saying it's ready to be merged into the spec.
• Lee: yes, we have a phase guideline
• Andreas: do we have a more specific criteria for this specific project?
• Lee: good question. Since this is a huge change to the surface area of GraphQL, we need to make sure we have a high degree of confidence in the reference implementation.
• Andreas: I'd like to require that we have a second implementation outside of GraphQL.js before we merge it. We want to make sure that the assumptions we're making are valid outside of JS users. It doesn't block anything and we don't need it right now, but I think it's important to consider adding this as a formal requirement.
• Lee: I'm comfortable with that. I know it's been built in collaboration with the non-JS implementation in Facebook, but if you want to add this to GraphQL Java great.
• Michael: we're targeting October to release this in .NET. It'll have similar challenges to GraphQL Java.
• Andreas: in previous implementations it was hugely underspecified in the transport layer. This needs to be tacked explicitly.
• Rob: I have an RFC about this against the GraphQL-over-HTTP spec; Benjie pointed out an issue that we're addressing.
• Andreas: great. We're looking forward to the .NET implementation too to get feedback; not sure when the Java implementation will get around to it. We removed the Apollo version.
• Ivan: I want to clarify: I'm not merging this PR not because of a problem with the spec, but because JS AsyncIterable has a problem with a leak, so purely technical reasons. You can implement this completely independent of JS; I don't expect any changes to the Spec based on the JS implementation, so you can go ahead.
• If you want something from GraphQL.js, you can join the GraphQL.js working group; e.g. if we can make your life easier.
• Rob: happy to join the GraphQL.js meetings.
• Lee: anything else you all need?
• Rob: I think that's it!
• Lee: it's super exciting; thank you all for continuing to move this forward!

Scalars subproject (Andreas Marek)

(Late addition to agenda)

• There's been no progress; please contribute and give feedback. We have a repository to discuss issues/etc. Hopefully other find this project useful and want to contribute, I think it has the possibility to have a huge impact, but we're currently lacking traction and engagement.
• How can we make this better?
• Michael: the GraphQL scalars website?
• Andreas: the repo inside the GraphQL GitHub org https://github.com/graphql/graphql-scalars/
• Lee: yes, the website.
• Andreas: if we want to merge one, it needs to be approved by the GraphQL Foundation (the submitter has to have signed the paperwork); we need a way of implementing this.
• Lee: this came up earlier in the meeting. Brian has to do the paperwork and the tooling, so he's working on both.
• Ivan: the project doesn't currently have a license. Merging things without a license could be problematic.
• Andreas: that's why I've opened this issue. graphql/graphql-scalars#7
• Lee: GraphQL foundation gives us clear legal rules so people know how they can use these scalars.
• Andreas: agreed; this is really nice.
• ACTION - Lee: I'll follow up with Brian and make sure he's prioritising this.
• Lee: it'll be the OWFa license
• Ivan: attribute to GraphQL Contributors, not GraphQL Foundation.
• Ivan: what's the proper repo to put template license?
• Lee: there's a legal repo at the top. It's called "Foundation" https://github.com/graphql/foundation
• Lee: markdown document; you can ask Brian for help and he'll work with his legal team to make sure it's done correctly.

Any other business

• Abhimanyu: when I insert three nodes via a mutation, should they be returned in the same order when I query them?
• Lee: are you asking if you query a list multiple times should it be the same order?
• Lee: that's totally implementation specific
• Abhimanyu: so even if it's not insertion order it's okay?
• Lee: I don't think the spec even states anything about the order.
• Abhimanyu: the spec says it's an ordered list
• Lee: that means a consumer of the list should use an ordered list. Beyond that, the particular order that's used, is completely implementation specific.
• Abhimanyu: next question; with input unions.
• Benjie: If you have a better solution, bring it to us please ;) The RFC is currently being updated. I’ve been focusing on updating the types RFC itself, it is the one that is replacing the solution 5. We decided that the better way to solve this is to add a new type. You are welcome to contribute or propose a solution.
• Abhimanyu: We will wait then for the types to be completed before starting to use them.
• Benjie: The union type contract is it will return one of the possible types. It’s not dependent on the data on the nodes themselves as long as they conform to the types they belong to.
• Abhimanyu: Another question, we changed our implementation to allow concrete types implementing interfaces to omit declarations of the interfaces fields. Any reason why not to, any problems that could arise?
• Lee: this has been discussed many times in the past. Many reasons. One is living in an object oriented type system for too long and having to follow the chain to see what's implemented. We wanted to avoid that and allow you to see the types directly on the object. The other piece is that it's important to clarify intent. You may specify a field that's more specific than the interface defines. For example if the interface says you have a field with no arguments, you may add arguments to some of the object types. If you say a field on the interface returns a union, then the field on an object that implements that interface could choose to only return objects of one type within the union.
• Lee: also it's more important to be readable than writable; we want to avoid confusion when people are reading the GraphQL schema even if it needs more effort to write it.
• Matt: we abuse the SDL and have automated transforms that make sure the SDL is ultimately spec compliant. It's up to you what tooling you'd like to use.
• Benjie: I'm still making progress on Query Query Query from a few WGs ago, but it's below Tagged on my priority list.
• Ivan: where's the video for the Input Unions WG?
• Benjie: they're on YouTube; at least the latest one
• Ivan: wasn't there a recent one?
• Lee: no, the last one was 3 months ago.
• Ivan: ah, Benjie's notes - better late than never!
• Benjie: :D
• Lee: I want to make the GraphQL Spec repo only deal with spec changes, and discussions should go into the WG repo.
• Ivan: if we merge RFC into WG repo but we don't have PR against the spec, how does someone track changes?
• Lee: good point; I'll add something to the README to make it clear that pro-active discussions around change take place in the WG repo. The reason I propose this is that it gets us one step closer to a master list of all the topics under discussion, who the champions are, and what the progress is. That's my intent with unifying these things.
• Andreas: this gives more value to all events from the spec repo. This would make the signals more valuable.
• Benjie: should we disable issues on the Spec repo and only have issues on the WG repo?
• Lee: good question. Don't know. I think we get high quality issues. We use issues for the WG for internal WG actions. Maybe it makes sense to keep this separate, I don't have a strong opinion.
• Andreas: I like the idea, but maybe it's too far and discourages opinions?
• Lee: for clarity, I think issues for Spec RFC issues make sense in the Spec repo; it's the documents that get merged (the RFC documents) that make sense to extract out so that the git history seems clearer.
• Ivan: if we write a document with policies, we can have more people help with merging.
• Lee: we can be more liberal with giving merge access to the WG repo. Anyone who's a frequent guest of these meetings we can give commit access to.
• Ivan: especially the champions. You just promise to not merge anything in other people's RFCs, only in your RFC.
• ACTION - Lee: make this happen