Skip to content

Latest commit

 

History

History
229 lines (206 loc) · 19.3 KB

2020-10-01.md

File metadata and controls

229 lines (206 loc) · 19.3 KB

GraphQL WG Notes – October 2020

Watch the replay: GraphQL Working Group Meetings on YouTube

Agenda

  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. Query ambiguity: discussion of replacement terms (25m, Benjie)
  7. __typename and subscriptions (5m, Benjie)
  8. Schema Coordinates RFC check in (5m, Mark)
  9. Tagged type update (15m, Benjie)
  10. graphql-js working group short update (5m, Uri)

Determine volunteers for note taking (1m, Lee)

  • Benjie
  • Mark

Update on action item tracking (Benjie)

  • We’ve not been disciplined filing action items recently
  • Would be useful to file them, keep track where they came from so we can see progress of each meeting’s action items, see older items get closed
  • New Github project, each meeting action goes into a project
  • When someone thinks their items is ready for review, there’s a review
  • Intention is to have the item get closed during working meeting
  • Already gone through past action items, entered them and closed them
  • ACTION - Benjie - split 507 WG issue, so GraphQL.js task goes into separate issue. [This particular task was done.]

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

  • All action items
  • @directive on input fields merged into graphql-js but blocked on TS issue
  • dotan volunteering to help
  • Should be split into multiple actions
  • _Marked as closed _

Query ambiguity: discussion of replacement terms (25m, Benjie)

  • Spec edits/glossary graphql-spec#777
  • Original issue graphql-spec#715
  • Previous discussion - May 2020
  • Issue with people new to GraphQL find - hard when you’re trying to write Documentation for query that “query” can be refer to multiple things
  • Gone through the entire spec, looked for all instances of “query””queries” etc, put them into buckets for meanings.
  • Made edits in PR777 so we can see the changes in context
  • In a lot of the spec, we refer to queries but we mean all operations (mutation, subscription, future things)
  • Changed a lot of instancesof “query” to “operation”
  • Surfacing this for discussion
  • [Lee] Query also has meanings outside of graphql - most people know it to be a “request”. Want to make the terminology more “crisp”.
  • [Lee] Curious to see benjie’s perspective on
  • [Benjie] More natural to talk in terms of “query”. Makes text easy to parse, but potentially harder to understand
  • Gone deliberately too far in spec edits to highlight this
  • In the wider ecosystem, folks refer to root fields as “queries”. Beyond the spec, it’s overloaded everywhere.
  • If we can come up with a way of reducing ambiguity without going quite as far as I have in this PR, that would be beneficial
  • [Dan] Could make non normative note “if we mean only queries, not subscription etc we’ll say so - otherwise we’ll could just query”
  • [Mark] Document is also used to represent “thing the client sends to the server”
  • [Evan] Spec is a technical document, let’s pick specific meanings and define them upfront. Clarity trumps accessibility in a technical doc
  • [Lee] I’m with ya. We hope it’s accessible. Non technical person should be able to start at the top and read through it. But being crisp is super important.
  • Sometimes when we say query, we literally mean a query operation
  • Appreciate approach of taking it to the extreme, then scaling it back
  • [Dan] “What if we replaced everything” vs “non controversial version”. Spectrum of changes that are clearly better vs could be confusing. Let’s land the uncontroversial ones, then we can iterate from there
  • Adding surrounding context could make prose unambiguous
  • [Benjie] Like the idea of doing uncontroversial changes first
  • Instances where it leaves things undefined e.g. where it talks about “query execution”, but it’s literally giving an example using the query operation. So mutation and subscription behaviour technical undefined/ambiguous
  • [Dan] Evan could you talk about query.query.query.query
  • Worst ambiguity comes from looking at HTTP request spec
  • Discussed as part of graphiql/http working group. Should be “Document”.
  • [Dan] historical aspect - we used query b/c query was originally the only thing available
  • [Benjie] Content type headers might introduce transparent rewriting of query -> document, which would be cheap
  • [Dan] “Query” is easy to understand in context. Document/request/operation are not.
  • [Mark] Any value in looking at other domains? E.g. HTTP spec. Always uses “GET request”
  • [Benjie] Part of my edits are to ban “naked” instances of “query”, should always be put in context
  • [Benjie] Looking to get feedback on glossary.
  • Action item: pulling non controversial edits in PR
  • Action item: should we put glossary into a PR for the spec
  • [Lee] Glossary should be part of spec.md. Good long term direction
  • [Evan] Non controversial bit of this is glossary + non normative note to say “query” is ambiguous
  • [Lee] Would love to see us be able to use “query” in an unambiguous way - usually a query operation
  • [Ivan] We use these terms in algorithms. Should be totally unambious
  • As non native speaker, confusing to have different types of queries. Easy to have document, operation which distinguishes things.
  • [Lee] Action item: Lets have folks look at the PR

