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)
- RFC: __typename is not valid at subscription root (10m, Benjie)
- RFC: Schema coordinates (10m, Mark)
- To talk about Foo.baz.bar vs Foo.baz(bar:) (by popular demand)
- RFC: Default value coercion (20m, Benjie)
- Review previous meetings' action items (15m, Benjie and Lee)
- Benjie
- Mark
- Benjie: would like to call for a Input Union WG meeting
- Ivan: we have a stage 3 proposal to discuss
- Lee: we'll add these to the agenda
- Benjie: It’s not valid at the moment! It validates, but it never gives you a successful result...no async iterator. Always gives you an error. Confuses tooling, because it thinks it’s valid.
- Want to make it explicitly clear rather than an accident. Proposing a slight modification to the spec. Got some feedback.
- Benjie: Main aim to gather feedback. I’ve done a basic implementation in graphql-js, gonna talk to ivan about some issues.
- Dan: Main aim is “this is weird”. Subscription object is already weird (async). This [change] feels right to me.
- Lee: Any dissenting opinions? What’s the cost of this change?
- Yaacov: I think it’s great to point this out. I ran into this as a bug from my users, expecting this to work. Fine to change the spec to match the implementation, but better to change the implementation to match the spec? Going forward, seems like it can’t hurt to allow this to actually work.
- Lee: need to decide how it would actually work.
- Yaacov: Just return “Subscription”? Like any other type?
- Lee: Quirky cos all the fields don’t return values but they return async iterators, or streams of values. Maybe this one is a special edge case that only returns one value…
- Yaacov: I think that would work
- Matt: Long term, likely makes sense for us to rethink subscriptions and have multiple root fields. Brings it in line with everything else. We don’t allow that yet, would be gnarly. Bringing spec inline with current expectations feels more right than current state.
- Yaacov: If I had more time, is this something people are interested in seeing? Would be willing to work if I had time. Not the biggest deal, there are workarounds.
- Lee: Gotta make sure this makes for all implementations, not just reference implementations. Want to make sure this would be viable for many.
- Evan: Are there any other introspection fields that we’d also want to support on the subscription root if we wanted to handle __typename?
- Yaacov: It’s harder for me [to implement], familiarly is typescript not flow
- Ivan: Tradeoff for yaacov is that it becomes more complex to handle special cases. What benjie proposes is resolution of ambiguity. Alternative is adding new features.
- Lee: Good point - makes it clear we’re not faced with either or. We could continue with what benjie is proposing. Safe to allow something not being allowed to being allowed - other way is not safe. Could follow up with yaacov’s proposal.
- Yaacov: I’m ok to bring spec back into line, then propose this as change.
- Lee: Anyone have any problems?
- Benjie: this is to do with the one field. What you’re proposing at the moment opens the door for multiple fields. That brings about the hydra that is what happens when we have multiple event streams? What happens to __typename then?? Here be dragons
- Lee: Much bigger effort. We limit selection set to one because it makes it easier to pull the introspection field out.
- Mark: Schema coords:
User.name< a formal specification for this - Lots of discussions on the spec edits - thanks!
- Seems we're at the dotting i's and crossing t's phase.
- Main discussion: referring to arguments in a field; what should that look like?
- Imaging
type Query { searchUsers(name: String) }reference field withQuery.searchUser - Referring to the argument; is it
Query.searchUsers(name:)orQuery.searchUsers.name? - Parens look like what's in the query, and reduce ambiguity, but I'm generally ambivalent
- Evan: I've been arguing for the dots format; I've been thinking about it as a list so using simple separators for everything in the list makes sense to me. Benjie has given some counter examples that show some ambiguity, so I'm coming around to that, but I'm not a fan of the colon.
- Lee: why do we have the colon?
- Mark: some galaxy brain thing we were discussing, also it looks like Swift ;)
- Evan: if we want to list multiple arguments, we should use a comma.
- Lee: [anecdote about commas ascii-art in early GraphQL]
- Mark: I think the main reason for adding the colon was to reduce ambiguity
- Lee: that's part of the reason I like it - it doesn't look like a value being passed to a function, makes it clearer it's the left side of the colon rather than the right side.
- Lee: if we only care about terseness then dots wins, but if we want users to read it then legibility is more important than terseness - people should be able to look at it and intuitively know what it's talking about.
- Evan: we mostly use paths (through a query) rather than schema coordinates.
- Mark: when GraphiQL shows results it does things like
Query.user: String, so there's a possibility that it may need to be updated. - Lee: if you just use the dots then you don't know if the thing after that is returning to the return value or the type the argument accepts.
- Benjie: we should try and keep in mind that though this is a Schema Coordinates spec, we should try and make it so people can repurpose it for other usage in future.
- Lee: I think we should keep the colon because it makes it a lot clearer that we're referring to the parameter name rather than passing an argument.
- Ivan: can we do it with a callstack instead?
- Benjie (chat): @evan
Mutation.doThing(input.user.name:) - Evan: we currently refer to this as
Mutation.doThing.input.user.name, but I really like that syntax Benjie. - Lee: we need a delineator between what field is in use and what the path is inside the arguments
- Evan: okay; I'm sold.
- Lee: we are directionally aligned, which is good. Let's see how far we can get working it through in the RFC
- Mark: bikeshedding... we landed at "schema coordinates" because that's what GraphQL Java did, but I wonder if people outside this group might prefer something more flashy, e.g. "schema selectors"? I'm not tied to the name
- Lee: I'm also not tied to the name. We should do a poll! It's your proposal, rename it if you want to.
- Ivan: it's currently schema coordinates, but if in future we want to do the route that Shopify is using then it's selectors because it's selecting it based on a field and path rather than being an unambiguous reference to the item.
- Benjie: I really like the 1-to-1 mapping of the coordinates, selectors may be a broader thing in future.
- Benjie: I was looking through the spec….there’s some interesting things that go with default value. [Explains the PR]. This feels like a bug in the spec.
- Lee: I agree this doesn’t seem quite right. There’s a reason why default values don’t go through coercion: It introduces the ability to create this pattern. 1: wanted to make sure it’s statically nullable (you don’t have to execute anything to know). 2: you want to be able to read the query. If you see default values and it looks like an an empty object, you might be confused when you see it actually has a number in it cos it had a default. There’s a twist - either we have to repeat ourselves, or do something like you proposed. You mentioned you could pass in number 7 and that’s what you get. But you can’t because there’s a validation rule, and 7 != 3. Empty object does pass validation because the number field is empty. It’s a bug, because it’s empty but deemed as a valid type, so it’s missing a step.
- Maybe all values should require manual compression at runtime. Downside is default changes, forgets to update types….maintenance risk. Idk if it’s better, but wanted to toss out alternative solutions
- Benjie: What you said about seeing what the default would be from a user point of view....i’d like to challenge that! I don’t think it would be confusing. Effectively saying if you don’t specify this, same as
- What happens if I specify it but only give one of the properties?
- Tooling issue - even if introspection says one thing, tooling could still show defaults. It’s what graphiql does. Issue beyond actual introspection issue.
- We should make it do what people expect it to do. Should be the same as if user typed in an empty object.
- Matt: Idea that empty input object means fields provided have default makes sense - we need a way to specify some fields but server can decide the rest. You need a way for that always to be true. Have example with no input object be same as example with input object set to empty. What benjie is proposing.
- Benjie: I think this a non breaking change. We have to have already put these default values in place...should still coerce the same. Think this is a non breaking change.
- Ivan: Need to research current behaviour for query variables - in spec we don’t coerce default values of query variables.
- Benjie: We do - query d in the issue. Shows example.
- Ivan:
- Andreas: Problematic area - we gotta do something. Graphql-java doesn’t do same as graphql-js. Generally not easy to implement correctly or same way. I’m not sure about consequences of proposal. General comment: I don’t like implicit nature creeping up. You define empty thing then suddenly there’s a default value somewhere else. Breaks good boring mental model of graphql. We prefer repetition over jumping around schema, instead of implicitness.
- Lee: Could be meaningful differences
- Matt (chat): This also brings up a devex issue: there’s no way for a client to have a variable for an argument, where that variable results in “use the server default value”.
- The only way to implement that is to allow the argument to be null, then have the server decide at execution time that null is the same as some default value.
- Matt: We almost never use schema level default values. Actual code on the server decides what the default is if you pass in a null. Default values assume server tells you what default will be.
- Lee: Two algorithms for coercing. Issue only changes coercing of variables, but we should double check there aren’t differences across both algorithms.
- Benjie: Covers both argument values and variable values. Argument values is really the one that needs fixing.
- Lee: You have change in both places. This is probably the right path forward. We should fix this quickly. As matt says, breaks the contract between client and server. Also potential security / stability issue, if you can crash a server that doesn’t handle NPEs.
- Benjie: @ivan this would be a bigger change to graphql spec than this would be.
- Andras: What does introspection return for this?
- Benjie: empty object. This isn’t really a valid schema. Default value doesn’t get coerced. Up to the schema writer to make sure it’s the case.
- Lee: Introspection is doing the right thing here.
- Lee: Marking as stage 1, seems directionally good.
- Ivan: graphql/graphql-spec#701
- ACTION - implementors, implement ;)
- ACTION - Benjie: work with Ivan to add this to GraphQL.js
- Need implementations in place before we can move this to stage 2
- Benjie: closing issues (https://github.com/graphql/graphql-wg/issues?q=is%3Aissue+label%3A%22Action+item+%3Aclapper%3A%22)
- Andreas: We had a graphql scalars meeting, we have a meeting as a follow up
- Lee to own GitHub infrastructure for WG (? citation needed)