Skip to content

Latest commit

 

History

History
221 lines (191 loc) · 18.8 KB

2019-09-12.md

File metadata and controls

221 lines (191 loc) · 18.8 KB

GraphQL Working Group Meeting 2019-09-12

Agenda Attendees

Action Items

  • [Ivan] Write proposal for how graphQL cats can be integrated into graphql-js
  • [Lee] Every agenda item should have a link (e.g. RFC, PR). Add this requirement to the agenda template.
  • [Lee] Open issue on github to discuss meeting time
  • [Mike] Publish a post about interface implementation
  • [Lee] Sync with James B (Apollo) on interface implementation
  • [Mike] Move interface proposal to Stage 2
  • [Vince] Summarize input union problem statement with concrete examples
  • [Vince] Create a new issue for input unions
  • [Lee] Close outstanding input union issues and link to new one
  • [Matt] Sync with team about Facebook Identity and figure out what we want to put up.
  • [Lee] Post time suggestions for next meeting

Notes

Previous Action Items

  • [Unclaimed] Add maintainer list for relevant and breaking changes, send to reference library maintainers
    • [Ivan] Was under the context of lexer
      • Need to notify everyone affected when breaking changes are made
      • [Ivan] I can take this for next meeting
  • [Lee] Grant Erik collaboration access to libgraphqlparser
    • [Lee] technically done
    • [Erik] some hangups with the CLA
    • [Lee] reaching out to some lawyers to clarify the ambiguities, consider it not done yet
  • [Lee] Contact GitHub and see what we can do about moving the GraphQL cats repo into the GraphQL org.
    • [Lee] Progress here, but not done yet. I am still working on this - optimistic that it will happen.
    • [Ivan] Last time we discussed GraphQL cats in general
      • [Ivan][ACTION] Write proposal for how graphQL cats can be integrated into graphql-js
    • [Tanay] offered to help with typescript, as this is partly blocking (?) the gql cats work

[Ivan] Suggestion - every agenda item should have a link (e.g. RFC, PR)

[Lee][ACTION] add this requirement to the agenda template.

GraphQL WG 2020 schedule (Lee, 10m)

  • 3 more meetings, cadence so far has been 1st Thursday of the month, 9am Pacific
    • Working pretty well
  • Cadence: How do we feel about this cadence? Do we want to continue this or propose changes?
    • [MikeM] 👍 frequently enough that missing a single meeting isn’t terrible
    • [Michael S] Monthly is good. If we met more frequently we wouldn’t have enough to discuss
      • Ivan: +1
  • Time: Pick a new time? Rotate time?
    • This time works time for India
    • [Matt M] The people with the worst problems probably aren't in the meeting to comment

    • We should consider graphq-java folks - this meeting is 2am for Atlassian (Australia)
    • [Ivan] Can we open a github issue for this?
  • [Lee] Propose -
    • Keep cadence
    • [ACTION] Open issue on github to discuss meeting time

Lexer lookaheads (Lee, 10m)

RFC 1 RFC 2

  • [Lee] What’s new:
    • Feedback previously: We should move to RFC stage 1 and look closely at remaining pieces
    • PR on graphql-js merged
    • Another PR awaiting feedback
  • Are we ready for RFC stage 2 or even stage 3?
    • [Ivan] Concern about negatives. Prefer general solutions, but want to avoid breaking changes
    • [Lee] Not too concerned with syntax change being breaking even if we wanted to add math in the future
    • Are there any issues for AWS AppSync or other impls?
      • [Antoine] Ok with the change, just need to determine exactly what will break. It’s on us to come up with a versioning system (we don’t have that currently). This change is OK with us.
    • [Lee] Would be surprised if this actually breaks queries in practice. These queries would likely already fail validation. Intent is for malformed queries to be caught by the lexer.
    • [Lee] Any other support/assent?
      • [Michael S] Support
      • [Ivan] Stage 2 makes sense. No need to rush to stage 3.
    • [Lee] Drafts are still open to feedback - may change. Accepted means that it is not expected to change.
      • Let’s move this to Stage 2 and gather more feedback
      • Move to Accepted after getting more feedback

