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)
- New meeting time (2m, Lee)
- Alternate WG meeting times (15m, Andi)
- Review previous meeting's action items (5m, Lee)
- Advancing replace 'query error' with 'request error' (5m, Benjie)
- RFC: Default value coercion rules (15m, Benjie)
- Allow fields to diverge more (15m, Andi)
- RFC: Operation Expressions (15m, Benjie)
- Directives via Introspection support (15m, Andi)
- RFC: Oneof Input Objects and Oneof Fields (30m, Benjie)
- Benjie
- Stephen
- Lee: heads up, we're trying to shift times to be more amenable to the asia timezone. Next time is shifted 2 hours plus potentially daylight savings differences too. It's updated in the Linux Foundation calendar.
- Evan: the LF calendar seems to show the old time.
- ACTION Lee: I'll follow up with Brian
- Andi: there's not a lot of science behind it, just trying to have more suitable times across the world. It's kind of an unsolvable problem, so the idea is to potentially alternate over 3 or 4 meeting times. I've put a proposal in (the above issue).
- I've given preference to Europe/US because more people attend from there, but attempted to ensure that there's one good meeting time for the three different cities chosen.
- Benjie: could we reduce it to 2 meeting times to simplify things, make it easier to remember?
- Andi: I think two meeting times is still much better than one, and it looks like 2 of the times I proposed would cover most cases.
- Andi: does Lee have to be at every meeting?
- Lee: It's not a formal requirement. Maybe we can have a main meeting and satellite meetings that are more amenable to focus areas.
- Andi: one time doesn't work for all timezones.
- Michel: I'm okay if the Europe one wants to be a bit later.
- Lee: maybe we can hold the meetings more frequently and have them be shorter. I've reduced the length of the next meeting to 2 hours. Having a shorter meeting might keep us more on point. We could do one every 3 weeks or two a month.
- Andi: every 3 weeks, alternative meeting times, 2 hours
- Lee: lets shift May to afternoon for the US timezone, which puts us in the danger zone for Europe, but puts us in a better indian/asian timezone. Do this as an experiment.
- Another option is a satellite working group for Europe/Asia that's less good for the US (and rules out about 2/3 of the TSC; but Michel, Benjie, etc could facilitate) but is much better for those timezones.
- Michel: try it out, see how it goes
- Benjie: happy to do that.
- Andi: we should announce this to see what the effect is.
- Lee: we're now ready to release a cut of the spec and I'm working on a grant programme, will be announcing that soon.
- Mid-April maybe for this experiment
Review previous meeting's action items (5m, Lee)
Open Action Items Ready for Review 🙌
- #598; primary branch from master to main -> done
- #593; explaining request error - Benjie discussing today -> closed
- #592; closed
- #591; data collection techniques for finding good timezone; discussed above -> closed
- #585; examples of defaults in schemas -> closed
- #566; default value coercion -> closed
- #550; closed
- Ivan has removed everyone who is not in any team from the GraphQL org members (e.g. Facebook accounts) - roughly 20 accounts
- Benjie: In previous discussion, has been stopped by “What even is query error”. Added paragraph to RFC. Have gone over changes. One could go either way - left as is.
- Benjie: Ready to merge if others agree - I propose moving forward.
- Michel: 👍 Reduces ambiguity.
- Lee: 👍 Looks great
- Lee: ACTION - I will do a careful review and merge it.
- Lee: Need a section on errors. Maybe pull this into that section.
- Benjie: Glossary of errors could be automated with formal definitions as they appear
- Lee: Automation would be great.
- Stephen: Is the appetite for error codes in the error body?
- Lee: Yes. I think we’ve discussed this before. Where did this land?
- Benjie: We restricted fields in response and added extensions to allow space for this
- Michel: If we put code property on error
- Lee: Error codes is just tactical. How might tools take advantage of these? E.g. validation errors
- Lee: last time we talked about codes it was relevant to the GraphQL test suite.
- Andreas: Error classification in GraphQL Java: https://github.com/graphql-java/graphql-java/blob/master/src/main/java/graphql/ErrorClassification.java
- Evan: Example from graphql-ruby (each error is in its own file right now): https://github.com/rmosolgo/graphql-ruby/blob/master/lib/graphql/static_validation/rules/fields_will_merge_error.rb#L16-L20
- Andreas: Errors before execution treated differently that errors after execution
- Michel: There are discussion in HTTP group around this
- Andreas: For this spec addition (No transport specifics), request refers to GraphQL request, not HTTP request.
- Benjie: Agree with this topic. In GraphQL spec, “request” is already used to refer to GraphQL request. That said, a non-normative note would probably be helpful.
- Lee: Eros topic merits more discussion, but let’s go ahead and merge this.
- Andreas: Don’t want to block effort, but let’s make it more clear
- Lee: ACTION Benjie and I will do that as part of editorial effort
- Benjie: Haven’t written GraphQL.js PR yet. Two ways we could go. One way is as this RFC, other is a validation on default values themselves. Coercion could mean uncoerced value is invalid. Personally, more in favor of the later. Current behavior isn’t right. Fixing it the way I’m proposing is unlikely to break anything because anyone who has this issue is already dealing with it. Could affect some server-side implementations when using native objects.
- Benjie: I believe at least wording is now valid. I am a fan of this approach, but not planning to do any js impl until we decide this is the way we want to go.
- Benije: Decision is basically - should we require default values are fully specified?
- When you write it out in SDL - do you need to go through every reference and add to default? I don’t think that should be required.
- Could use tooling, but some issues with that.
- Michel: We’ve generally favored readability over writability. We should be consistent and not fill in things magically
- Benjie: I’m on the fence whether filling in automatically improves readability. When you see all default values, that adds visual noise.
- Lee: I tend to agree with Benjie. The principle of readability over writability is correct. I think this proposal aligns with that principle. You should be able to look at the default value, copy that, and use that value.
- Michel: Should we make the same argument for interfaces? We repeat interface definitions. More consistent way to improve SDL.
- Lee: A reasonable point. Interfaces are an inheritance hierarchy, especially now that interfaces implement interfaces, you might have to crawl a whole hierarchy. Intent was to remove need to crawl through documentation. I think this is similar but not as nefarious as interface problem. You need to click through anyway to look at field types. What he have here is a bug. We need to fix it one way or another.
- Benjie: This doesn’t make any change to schema. It’s only execution rules.
- Lee: Good thing to call out. More impl should memoize
- Benjie: Where to put that note?
- Lee: There already algorithm non-normative notes. Just add to those.
- Michel: If you memoize objects - they could be mutable in some languages. If you memoize, this could cause issues.
- Ivan - Not a big issue since we already do memoization of coercion for arguments. Can not repase for every resolver
- Michel: We have literals for every object so we don’t parse them. We memoize for immutable things, but not mutable objects.
- Michel: Also, this is input values. In mose things you don’t have huge trees that you coerce. Not same impact as for output data. Agree though - memoize where possible.
- Lee: I think we generally agree - this is the right way forward
- Ivan: Means a bunch of different representations of same value inside introspection
- Lee: ACTION - I will advance from proposal to draft.
- ACTION - We need to make sure that when applied to existing impls that this works
-
Andi: looking for feedback. One of the most complicated validation rules we have is about overlapping fields (doesn't force you to ensure every field is unique in the query, especially if you compose queries). Engine has to merge them together and ensure the field is only executed once. There's a rule to ensure this merging can be done.
-
Nov 2015 there was a suggestion that we could loosen these rules for object types that cannot be realised at the same time. [Reference to examples in issue.]
-
Michel: this discussion was raised by Sasha from Twitter previously. It was open to be explored.
-
Sasha: next step is to come up with an RFC. We want this pushed forward because clients have to use aliases which is tedious. I've not managed to get around to this yet.
-
Andi: it seems inconsistent at the moment
-
Matt: I have to deal with a codebase restricted by this problem. We used "Type models" at Facebook at the beginning of GraphQL. In the last 6 years we've used "Response models" - models that map what the fields in the JSON response are going to give us. It's okay if a JSON object has a key that can map to other (different) objects; but it's not so good if the same key on the object might sometimes be an integer, sometimes a string. It's impossible for us to lift this restriction, and Relay would have this problem too - you have to use aliases in order to solve this.
-
Lee: if you follow any path through the payload you should statically know what type that field will give you. It's okay for things to be type X or null, but not for it to be type X or type Y.
-
Evan: Currently, is the following query allowed by the spec?
{pets { ... on Cat { friend: catFriend { someField }} ... on Dog { friend: dogFriend { someField }} }}- Andi: Yes
- Lee: As long as Cat.catFriend and Dog.catFriend are same serialized type, it should be ok
-
Matt: this is why this validation rule is one of the slowest
-
Lee: because it has to cascade the knowledge down the tree
-
**Andi: so, in a high level sense,
>pets>friend:Cat.catFriend>catFriendNameand>pets>friend:Dog.dogFriend>dogFriendNameare okay because the fields have different names; but if they had the same name but different scalar types they would not be allowed.{pets { ... on Cat { friend: catFriend { catFriendName }} ... on Dog { friend: dogFriend { dogFriendName }} }}- Lee: Older type system might have both
catFriendNameanddogFriendNamenullable. Newer type system could have acatFriend | dogFriendtype
- Lee: Older type system might have both
-
Lee: if this rule is bringing net negative value for recent clients (TypeScript, Kotlin, Swift, etc) then we should revisit it.
-
Matt: we add a lot of extra validation rules on the client, so for Facebook we could take the "for old clients" validation rules and force them on the client.
-
Lee: yeah we already have this for Relay where technically possible GraphQL things are not allowed.
-
Matt: to do that we'd need to make a strong recommendation that clients that get built do not use response models.
-
Lee: response models are okay (e.g. in TypeScript) but in older languages they may be ambiguous.
-
Matt: this only works because TypeScript is really JavaScript and this is not typed; for strongly typed languages you'd have to implement a lookback mechanism.
-
Andi: I'm happy to understand the reasoning behind this decision, and that Sasha already raised it.
-
Michel: lifting this restriction would add value to clients because it allows for new use cases e.g. differentiating types.
-
Lee: we have this rule so that painfully sub-par type generators can support it, but if you want to allow that you can implement an additional validation rule of your own.
-
Lee: ACTION: Add prose or a non-normative note to this validation rule to explain this. (Overlapping field can be merged.)
-
Andi: adding prose would be great to explain why objects and scalars are different here. I'd be happy to
- Benjie: Mark has been proposing schema coordinates RFC. This is great. Also, I’d like to explore how this syntax could be expanded for other use cases. Gathered some use-cases. My motivation for bringing this up at WG is to gather more use-cases. Don’t get hung up on the current syntax. Aim is to understand what other things we might want to build on schema coordinates RFC.
- Benjie: Current syntax uses colon (:). A period might be used for other cases. Namespaces, for example. A period would make sense for namespaces.
- Benjie: What other use-cases might this be useful for?
- Lee: Is the intent to explore all possible use-cases to make better syntax decision in the future, or is this about adding specific syntax
- Benjie: Former. Just want to make sure that we won’t regret later syntax we add.
- Benjie: If you have thoughts now, that’s great, but also feel free to make PRs on RFC
- Mark: This looks great. Solves some of the comments people had on schema coordinates.
- Would relative expressions (e.g. for deprecation notice) be supported?
- In Schema coordinates we landed on a dot separator. Ivan mentioned this might be a problem. We saw that in js impl.
- Marc-Andree: They are notoriously hard to implement by ecosystems (consistency across implementations) but definitely a use case we've seen
- Lee:
.fieldin the context ofTypeimpliesType.field- Benjie: it kind of is...
... on Type { field }
- Benjie: it kind of is...
- Ivan: Should be considered only if it improves developer experience. For things in errors, just structure is ok. Special syntax is good if it’s intended to be consumed by users directly. Schema coordinate is used in many places (GraphiQL, Apollo tools). For more complex stuff, I’m not sure that we need to invent developer experience
- Lee: Let’s focus on todays goal. Now that you’ve done this exercise - is schema coordinates path ok?
- Benjie: Yes, much more confident with schema coordinates. Potentially a question with directives. Just wanted to see if there's additional use-cases that should be considered.
- Benije: This is effectively already used in Apollo Studio. Even use (>) symbol. Some of this was inspired by discussions with Danielle@apollo. Wanting to represent a state in GraphiQL is a use-case. Wanting to get a field in tree is a pain-point. Having a consistent syntax for doing that has value.
- Evan: Similar thing in use by Shopify’s API Health dashboard too: https://shopify.dev/concepts/about-apis/versioning/api-health Though we just use dots for everything :)
- Marc-Andre: Same here
- Ivan: Maybe a screen-shop from Apollo studio could be added as an example in RFC. Others could be added to.
- Benjie: Yes, once we get to that point, we’ll do that. Just want confidence in schema coordinates for now.
- Lee: Do we need to look at schema coordinates and consider its stage?
- Benjie: Marc-Andre mentioned schema selectors (e.g. matching multiple schema members). We might want to consider that.
- One use case that definitely needs to be considered (but comes with a ton of additional complexity) are coordinates that point not to a specific schema member, but rather a selection of schema members, matching certain coordinates.https://goessner.net/articles/JsonPath/ https://jmespath.org/
- Andreas: Custom directives are not currently introspectable. This has been a big discussion over the years. Recently, graphql-java has implemented (almost merged) the ability to query directive by introspection. This aligns with graphql.NET implementation.
- Andreas: Actual design is not so important, just wanted to mention we’ve done this and a standard for it is missing in the ecosystem.
- Michel: We copied this from your Issue. There should be some spec for this still.
- Michel: Iven was going in direction of an extension point
- Andreas: We could still do an extension point, but for this we aligned with GraphQL.NET so as not to fragment ecosystem. Agree that document would be nice. We haven’t written it yet
- Michel: We follow you so that there aren't multiple approaches to this. We can help write the RFC if you want to collaborate.
- Andi: I'm happy to hear that; I'm happy to write a spec document where we outline a spec.
- Michel: we can evolve it in a document and see where it goes from there.
- Lee: I'd like to see that.
- Andi: I won't have time or energy to get this to completion.
- Michel: I think there are a few people interested in this, we can all help.
- Andi: do we know more implementations of this?
- Ivan: Ruby allows you to add additional fields in introspection, but they don't expose directives.
- Ivan: In graphql.js I added "extensions" to types, and initially people were against it but I communicated the reasoning why and I stand by them: directives have short and nice names and are connected to SDL in a major way. I think from graphql-js users, I convinced people to start using extensions, and they're used more and more. They're not exposed through introspection. Now I see how big this problem is; we're in a domino situation.
- Lee: let's get an RFC started so we can gather implementations and reconcile them.
- Lee: let's make sure it's a problem-driven RFC: what problem are we solving?
- Lee: exposing directives through introspection might have a security concern - e.g. directives used building the schema might not want to be exposed.
- Lee: directives, if you take data and try and map it back out, it's not always in a structure that's easy to consume.
- Michel: the values in this proposal are serialised as a string that's a GraphQL object. We follow that since it'd be a pity to have multiple implementations. It's not so nice, and Ivan's approach to have an extension point to create a better output structure. In GraphQL directives we have effectively input data. We have internal and external directives - only the external directives are exposed.
- Lee: lets make sure that internal is the default for security/privacy reasons.
- Morris: in issue 300 I put the suggestion that keyword "exposed" would be added to directives similar to how we have the "repeatable" keyword.
- Michel: there are Hot Chocolate users who have authentication directives and we didn't want to expose these automatically so we have this internal.
- Morris: it's the option to extend the schema and describe things that's additional.
- Lee: this is great, we don't want to narrow in too soon on an implementation - we want to follow it from first principles.
- Andi: we let you filter out all directives programmatically. Because it's not officially specced we didn't add a new syntax for it. We follow the spec strictly over all, but this is an issue that came up too often and too much and we want to expose directives in our own projects: it's metadata around the schema that's desired. We went down the path because we felt there was no choice.
- Marc-Andree: My concern with this is that now there is additional context encoded about the "schema" that starts dealing with who is looking at it / the environment its in (internal, external). It might be useful to think of 2 schemas, one you're using internally, and one you're publishing externally. (Agreed on frameworks defaulting to filtering out directives at first though)
- Lee: I don't want to imply that exposing via directives won't work, I just want to make sure we analyse the problem and make sure that this is the right solution.
- Benjie: I’m also interested in this. I will contribute.
- Morris: I'm also interested, but I don't know how much time I have. We need something that everybody is using so that we can use it. Ping me.
-
Benjie: We’ve discussed this a lot over last 18 months. Has gone in circles. This is latest write-up for my solution which is broadly similar to my May 2019 proposal.
- Doesn’t have to be a directive. It’s a directive currently just because that’s not a spec change. We could make it a keyword instead.
- Proposal is to add types to spec for fields on input objects and arguments on output type fields that say exactly one of these things must be present.
- You may have noticed there’s not a strongly typed way to do this at the moment.
- This proposal allows specifying just one field. Generally representing polymorphic inputs. There are many potential examples where this might be used.
- This solution doesn’t change spec too much and is generally compatible with existing clients
-
Lee: Do we want to focus specifically on input objects for this RFC? For arguments short-term path is input object as a single argument
-
Evan: Other challenge with arguments is how it interacts with variables
-
Benjie: This is still a concern with input objects
-
Lee: That’s worth a test of some kind. My understanding is that this should solve it.
-
Benjie: Yes, variables should be handled
-
Lee: Need variable validation rules. Are we confident we appropriately can type variables?
-
Lee:
For Pet Input: { cat: $cat,, dog: $dog } — what are the allowed variable types for $cat and $dog
-
Benjie:
Literal Value | Variables | Coerced Value ------------------------ | ----------------------- | --------------- `{ a: $var, b: 123 }` | `{}` | `{ b: 123 }` -
Lee: https://spec.graphql.org/draft/#sel-HAHhBVNBADBAADFANznR
-
Benjie: Yes, this is overlooked
-
Lee: Except for syntax (still up for bikeshedding) this looks like we’ve landed
-
Evan: Variables only have one state (bang/not-bang) arguments also have not-provided
-
Ivan: Variables can have default values
-
Benjie: Even with default value can be nullable
-
Matt: Conflation of required/nonnull that default values fall into
-
Evan: That’s terrible
-
Lee: https://spec.graphql.org/draft/#sec-All-Variable-Usages-are-Allowed
-
Ivan: Do we need to be able to specify on input unions?
-
Lee: That’s the right question. You could make that illegal. That’s a blunt hammer. The crux of this is the type changing behavior. You specify as not nullable, but with oneOf types become nullable.
-
Benjie: This is all fine. In a oneOf, every field must be nullable. Coercion applied non-nullability constraint on one provided key
-
Lee: This is still quirky. There’s a difference in static and runtime types.
-
Michel: That would not allow setting one of them to null, but just one field. OneOf is about setting one of them.
-
Benjie: This only handles use case where one value is explicitly non-null.
-
Lee: Recommendation - write some real code and unit tests against this. Revise based on what code does to account for all of these use-cases.
-
Benjie: In summary - I think all of these concerns are addressed and this is mergeable. Would love some reads of the RFC to make sure I haven’t missed anything.
-
Michel: Just on input objects or also arguments?
-
Lee: Let’s still consider arguments, but keep late-stage RFC specific to reduce surface area of change:
-
Benjie: Makes sense. I’m glad to merge in two stages. Smaller changes is better. My main concern presently is - what syntax should we use? If it’s not for arguments, custom syntax makes sense. In introspection they’re the same thing, and directive makes sense in that case. If we were to extend to arguments, concern is that directive applied to field is effectively applied to field. Not super happy with that, but don’t have an alternative. If we want to expand to arguments, we need to address that now with our syntax choice.
-
Evan: What about
<>instead of()or{}to indicate oneof? -
Benjie: Hit me up with any alternative syntaxes
-
Lee: I’m a little nervous that in attempt to solve both, we weaken primary use case (input objects). Maybe it’s fine if we have two directives that describe similar behavior but are slightly different.
-
Lee: I might have a better solve to directive shenanigans - rule that oneOf object bodies must have one field. If applied to literals, this problem goes away. Regardless of variable or literal it should be validated in same way. Doesn’t apply for one argument. Solves for field problem and is a reason to narrow focus
-
Benjie: I don’t support that approach. I like the idea, but already we have situation of variables. We should continue and not add additional code to break that. I’d rather it behaves the same broadly.
-
Lee: Seems reasonable
-
Lee: Do we have clear next steps? Feedback collected? Concerns raised?
-
Lee: Thank you for hard work on this Benjie. We have pursued many paths, and come back to this,. I have marked this as PRoposal RFC 1 so that we can keep track of it
-
Benjie: ACTION (all) - Contribute syntax thoughts and on RFC itself
-
ACTION (Benjie) - Write an implementation in GraphQL.js
- Benjie: I didn’t file ACTIONs from last month, so there might be a lot next time