__typename and subscriptions (5m, Benjie)

  • If you’re doing a subscribe operation with graphql-js and subscribe to __typename field, you get an error cos it doesn’t return an async iterable
  • This doesn’t make sense it’s not real time information
  • Getting an error back doesn’t make sense either
  • Is this just an edge case we don’t care about? Or should we….I don’t know! Doesn’t seem right.
  • If you send a regular resolve/execute to the subscription type for that type, it will resolve. It’s not that it can’t exist on the subscription type, it’s just that it can’t be subscribed to.
  • [Michel] It is an edge case. No value to this. Maybe just disallow it?
  • [Benjie] maybe put a non normative note in and say “this won’t work”
  • [Lee] Tricky. Rare logical inconsistency. There’s a rule that says you can always use __ typename in a selection set. Leaving it be/having a note might be good approach
  • [Ivan] Feel like we should support __typename. Have a property that we always want to be able to figure out what the type is. Right now we’re pushing down a corner case. Needed for introspections.
  • [Michael] what do we do with __typename? Could up a stream that has one item then finished…
  • [Ivan] Could change the spec to require subscriptions to require a non introspection field, otherwise it’s a validation error.
  • [Michael] You want to add new validation rule
  • [Benjie] Could ban subscriptions with just __typename. It already doesn’t work. Rule is that it must have at least one non introspection field
  • Maybe in onegraph - __typename was being added accidentally.
  • [Lee] Spec already calls out that introspection fields are counted in the rule for selection set count.
  • On board with just outlawing this.
  • [Benjie] Great! That’s what the PR does
  • Agree with ivan that introspection everywhere is important
  • [Lee] Can anyone think of where this will hurt us in long term?
  • [Benjie] It already doesn’t work!
  • [Andi] Open an issue and close it again to make sure this issue is searchable
  • [Benjie] Already opened a PR with spec edits
  • Conclusion is the PR to close this loophole
  • [Lee] Make it a proposal. We can tidy up and merge. Let’s change the validation rule.
  • ACTION - Benjie - add this rule to the validation rule for subscription operations (both in spec PR and GraphQL.js)

