Skip to content

Commit 7e58000

Browse files
benjiebenjie
authored
Add notes for Feb WG (#1262)
1 parent a59a135 commit 7e58000

File tree

1 file changed

+194
-0
lines changed

1 file changed

+194
-0
lines changed

‎notes/2023/2023-02.md

Lines changed: 194 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,194 @@
1+
# GraphQL WG Notes - Feb 2023
2+
3+
**Watch the replays:**
4+
[GraphQL Working Group Meetings on YouTube](https://www.youtube.com/playlist?list=PLP1igyLx8foH30_sDnEZnxV_8pYW3SDtb)
5+
6+
# Primary meeting
7+
8+
## Agenda
9+
10+
- Agree to Membership Agreement, Participation & Contribution Guidelines and
11+
Code of Conduct (1m, Lee)
12+
- [Specification Membership Agreement](https://github.com/graphql/foundation)
13+
- [Participation Guidelines](https://github.com/graphql/graphql-wg#participation-guidelines)
14+
- [Contribution Guide](https://github.com/graphql/graphql-spec/blob/main/CONTRIBUTING.md)
15+
- [Code of Conduct](https://github.com/graphql/foundation/blob/master/CODE-OF-CONDUCT.md)
16+
- Introduction of attendees (5m, Lee)
17+
- Determine volunteers for note taking (1m, Lee)
18+
- Review agenda (2m, Lee)
19+
- Review prior secondary meetings (5m, Lee)
20+
- [WG secondary APAC](https://github.com/graphql/graphql-wg/blob/main/agendas/2023/01-Jan/11-wg-secondary-apac.md)
21+
- [WG secondary EU](https://github.com/graphql/graphql-wg/blob/main/agendas/2023/01-Jan/19-wg-secondary-eu.md)
22+
- Review previous meeting's action items (5m, Lee)
23+
- [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)
24+
- [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)
25+
- [All open action items (by meeting)](https://github.com/graphql/graphql-wg/projects?query=is%3Aopen+sort%3Aname-asc)
26+
- TSC: Election Results (5m, TSC)
27+
- TSC: Management of build and publish infrastructure, secrets, etc (20m, Benjie
28+
/ Rikki)
29+
- This meeting is being recorded, so be mindful of the audience - we can take
30+
some of these communications private if need be
31+
- GraphiQL's last publish was partially unsuccessful due to token issues:
32+
[graphql/graphiql#2997](https://github.com/graphql/graphiql/issues/2997)
33+
- GraphQL Foundation should have a more robust process for handling planned or
34+
emergency token revokation for individuals
35+
- [Rikki has proposed a solution](https://gist.github.com/benjie/739e119b68422a14864486a9ff0b2a8c)
36+
- Fix ambiguity around when schema definition may be omitted (10m, Benjie)
37+
- [RFC](https://github.com/graphql/graphql-spec/pull/987); previously
38+
discussed in September
39+
- Aim: merge this (mostly) editorial change
40+
- Advance argument name uniqueness to Stage 3 (10m, Benjie)
41+
- [RFC](https://github.com/graphql/graphql-spec/pull/891)
42+
- [GraphQL.js implementation is merged](https://github.com/graphql/graphql-js/pull/3208)
43+
- @defer: fully deduplicate response (30m, Ivan)
44+
- [RFC]
45+
([robrichard/defer-stream-wg#65 (comment)](https://github.com/robrichard/defer-stream-wg/discussions/65#discussioncomment-4757697))
46+
- GraphQL.js implementation: TBD
47+
- Default Value Validation Status Check (5m, Yaacov)
48+
- [Lee's original implementation PR stack at graphql-js](https://github.com/graphql/graphql-js/pull/3049)
49+
- [Now rebased on main](https://github.com/graphql/graphql-js/pull/3814)
50+
- [Benjie's Spec PR](https://github.com/graphql/graphql-spec/pull/793)
51+
52+
## Determine volunteers for note taking (1m, Lee)
53+
54+
- Benjie
55+
- Hugh
56+
57+
## Review agenda (2m, Lee)
58+
59+
- [no comments]
60+
61+
## Review prior secondary meetings (5m, Lee)
62+
63+
- [WG secondary APAC](https://github.com/graphql/graphql-wg/blob/main/agendas/2023/01-Jan/11-wg-secondary-apac.md)
64+
- [WG secondary EU](https://github.com/graphql/graphql-wg/blob/main/agendas/2023/01-Jan/19-wg-secondary-eu.md)
65+
- Main advancement: Scalars website is now live.
66+
- Secondary advancements: the increased cadence has helped increase the velocity
67+
of stream/defer and fragment arguments discussions.
68+
- Review of fragment arguments would be useful now, so please have a read!
69+
70+
## Review previous meeting's action items (5m, Lee)
71+
72+
- [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)
73+
- [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)
74+
- [All open action items (by meeting)](https://github.com/graphql/graphql-wg/projects?query=is%3Aopen+sort%3Aname-asc)
75+
- [Skipped]
76+
77+
## TSC: Election Results (5m, TSC)
78+
79+
- Successful election with 10 nominees and only 1 person who put themselves up
80+
for reelection.
81+
- Congrats to the new TSC members, and to Andi on re-election!
82+
- Thanks to everyone who nominated themselves.
83+
- Best thing about TSC is we mostly defer to the WG, so everyone can take a
84+
leadership role.
85+
86+
## TSC: Management of build and publish infrastructure, secrets, etc (20m, Benjie / Rikki)
87+
88+
- This meeting is being recorded, so be mindful of the audience - we can take
89+
some of these communications private if need be
90+
- GraphiQL's last publish was partially unsuccessful due to token issues:
91+
[graphql/graphiql#2997](https://github.com/graphql/graphiql/issues/2997)
92+
- GraphQL Foundation should have a more robust process for handling planned or
93+
emergency token revokation for individuals
94+
- [Rikki has proposed a solution](https://gist.github.com/benjie/739e119b68422a14864486a9ff0b2a8c)
95+
- Thomas is stepping in for Rikki
96+
- Matt: Ideally two or three owners. TSC members? Require 3 of these users to do
97+
a minor version bump and release (even a NOOP), just to test. If we don't
98+
exercise it, there's no guarantee it will work.
99+
- Benjie: this partly relates to the auto-deployment from CI, which has security
100+
implications around tokens and two-factor authentication tokens.
101+
- Lee: GitHub Actions auto-deploy is the best balance between security and
102+
convenience. Local project maintainers just need to do the mechanics of a
103+
version bump, they don't have to worry about tokens/MFA/etc.
104+
- Benjie: what's the next steps? GraphiQL is currently broken.
105+
- Lee: there's the right way, which I'll take ownership of. Is there something
106+
we can do today to unstick GraphiQL?
107+
- Thomas: short term would be to have a token with all the necessary
108+
permissions, even a personal token.
109+
- Benjie: I believe the issue is that even TSC members don't have access to the
110+
`@graphiql/*` package
111+
- **ACTION**: @leeb - address this.
112+
113+
## Fix ambiguity around when schema definition may be omitted (10m, Benjie)
114+
115+
- [RFC](https://github.com/graphql/graphql-spec/pull/987); previously discussed
116+
in September
117+
- Aim: merge this (mostly) editorial change
118+
- Lee: this is a change in behavior but it does seem to align with the intent.
119+
- Lee: lets check this with the implementations to make sure there's no issues
120+
there. It may be that when printing the schema out there is an issue where the
121+
old behavior was followed.
122+
- Benjie: I think the code block in the top comment can be used as a test case -
123+
read the code in, generate the schema from it, and print it out - it should be
124+
the same (i.e. no `mutation` operation)
125+
- **ACTION** - Benjie - tag implementors and check that this won't cause issues.
126+
127+
## Advance argument name uniqueness to Stage 3 (10m, Benjie)
128+
129+
- [RFC](https://github.com/graphql/graphql-spec/pull/891)
130+
- [GraphQL.js implementation is merged](https://github.com/graphql/graphql-js/pull/3208)
131+
- Issue still remains; generally agreed this needs to be addressed
132+
- Champion hasn’t been at the WG’s, so what are our next steps here; what is the
133+
process for moving this forward
134+
- Someone should claim champion (Ivan did on the code side; PR was merged a
135+
while back)
136+
- Spec needs to be updated to agree with the reference implementation now
137+
- Looks straightforward - folks agree it’s ready to advance
138+
- It’s ready to merge … merged, YAY!
139+
140+
## @defer: fully deduplicate response (30m, Ivan)
141+
142+
- [RFC]([robrichard/defer-stream-wg#65 (comment)](https://github.com/robrichard/defer-stream-wg/discussions/65#discussioncomment-4757697))
143+
- GraphQL.js implementation: TBD
144+
- Agreed at the previous working group that deferred fields that also exist in
145+
the parent (non-deferred) selection set should be omitted since they're
146+
already satisfied.
147+
- Then we agreed that sibling `@defer`s should also merge, so
148+
`... @defer { a b } … @defer { a c }` should become `... @defer { a b c }`
149+
- Rob has come up with some challenging examples to see what would happen.
150+
- For some examples we can solve them statically and they don't cause any
151+
issues.
152+
- A defer directly inside a defer (`... @defer { … @defer { ??? } }`) is a bit
153+
controversial, and it's hard to extract the intent that the user had when
154+
writing the query
155+
- The incremental delivery WG generally agree that all the defers at the same
156+
"path" (independent of selection sets) should be merged together, so
157+
`... @defer {a {b} … @defer {a { c } } }` should be equivalent to
158+
`...@defer { a { b c } }`.
159+
- For more complex examples, we cannot determine statically how to fully
160+
deduplicate because e.g. two defers can come in either order. One option is to
161+
deduplicate at runtime and handle it at the transport layer.
162+
- Matt: we shouldn't have incremental payloads that don't satisfy at least one
163+
@defer - i.e. a defer should not be split over multiple payloads.
164+
- Matt [chat]: I feel weakly that there should be _at most_ one incremental
165+
payload at the same path, and if there’s response duplication or _fewer_
166+
payloads than defers, that’s preferred.
167+
- Matt [chat]: I think product developers are expecting _at least_ a complete
168+
set of data for the path they’re asking for and can’t actually _use_ an
169+
incremental payload that isn’t yet completed, at which point there’s no use
170+
to splitting the payloads beyond once per path
171+
- Ivan's slides:
172+
[https://docs.google.com/presentation/d/1hlq7sLCyxm4DQbi-7rciRFRZePmdrbgXD4dB5NL_DaM/edit?usp=sharing](https://docs.google.com/presentation/d/1hlq7sLCyxm4DQbi-7rciRFRZePmdrbgXD4dB5NL_DaM/edit?usp=sharing)
173+
174+
## Default Value Validation Status Check (5m, Yaacov)
175+
176+
- [Lee's original implementation PR stack at graphql-js](https://github.com/graphql/graphql-js/pull/3049)
177+
- [Now rebased on main](https://github.com/graphql/graphql-js/pull/3814)
178+
- [Benjie's Spec PR](https://github.com/graphql/graphql-spec/pull/793)
179+
- Yaacov: call to action: review the implementation PR and how it aligns with
180+
the spec PR.
181+
- **ACTION** - everyone: review these PRs!
182+
- Matt was asking about what would happen if a scalar input value could be
183+
serialized to undefined. Does this lead to surprising behavior? Okay to just
184+
throw an error? Hopefully not a breaking change.
185+
- Ivan: not a breaking change to the spec. If necessary we can make a breaking
186+
change to GraphQL.js in a major version. I'd release this as a major release
187+
of GraphQL.js anyway. GraphQL.js aims to be replicable in other languages
188+
(e.g. GraphQL python is a direct copy) and most languages don't have a
189+
difference between null and undefined.
190+
- Benjie: every other language that implemented the old behavior should also
191+
release this under a major version bump because it concretely changes behavior
192+
of edge cases.
193+
- Lee: we should be clear about this because the difference is subtle.
194+
- **ACTION** - Matt / Yaacov - tag Lee at the relevant line of code to review.

0 commit comments

Comments
 (0)