Finalize December notes (#1214)

benjie · web-flow · commit 1131cdb2977a · 2022-12-16T08:54:06.000Z
diff --git a/notes/2022/2022-12.md b/notes/2022/2022-12.md
@@ -307,3 +307,248 @@
   meeting.
 - Separate between 2 issues: 1. when to ignore / not ignore and 2. What info to
   give the server to handle response.
+
+# Secondary meeting - APAC
+
+## Agenda
+
+- Agree to Membership Agreement, Participation & Contribution Guidelines and
+  Code of Conduct (1m, Lee)
+  - [Specification Membership Agreement](https://github.com/graphql/foundation)
+  - [Participation Guidelines](https://github.com/graphql/graphql-wg#participation-guidelines)
+  - [Contribution Guide](https://github.com/graphql/graphql-spec/blob/main/CONTRIBUTING.md)
+  - [Code of Conduct](https://github.com/graphql/foundation/blob/master/CODE-OF-CONDUCT.md)
+- Introduction of attendees (5m, Lee)
+- Determine volunteers for note taking (1m, Lee)
+- Review agenda (2m, Lee)
+- Review previous meeting's action items (5m, Lee)
+  - [Ready for review](https://github.com/graphql/graphql-wg/issues?q=is%3Aissue+is%3Aopen+label%3A%22Ready+for+review+%F0%9F%99%8C%22+sort%3Aupdated-desc)
+  - [All open action items (by last update)](https://github.com/graphql/graphql-wg/issues?q=is%3Aissue+is%3Aopen+label%3A%22Action+item+%3Aclapper%3A%22+sort%3Aupdated-desc)
+  - [All open action items (by meeting)](https://github.com/graphql/graphql-wg/projects?query=is%3Aopen+sort%3Aname-asc)
+- Licensing & final legal sign-off for GraphQL Scalars project (5m, Donna)
+- Review a few active approved simple PRs and merge
+  [974](https://github.com/graphql/graphql-spec/pull/974),
+  [975](https://github.com/graphql/graphql-spec/pull/975),
+  [980](https://github.com/graphql/graphql-spec/pull/980),
+  [981](https://github.com/graphql/graphql-spec/pull/981),
+  [982](https://github.com/graphql/graphql-spec/pull/982),
+  [983](https://github.com/graphql/graphql-spec/pull/983),
+  [985](https://github.com/graphql/graphql-spec/pull/985)
+
+## Determine volunteers for note taking (1m, Lee)
+
+- Benjie
+- Matt
+
+## Review previous meeting's action items (5m, Lee)
+
+- [Ready for review](https://github.com/graphql/graphql-wg/issues?q=is%3Aissue+is%3Aopen+label%3A%22Ready+for+review+%F0%9F%99%8C%22+sort%3Aupdated-desc)
+- [All open action items (by last update)](https://github.com/graphql/graphql-wg/issues?q=is%3Aissue+is%3Aopen+label%3A%22Action+item+%3Aclapper%3A%22+sort%3Aupdated-desc)
+- [All open action items (by meeting)](https://github.com/graphql/graphql-wg/projects?query=is%3Aopen+sort%3Aname-asc)
+- Benjie: No updates on action items
+
+## Licensing & final legal sign-off for GraphQL Scalars project (5m, Donna)
+
+- Donna: Thanks to whoever connected the URL! Still need to sort out the license
+- Table to end of meeting
+- Need to assign task/action item onto Lee to deal with
+- Donna: if there’s a way for me to help out, I’m happy to pitch in.
+- Benjie: open PR, don’t actually merge, and have Lee push the button
+
+## Review a few active approved simple PRs and merge [974](https://github.com/graphql/graphql-spec/pull/974), [975](https://github.com/graphql/graphql-spec/pull/975), [980](https://github.com/graphql/graphql-spec/pull/980), [981](https://github.com/graphql/graphql-spec/pull/981), [982](https://github.com/graphql/graphql-spec/pull/982), [983](https://github.com/graphql/graphql-spec/pull/983), [985](https://github.com/graphql/graphql-spec/pull/985)
+
+- P30: looks good to Benjie
+- 981: Need to make an editorial decision for the spec capitalization (CREATE
+  ACTION ITEM)
+- 983: Action item on Matt Mahoney: review and help improve wording so we don’t
+  impose “this is really hard” emotion
+- 985: The sentence is not as accurate as it needs to be, but to fix it we
+  should address all instances of "parallel" in that section.
+- ACTION: Matt - provide specific suggestion on how to address 985
+- 984: [long discussion around why this is a _selection set_ and not a _mutation
+  operation_]
+  - Benjie this defines serial execution of a selection set, rather than
+    explicitly execution of a mutation operation.
+  - Matt's example shows an inline fragment spread, to show it's the selection
+    set not the operation that's important. \
+
+```graphql
+mutation M($newBirthday: Int, $newAddress: Int) {
+  ... {
+    changeBirthday(birthday: $newBirthday) {
+      month
+    }
+  }
+
+  changeAddress(address: $newAddress) {
+    street
+  }
+}
+```
+
+    * Matt: Maybe runnable examples in the spec would be helpful?
+    * Benjie: perhaps we should add a title or caption or a non-normative note that strongly indicates this is a selection set, not an operation/document (and maybe gives an example of a full operation). \
+
+Matt: "selection set" is meant to imply "part of operation" _ ACTION: Roman
+didn't read this as a selection set but instead as a document and felt that
+"mutation" is missing - we need to make it crisper and clearer that we're
+talking about selection sets here. _ Matt: we use selection sets throughout the
+documentation; this is where it jumped out to you, but if we fix it here we need
+to fix it everywhere where selection sets are referenced. \* BRING TO JAN WG:
+process for getting TSC eyes/approvals on small PRs that have at least 1
+approval.
+
+## TSC Elections
+
+- Nominate yourself!
+- At least three of the five currently-being-elected seats are not going to be
+  filled by incumbents
+- Votes early in January
+
+# Secondary meeting - EU
+
+## Agenda
+
+1. Agree to Membership Agreement, Participation & Contribution Guidelines and
+   Code of Conduct (1m, Lee)
+   - [Specification Membership Agreement](https://github.com/graphql/foundation)
+   - [Participation Guidelines](https://github.com/graphql/graphql-wg#participation-guidelines)
+   - [Contribution Guide](https://github.com/graphql/graphql-spec/blob/main/CONTRIBUTING.md)
+   - [Code of Conduct](https://github.com/graphql/foundation/blob/master/CODE-OF-CONDUCT.md)
+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)
+   - [Ready for review](https://github.com/graphql/graphql-wg/issues?q=is%3Aissue+is%3Aopen+label%3A%22Ready+for+review+%F0%9F%99%8C%22+sort%3Aupdated-desc)
+   - [All open action items (by last update)](https://github.com/graphql/graphql-wg/issues?q=is%3Aissue+is%3Aopen+label%3A%22Action+item+%3Aclapper%3A%22+sort%3Aupdated-desc)
+   - [All open action items (by meeting)](https://github.com/graphql/graphql-wg/projects?query=is%3Aopen+sort%3Aname-asc)
+6. Field error resulting from insufficient validation of variables (15m, Benjie)
+   - [spec#1002: Issue with reproduction](https://github.com/graphql/graphql-spec/issues/1002)
+7. Add a style guide to the GraphQL specification (15m, Benjie)
+   - [PR](https://github.com/graphql/graphql-spec/pull/1003)
+8. Update from stream/defer breakout group and new proposal (20m, Rob and
+   Benjie)
+   - [Proposal](https://github.com/robrichard/defer-stream-wg/discussions/52#discussioncomment-4380643)
+
+## Determine volunteers for note taking (1m, Lee)
+
+- Hugh Willson
+
+## Review previous meeting's action items (5m, Lee)
+
+- [Ready for review](https://github.com/graphql/graphql-wg/issues?q=is%3Aissue+is%3Aopen+label%3A%22Ready+for+review+%F0%9F%99%8C%22+sort%3Aupdated-desc)
+- [All open action items (by last update)](https://github.com/graphql/graphql-wg/issues?q=is%3Aissue+is%3Aopen+label%3A%22Action+item+%3Aclapper%3A%22+sort%3Aupdated-desc)
+- [All open action items (by meeting)](https://github.com/graphql/graphql-wg/projects?query=is%3Aopen+sort%3Aname-asc)
+
+## Field error resulting from insufficient validation of variables (15m, Benjie)
+
+- [spec#1002: Issue with reproduction](https://github.com/graphql/graphql-spec/issues/1002)
+- If passing null the variable default doesn’t kick in
+- Can ultimately lead to a runtime error
+- Seems like it should be a validation error instead
+- We thought this was fixed, but we only sort of fixed this (and kinda
+  backwards)
+- It sounds like we missed something here
+- The motivation to get this right was to explicitly disallow nulls from making
+  it through a well typed boundary; could be a security risk if we allow them
+  through
+- The code in the issue was evaluated from the point of view of the spec and
+  from a running test case, so it’s a real issue
+- Is there a straightforward solution here - when providing null it’s
+  interpreted as not provided; default values could override null
+- Or, we move the assertion to be later in the process
+- Solution here could be breaking
+- Queries that were previously valid would no longer by valid; validation rule
+  would complain, so this is probably a non-starter
+- What was previously erroring, feeding null in, does what it says and coalesces
+  into the default value
+- If we coerce to a default value it becomes impossible to actually pass null in
+  so this could be breaking as well
+- Breaking change that relates to the validation rule seems like the better
+  choice here, but this might only be fixable when we’re ready for a version
+  bump
+- We have a note about this in the spec:
+  [http://spec.graphql.org/draft/#note-ba9c8](http://spec.graphql.org/draft/#note-ba9c8)
+- Details:
+  [http://spec.graphql.org/draft/#sec-All-Variable-Usages-are-Allowed.Allowing-optional-variables-when-default-values-exist](http://spec.graphql.org/draft/#sec-All-Variable-Usages-are-Allowed.Allowing-optional-variables-when-default-values-exist)
+- Origin:
+  [https://github.com/graphql/graphql-spec/pull/418](https://github.com/graphql/graphql-spec/pull/418)
+- It should throw a field error but not a null pointer runtime exceptional; we
+  need to prevent that from happening
+- This situation seems like a bug in the spec; we’re enabling clients to give us
+  what appears to be a valid request, when it could lead to problems
+- We last worked on this problem 4 years ago
+- Could introduce a new validation rule with backwards compatibility saying this
+  validation is a should not a must
+- Could be enabled/disabled via an option flag in graphql-js for example
+- Nervous about changing the semantics of what it means to have a default value;
+  lots of issues trying to get this right in the past
+- General preference to introduce things as SHOULD
+- Can we describe what we want here and work backwards?
+- Not opposed to adding an appendix about this in the spec
+- What is the text of the field error from the reproduction - added here:
+  [https://github.com/graphql/graphql-spec/issues/1002#issuecomment-1353581951](https://github.com/graphql/graphql-spec/issues/1002#issuecomment-1353581951)
+- It seems like this line is the one in question and needs the change, but this
+  will need to be confirmed:
+  [http://spec.graphql.org/draft/#sel-IALbLDFCFCAACEYoxc](http://spec.graphql.org/draft/#sel-IALbLDFCFCAACEYoxc)
+- Action: having default mode be the happy path which will require to spec text
+  changes
+
+## Add a style guide to the GraphQL specification (15m, Benjie)
+
+- [PR](https://github.com/graphql/graphql-spec/pull/1003)
+- Inconsistencies in styling
+- Especially around capitalization
+- Providing a style guide here could help
+- We most closely match the AP style guide
+- Propose we follow the rules there
+- Have started a basic new style guide and updated things to follow
+- Chicago style guide is great as well, but decided not go with it because of
+  prepositions / conjunctions no matter how long they are, whereas AP only
+  applies to preps / conj under a specific length
+- Love anything we can just lint rule away
+- AP style guide is nice and simple
+- How do we preserve this going forward; add a lint tool to apply this moving
+  forward?
+- Looks like we could use something like:
+  [https://vale.sh/explorer/ap/](https://vale.sh/explorer/ap/)
+- Next steps: All folks think this is a good idea; Benjie is going to run with
+  it!
+
+## Update from stream/defer breakout group and new proposal (20m, Rob and Benjie)
+
+- [Proposal](https://github.com/robrichard/defer-stream-wg/discussions/52#discussioncomment-4380643)
+- The above is the outcome of a separate breakout session earlier this week
+- This is the leading proposal so far to solve the issue
+- Details and examples are broken out in the discussion thread so not repeating
+  them here
+- Definitely an interesting proposal
+- Question around labels and reasons and how useful they will be
+- Reason shouldn’t be required, nice bit of extra information, but not necessary
+- Labels and path are necessary to uniquely identify what triggered things
+- We did consider getting rid of label entirely but decided to keep it because
+  we might want to defer things separately and not have them be merged
+- Modeling these as promises / async interables
+- Path to know where you’re merging data in, but need the id in case you’re
+  merging multiple things at that path
+- What do labels and reasons solve for clients? Do they help or hinder?
+- Are labels redundant if deferred fields can already be identified separately,
+  meaning they can be pulled back in on the client and merged effectively
+  without the label
+- Reason is just noise right now; but may be useful in future
+- If labels are optional what is the situation that requires a client developer
+  to add a label?
+- Labels can be seen as identifying deferred fragments as separate promises,
+  conceptually
+- Use case of a label to distinguish between logically different defers that
+  can’t be merged makes sense
+- Less clear as to the advantage of client developers having to worry about an
+  array of labels coming back
+- Also concerned about adding a bunch of extra stuff in deferred responses;
+  could reach a point where all of the extra response overhead negates the
+  benefits of defer
+- Not sure about merging of the labels; now there is some specific meaning
+  behind if you want things to be merged or not that developers need to worry
+  about (the client library can’t hide all of these decisions from end users)
+- Proposal is super interesting but we need to figure out the label part
+- Next steps: We have a follow up breakout meeting scheduled for Dec. 19th