Discuss status of interface implementation by interfaces (Mike, 10m)

PR RFC

  • Context allows interfaces to use implement interfaces
    • Not to big of a change because there is not any actual inheritance
    • Types need to completely describe all fields
    • Interface implementing an interface needs to re-describe all fields
  • [Mike] One open discussion item
    • Do we require re-declarations of implementations? (A) Yes (B) No
    • Ivan and Lee have differing opinions here. Mike is indifferent.
  • [Lee] Principle for managing changes - Change of Least Cost
    • What would the cost for getting this wrong? What would the cost be to fix it?
    • It’s easier to change is if we require explicitly stating transitives. It would be a non-breaking change to move to inferring transitives later.
    • It would be more difficult to move back to explicitly stating if we discovered complications with inference
  • [Ivan] A clean breaking change can be simpler and easier to explain
    • example: because we adopted sdl in stages, we a while we had multiple accepted syntaxes and this causes confusion.
    • Some people generate schemas. It will breaking things even going from option A to option B
  • [Ivan] Explicitness of fields doesn’t lose anything. Explicitness of interfaces loses hierarchy
    • [Mike] I don’t think that we lose hierarchy information
    • [Ivan] example: adding an interface requires more changes and creates a large changelog and
  • [Ivan] Why would we prefer option B? Is it only a checkpoint before going from option A to option B?
    • [Mike] Not sure that we need option B at all.
    • [Lee] I agree with Mike here. Intent of the change isn't to have deep hierarchies; it is to allow type checking of interfaces. A big diff is a good thing. I think it is a weakness of traditional OOP that a seemingly insignificant change can have broad implications. Adding to interfaces would already be a breaking change. Not sure how much we should be concerned with breaking downstream/external schemas.
  • [Ivan] Agreed that intent is not about deep hierarchies. Change is still useful. Want to hear other opinions on two approaches to interface hierarchies.
    • [Michael S] For those coming from an OOP background it’s easy to explain why we need to re-state fields, but less obvious for interfaces.
    • [Matt M] The teaching aspect is a reason to get it right. If we were (hypothetically) 80% certain that we’d end up at option B, we should go ahead and do that. If we’re confident that option A is the final explanation, then let’s do that. If it’s 50/50, we need to be closer to consensus before proceeding with either option. IMHO breaking people out of the mental model that GraphQL is an OO programming language is good.
  • [Mike] RFC should stay as Draft (Stage 1 or 2) while we first roll it into reference inpl. I don’t see any harm in rolling this into reference impl and we can look for issues and gain consensus.
    • [Lee] Stage 2 ready - merge into graphql-js and then validate with real data
  • [Erik W] I like explicit (option A)
  • [Evan] In Shopify’s internal impl, we already have this (option A). We can already do this without schema changes - what is the value in this?
    • [Mike] Animal example
    • [Mike] Relay Connections Spec Example
      • Can’t currently describe a Connection - it must implement Node
    • [Benjie] https://facebook.github.io/relay/graphql/connections.htm#sec-Node


      • An “Edge Type” must contain a field called node. This field must return either a Scalar, Enum, Object, Interface, Union, or a Non‐Null wrapper around one of those types. Notably, this field cannot return a list.
      • [Evan] yeah, connections of scalars seem are a valid, important use case
we have a bunch of them :P
      • [Lee] It's a common request from clients to know which connections contain identifiable (cacheable) nodes
    • [Lee] Currently, you’re sometimes forced to use objects or unions
  • [Ivan] Proposal - majority seems to be in favor of option A. Don’t want to block; I’m ok with continuing to RFC stage 2. Lexer change is easily propogatable. Worried about things depending on client extensions (e.g. offline modes, Apollo federation). Want to wait for community (tooling, etc) before Stage 3
    • Before merging and announcing, publish an article explaining what this is and why it is how it is.
    • [Mike][ACTION] I can write something and put it up on Medium
    • [Matt M] We’ll likely start using this as soon as it’s available
  • [Ivan] This is a big change, should be RFC 2 for ~ a year
    • [Lee] I would prefer not to stretch this out. 6 months is already long. Idea has been out there for ~2 years already.
    • [Lee] YAGNI. I want to be sure that we need this.
  • [Lee][ACTION] Sync with Apollo on this - specifically James
  • [Mike M][ACTION] I’ll take a look at changes and move to Stage 2