Schema Coordinates RFC check in (5m, Mark)

  • graphql/graphql-spec#746
  • RFC is filed.
  • Next action is to turn this into an actual spec edit; but I'm not sure how.
  • Is this an appendix? Maybe one shared with non-execution notes/etc.
  • Lee: Spec's job is to declare what is/is not GraphQL, and this doesn't really do that, which explains why it's not clear where it goes. I can imagine it being an appendix, or being folded in where we introduce the concepts of where to find them. It could also be related to errors (machine generated errors that want to refer back to places in the schema) - that might be a good place to introduce this idea. Not sure if any of these are the right idea, they're just ideas.
  • Mark: thanks. I think I'm leaning towards the appendix, putting it in the errors section of the spec might not make sense from clients such as GraphiQL where the docs show schema coordinates.
  • Mark: I'll forge ahead with the appendix approach.
  • There's a few further ideas: what do we do with enums? It seems we should do the same as with type and field, e.g. FooEnum.BAR.
  • Lee: my only thought is if you want to make it obvious you're referring to an enum vs a type.
  • Mark: I don't believe you can have the enum name used in other types.
  • Lee: all types within the schema have a unique name, regardless of what type they are.
  • Lee: if you see Direction.north - is that an enum value, or a type field, or an input field? It's a trade-off, and maybe this is the right direction, I just want to make sure you have an opinion on this.
  • Mark: great point; I think that this resolves to a concrete type means this is okay.
  • Lee: that it's not ambiguous is great. For what it's worth, I think it's probably the right move.
  • Lee: one thing that might be helpful is to list out all the possible types, and all the things within them, and list out how they might be described. What does it mean for a union? It's okay to say it doesn't make sense to refer to a member of a union with a schema coordinate, but we should evaluate it up front.
  • Mark: that makes sense. Another thing I wanted to call out is the idea of wildcards and the wider, more generalised selection language. I want to make clear that this spec is not that, but I also don't want to close the door to that in case someone wants to work on that in future.
  • morris: what does this mean for directives? Another one for the list, perhaps.
  • Lee: what does it look like to reference the argument of a directive? It could make sense: start it with an @ sign to make it clear.
  • Mark: I'll think about it.
  • Lee: find all the places that we allow you to use documentation in the type definition language - everything that can be documented might want to have a schema coordinate.
  • Mark: do you define the definition of the directive, or where it's applied?
  • Morris: and directives can be repeated.
  • Lee: you should be clear whether coordinates are pointing to things within a document or things within the schema; that clarification might help.
  • Mark: to be clear, we're only referring to schemas.
  • Morris: there's still usages of directives in schemas, not just definitions.
  • Mark: yeah, agreed.

Tagged type update (15m, Benjie)

  • RFC #733
  • Benjie: Decent amount of feedback, thanks
  • One of the most recent changes - won’t allow for input and output. You will declare if it’s input or output tagged type.
  • Written some justifications to allow tagged type for output. Want to solve problems now.
  • Don’t think interfaces enter into this too much. Rules are relatively clear.
  • Comes down to - why would you use a union vs a tagged?
  • Tagged type more flexible.
  • Tagged is one type that declares if it’s valid for input or output.
  • Michel: doesn’t feel clean - lot of checks - if you introduce output types into tagged, it becomes output type. Lot of validation you have to do.
  • Benjie: That was all previously proposed - but now in the bin. Now you have to explicitly declare. You can look and see immediately if it’s input or output, and you don’t care about the rest of the schema
  • Lee: Valuable to make these the same thing
  • Benjie: Stripped down version of intersection of input/output base types
  • Don’t have to change introspection, can use members
  • Michel: If we take arguments away, looks similar.
  • Benjie: From implementation point of view, follows existing input vs output type logic. Validation/resolution works in the same way. Think it should be relatively easy to implement.
  • Michel: Kind of. Resoler translates into method. Raw data just properties. Distinct different things you have to have. Getter vs setter. Different things.
  • Benjie: Tagged type doesn’t need resolvers. Default resolver would be what you use. Parent field returns an object that confirms to tagged type. Implementation can decide if it wants to implement resolvers.
  • Lee: FB’s union type had a default resolver behaviour to make sure there was always a matching GQL type
  • Lee: Would it make sense to ever limit directive to only apply to output vs input tagged?
  • Maybe there’s a case where you want to say something specific about a tagged member
  • On every field right now, you can use if and skip. Not sure if we’ll want this for tagged
  • Andi: Not super happy with RFC - appreciate work and see the validity, but non clear schema design questions around interfaces vs unions. Adding another thing into this mix would be a problem.
  • Andi: Concerned about process - jumping a bit ahead. Applied a lot of scrutiny to input union over years. Output type of this is confusing.
  • Benjie: I broadly agree - my approach is go all in, then pull it back. That’s what i’ve done here too. We may want to allow output version of this in the spec - but maybe a separate process. Should still be evaluated because type could support both.
  • Worried getting into const vs let vs var territory.
  • Lee: Wise to consider them together, overall vision. Might want to split this up into two halves though.
  • Often, user wants to get data from API, manipulate it and send it back
  • Symmetry is nice so you don’t have to do mental gymnastics between input and output types
  • Michel: That’s how you often start, but as schemas grow, they move apart from that.
  • Benjie: GraphQL is used by companies large and small
  • Lee: Like allowing starting with symmetry, allowing them to grow
  • Next step: articulate how all these things come together, then tease them apart