Discuss policy for breaking changes to typescript definitions (Mike, 10m)

  • [Mike M] DefinitelyTyped. Checks that my definition is correct before exporting it. Lots of value for typescript users. Do we have a policy for breaking changes for typing?
    • [Ivan] Anything that can break should be a breaking release. Entire ecosystem is synchronized with Node releases. In January we’ll drop Node 8. For 15.0.0, we’ll discuss with Apollo and other community members. We can’t break anything in 14 branch. We can start on 16 branch. We can have alpha releases. Submit an issue and reference your branch.
  • [Benji] Would like to challenge Ivan’s comment on breaking changes with typings. We’re moving GraphiQL and other things to typescript. Breaking changes when it comes to typings that are more specific should not be considered breaking even though they are technically breaking.
    • [Ivan] We should discuss. Doesn’t need to be working group necessarily
    • [Lee] Benji, I agree. Sometimes a type change is only breaking for code that was already wrong. There’s probably space to allow a type error to show up and say that it’s not breaking. (Note: Lee has a few reservations about semver ;)
    • [Tanay] This shouldn’t break things [Mike] That would have been true, but now types are included in graphql-js

Input Union (Vince, 15m)

RFC

  • [Vince] Options are listed in RFC. Principle of Least Change is useful. Any other thoughts?
  • [Lee] Need to balance Least Change with making the Right Change :)
  • [Lee] My preference - near the top of list of options
    • New kind of type that is a list of all things
    • _typename field is discriminator when necessary
    • Easiest to teach
  • [Vince] Next step to put algorithm into spec language?
    • [Lee] 👍
  • [Ivan]
    • Take the things we agree on and put into single RFC
    • Next step is to put examples of what problems are solved
    • People want different things from a feature. We should evaluate proposal against common problems.
    • We discussed this a couple meetings ago.
  • [Lee] Agree. It would be useful to list common problems. Also the ability to have a Scalar in input union was discussed previously.
  • [Ivan] Concrete examples should be provided.
  • [Vince][ACTION] Summarization of problem statement with concrete examples.
  • [Lee] Animals was a bit contrived, but Relay Connections is known to be an actual use-case that is run into.
  • [Benji] Solution may not meet all use-cases and that’s OK. Each distinct problem statement deserves its own document, and RFC can just reference which ones it meets and which on it doesn’t
    • [Lee] Agree, but let’s keep it co-located in the context of a single doc.
  • [Ivan] Acceptance criteria on RFC - Merge anything that people agree on. Get feedback on other things. No need to wait on Working Group meetings. It’s great if most problems can be resolved offline.
    • [Lee] If there’s multiple Issues for an RFC, pick one, close others and link to them.
    • [Benji] Agree, but should be a clean thread. I think each individual problem should be merged into the RFC document as a single PR, that way people can use GitHub reactions to indicate support for that particular problem - may help as a way of understanding which problems are more important/popular than others.
    • [Vince][ACTION] I’ll create a new issue for this. Would prefer is someone known in community closes outstanding issues
    • [Lee][ACTION] Message me, I’ll do that.
    • [Ivan] When we close an Issue, make it clear that discussion has moved to RFC. Make sure that we don’t lose any information/open questions.