adhoc: meta notes discussion (everyone)

  • Writing down notes is hard
  • We upload videos of WG meetings to youtube
  • Videos are more helpful
  • These notes are just to make things textual searchable
  • Can we do speech-to-text?
  • Sometimes it’s gibberish

graphql-js working group short update (5m, Uri)

  • Ivan: Uri wants to chart the action items and create a roadmap.
  • Want to encourage contributors; had a GraphQL.js working group call (see YouTube), and we got a couple volunteers - The Guild Members and a couple people agreed to triage issues, create roadmaps, etc. It's risk free (no code changed, no published packages) so I went ahead and gave these volunteers triage roles. We'll see how the experiment goes.
  • There's a lot of items on me from the last meeting, but hopefully in future they'll be more evenly distributed.
  • Should we have a common policy for approving new maintainers? Rules for onboarding new maintainers. One set of rules for all projects? Each project decides on its own? I don't feel I can make this decision.
  • Lee: we're in a chicken and egg problem; normally you only want to define these rules once you've extracted the patterns of what works/what doesn't. We should treat them all as one-offs because volumes are low. It shouldn't depend on me to approve people because I'll cause a roadblock; same if it is dependent on a committee. Each individual project should be able to self-govern except when there's situations where they can't make the decisions.
  • Ivan: season of docs added contributors to GraphQL over HTTP (10?). Maybe we should have a rule that if someone has not contributed for a year, we should send them a notice to ask if they want to continue - if they've not responded within a reasonable time (or decline), we should remove them. Is that reasonable?
  • Lee: yes.
  • Ivan: one year?
  • Lee: yes. Some projects are on a slow cadence, so a year seems reasonable. You can also use your own judgement.
  • ACTION - Ivan - send notices to existing contributors to GraphQL.js
  • ACTION - Ivan/Uri - write a template for this notice to be polite with good explanation.
  • Uri: Thanks Ivan for making this WG happen; I think the most important thing is to have a regular cadence.
  • 3 biggest topics:
    • 2 related to "bus factor"
    • 1 was "how can we make Ivan's work better"
      • what do we need to do to minimize your work when you need to review something?
    • Can we get more Ivans?
      • who can review/commit/etc
  • We have a bunch of tasks here, and they're documented in the GraphQL.js WG issue.
  • Everyone is encouraged to join.
  • Last point: the most important thing I think is a clear roadmap; a lot of people want to contribute and there's a lot of changes we're doing, but because of the bus factor we're not sure what is the next thing to do to make things to happen. Currently this means just more work for Ivan; hopefully after the first wave of tasks it will lead to having less work for you and more work can be shared.
  • Ivan: sharing some perspective, about trying to onboard more people, you need to do these things in the right order; you need contribution guides, you need to explain, if PR is not reviewed for a long time, people get demotivated; so if you don't do this it makes it hard to get and keep maintainers.
  • So these are the rules:
  • First: what do you expect from PRs? What do you expect from people?
  • Second: What help do you need? What topics should people work on?
  • Third: Onboard people.
  • I'm the bottleneck right now, but I will work on this to help encourage more contributions and help us move faster.

Any other business

  • Ivan: Would like to have an asynchronous working group for GraphQL over HTTP. We'll share our results.
  • Michel: working on issues rather than meeting up?
  • Ivan: yes.