Facebook Identity work (Matt, if time)

  • [Matt M] Mostly a heads up. FB is iterating here.
  • [Matt M] Could be a facebook-specific problem because of their early experiments/abuses of Node and ID.
    • We (FB) need a common way across clients - given some object instance, how are we keeping it consistent? Given this instance, how do we track an update to it?
    • Current implementation is spec compliant (uses directives)
    • Likely to release in relay as an alternative to current scheme.
    • Is it even possible to ask server for an updated instance of this thing? Is it ephemeral?
  • [Evan] Can you be more specific about why ID is not sufficient?
    • If you designed correctly, maybe ID is sufficient. FB has a lot of legacy use cases
    • Not sure if it’s just a FB problem or if it should be pushed to open source.
  • [Matt] It also could be very expensive given only an ID. IDs might not be unique across entire object space
    • [Ivan] Don’t encode typename in ID?
    • [Matt] Correct. That would be another way to solve this. But it will probably end up in Relay as an alternative to just ID. Given where we are, it is problematic at FB to change ID format.
  • [Mike M] Interesting problem. I do a lot with offline, and this is applicable. How do we follow this?
    • [Matt] Not ready to be a proposal quite yet.
    • [Matt][ACTION] Will sync with team and figure out what we want to put up. Likely an issue on the spec that describes our use-case. Might never become part of spec.
  • [Stephen] Apollo’s @key directive introduced in the federation spec is tackling a similar issue. Any likely convergence there?
    • [Matt] When we started on this ~6 months ago, we weren’t even sure what the scope of issues would be. We’ve kept tabs on Apollo spec, but haven’t used that. It looks like impl details will probably be different. For example, we have no interest in compound keys - adds complexity to the problem space. Not to say open source recommendation wouldn’t fall closer to @key directive.

Wrapup

  • [Evan] In future, could we prep the agenda a day in advance so everyone has a chance to read and get a little context on agenda items like this in advance?
    • [Lee] 👍
  • [Ivan] Lee gave a great talk on the future of graphql a few years ago. It would be really nice to have a follow-up/outcome on a lot of these experiments. Maybe from somebody who still works at Facebook?
    • [Michael] there’s a good discussion in #relay about some of these things
    • [Matt] from facebook, some of it is still experimental, some of it is in production but not final, still up in the air
    • [Lee] there’s a whole series of fascinating blog posts that we should write up some day
  • [Michael] We are not colocating community specs such as HTTP spec
    • Would be great even if it’s just a bunch of links.
  • [Matt] On batches specifically, FB is moving away from it. The ability to have queries with multiple root fields wasn’t originally possible. A different API/language isn’t needed anymore.
    • {Michael] Still useful on mutation side, e.g. for dataflows. Not as much just for delaying data loading. In any case, would be great if information was colocated and you could find this information.
    • [Lee] Agee on HTTP we should move it somewhere easier to find. Some experiments are intentionally harder to find.
  • [Ivan] Working group repo - PRs sit until next working group. This is practical, but when people look at the page it gives a bad impression when agendas and notes are missing/delayed. Can we distribute the work so that the page can be more up-to-date?
    • [Mike M] Agree. I could have been more prepared if agenda items/link had been merged
    • [Lee] I will get to PRs sooner. Aim for Monday prior to next meeting. Anyone with commit rights should feel free to merge. If anyone wants to volunteer to help, send me a message.
    • [Matt M] Proposal: Require that context is provided 48 hours ahead, otherwise pushed to next meeting
      • [Lee] No strong feelings on this
      • [Jafar] We did something similar on TC39
  • [Evan] Working on Bulk GraphQL variant
    • Context: 3rd parties hit our API and they need all products. Instead of needing to paginate, they can make a bulk request and we’ll do the collection work and give a list of results.
    • Maybe shopify specific. Is there any interest in this going further?
    • Is this like stream directive? [Evan] No, this is not currently spec compliant.
    • [Lee] Interesting. If you want to collect more use-cases and move things towards an RFC, we could discuss in a future meeting.
    • [Lee] This doesn’t concern spec, so let’s keep the spec repo clean. The working group repo can be a space to discuss more ideas like this.
  • [Lee][ACTION] Will post time suggestions for next meeting