Skip to content

Commit 385280b

Browse files
benjiebenjie
authored
Add notes for September meeting (#1389)
1 parent 65e4a45 commit 385280b

File tree

1 file changed

+184
-0
lines changed

1 file changed

+184
-0
lines changed

‎notes/2023/2023-09.md

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
# GraphQL WG Notes - August 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
7+
8+
Agenda:
9+
[https://github.com/graphql/graphql-wg/blob/main/agendas/2023/09-Sep/07-wg-primary.md](https://github.com/graphql/graphql-wg/blob/main/agendas/2023/09-Sep/07-wg-primary.md)
10+
11+
### Determine volunteers for note taking (1m)
12+
13+
- Kewei
14+
- Young
15+
16+
### Action items:P
17+
18+
- Closing old action items soon! Please comment on them!
19+
- Calling volunteers to close pull request 1335
20+
21+
### Explicit BlockString definition
22+
23+
- Benjie: GraphQL
24+
[spec change](https://github.com/graphql/graphql-spec/pull/1042). We use
25+
BlockString in our grammar but not have it defined anywhere else. So this
26+
change adds the definition
27+
- General 👍
28+
29+
### Full Schemas
30+
31+
- [Slides](https://docs.google.com/presentation/d/1R8b4duEIi8iigAH4AU2zNNGlEC6f9CZYC9sVTgLjFHI/edit#slide=id.g27972193ce9_0_97)
32+
- Martin: a follow up on the last wg meeting
33+
- Apollo Kotlin
34+
- Apollo has started to encourage people to use SDL for tooling
35+
- When they have custom scalars, the tooling breaks because the spec forbids
36+
scalars from being transmitted in SDL
37+
- Full schema = Regular schema + Built-in definitions
38+
- Built-in definitions are
39+
- Builtin Scalars (Int, Float, etc.)
40+
- Directives
41+
- 3 questions
42+
- Do we want to introduce 2 concepts in the spec?
43+
- Martin: personally feels that this is too much. It’s more flexible for
44+
the implementation to introduce its own definitions
45+
- Kewei: How do we know how many bits “Int” scalar uses if we omit the
46+
definition?
47+
- Martin:
48+
- Is therea rule that all SDL definitions must be used?
49+
- Matt: What does it meant to be “used”?
50+
- Martin:
51+
- How far should there be 1:1 equivalence between introspection & SDL
52+
- Martin: PR is up to update the spec to define built-in definitions
53+
- Matt: Really likes the appendix. Relay has these defined but would like to use
54+
something from the schema
55+
- Benjie: same for the directives too
56+
- Martin: will remove the “must” language on the scalars from the spec
57+
- Matt: when you are handwriting a schema, you don’t want to handwrite these
58+
built-in definitions
59+
- Matt: It would be messy to get these definitions in the spec. There is a
60+
difference between the schema the tooling should consume and the handwritten
61+
schema. Should the schema always be compliant to the spec?
62+
- Martin: a newcomer will not know what these scalars are without them in the
63+
spec
64+
- Matt:
65+
- Martin: why did the spec forbid including these definitions?
66+
- Benjie: removing the definitions prevents custom definitions from conflicting
67+
with the spec’s definitions
68+
- Martin: why can we not provide the flexibility and fail early if the server
69+
doesn’t support?
70+
- Benjie: that introduces a lot of edge cases
71+
- — some gap
72+
- Matt: we may require a different type of SDL. the current spec specifies the
73+
resolver type schema
74+
75+
### Relay Error Handling
76+
77+
- Itamar:
78+
- Why?
79+
- Graphql responses with errors are just discarded and it’s not possible to
80+
distinguish null from nulls due to errors.
81+
- Relay error handling
82+
- Default behavior: field errors throw
83+
- Replaces null on field-level server error.
84+
- Opting into the old behavior with ‘@catch’ directive
85+
- @catch directive returns the error at the field level. With an optional
86+
parameter for the old-behavior
87+
- @catch does not change the graphql response, only Relay response
88+
- Interop
89+
- If @required throws, @catch will catch the exception
90+
- CCN
91+
- @required and ! will behave the same
92+
- ? will null bubble, @catch will not.
93+
- Progress
94+
- [Post in Relay repo outlining the project](https://github.com/facebook/relay/issues/4416)
95+
- [Post about larger “True Schema Nullability”](https://github.com/graphql/client-controlled-nullability-wg/issues/19)
96+
in Client Controlled Nullability
97+
- Kewei: what will the types of the fields be? Is it a union?
98+
- Matt: discriminated union or <some other option>
99+
- Alex: Will @catch be defined in the spec?
100+
- Jordan: This will not be. Relay client will handle this. Relay will parse
101+
the directive out before the request goes to the server
102+
- Anthony: Does this unblock the concerns around CCN’s ‘!’ and ‘?’?
103+
- Jordan: It is aligned with CCN. Not sure if the server needs to change yet.
104+
Would like to see other smart clients adopt this first and then see if the
105+
server needs to change.
106+
- Matt: CCN provides a way for the server to express its true nullability and
107+
the smart clients can override it with CCN designators.
108+
- Martin: Can you share the slides?
109+
- Itamar: will do
110+
- Martin: how does it impact fragments merging?
111+
- Jordan: Because the behavior is all in the client side, different fragments
112+
will see different behaviors unlike CCN impacting fragments across.
113+
- Ernie: will the errors be surfaced in the field?
114+
- Matt: the same metadata on errors will be made available in the fields
115+
- Jordan: certain errors will be made opaque from the product engineers
116+
- Robert: what happens when multiple errors happen inside the selection set?
117+
- Itamar: if a parent sees multiple exceptions, all errors will be provided
118+
with their paths
119+
- Robert: does Relay want to support the schemas where a field’s type is a
120+
union type (result type + error type)
121+
- Matt: That is about generics in GraphQL which is a larger conversation
122+
- Itamar: For this proposal, uniform error type would be the goal
123+
- Robert: what should be the schema if other clients want to support the same
124+
behavior?
125+
- Itamar: This is a client side feature, so don’t plan on proposing schema
126+
changes.
127+
128+
### Changing ExecuteSelectionSet to ExecuteGroupedFieldSet
129+
130+
- Benjie:
131+
- When working on the incremental delivery, found the refactoring on the spec
132+
desirable
133+
- Please review https://github.com/graphql/graphql-spec/pull/1039
134+
135+
### Client Controlled Nullability updates
136+
137+
- Young: stripping down the proposal to have just the “!”. Semantics of “?” is
138+
very unclear and leads to ambiguity. The “!” semantic will just change the
139+
field nullability from null to nonnull.
140+
- Would like to go to stage 3, and seek feedback on whether there is concern
141+
with stripping the “?”.
142+
- Curious what Relay and others think of not having the “?”
143+
- Jordan: interested in “?” in the long term. Gives us flexibility in the long
144+
term, without the client developers needing to assert nonnull blindly. If the
145+
client can shield users from dealing with nulls from errors. Would be okay if
146+
this needs to be done as a second phase.
147+
- Matt: The outcome from the error handling workshop, the end state of client
148+
side error handling should mirror exactly the server behavior.
149+
- Total ideal for error handling to have the “!” that will transform the error
150+
to error.
151+
- Give us a migration path to truly understand the server nullability.
152+
- Jordan: any client using a consistency store is damaged by null bubbling. So
153+
“?” would gives us the ability to opt out of the null bubbling.
154+
- Anthony: remove “?” from CCN seems to be resolved by the @catch directive? Do
155+
we still want to pursue the “!” -only proposal. Or do we want to continue with
156+
both operators?
157+
- Young: relay’s concern is mostly resolved. But still concerned about potential
158+
6-month delay about re-opening conversations about discussing 2-3 options.
159+
- Alex: “!” is popular and easy to justify and easy to merge. Then we can figure
160+
out the stuff that’s less popular
161+
- Matt: “?” might need a different champion.
162+
- Martin: The community has limited attention span as well.
163+
- Jordan: “!” as a primitive makes sense. The community moves slowly so we need
164+
“!” in the meantime. Not oppose to it existing as a concept.
165+
- Ernie: People wouldn’t be able to control one without the other. So if we only
166+
introduce “!” we might limit the adoption because people cannot control it
167+
without “?”. Also a risk for having a new person champion the “?”, whether
168+
it’s going to be done. You need to have both in conjunction. Can’t have one
169+
without the other.
170+
- Benjie: in favor of simple “!”. But, when you have this null bubbling, you
171+
can’t do caching anymore because it’s not safe. We should spend some time
172+
thinking about how to not break normalized caches.
173+
- Robert: if you want to play with it, you can add a magical field “\_\_self”
174+
that refers to itself. We just need to be clear on the messaging that “?” is
175+
coming later.
176+
- Young: for a federated server to react to “?” correctly, it needs to resolve
177+
its dependency. It needs more time to figure it out.
178+
- Jordan: additional damage done by the “!”, Relay’s perspective is to implement
179+
it entirely on the client side, treat it similar to @required with fragment
180+
isolation.
181+
- Martin: if the user start copy pasting to GraphiQL, then they won’t get the
182+
same result. But @require is a directive, but “!” is on the language.
183+
- Jordan: It’s not practical to copy pasting with Relay already. We need to
184+
actually figure out the real query.

0 commit comments

Comments
 (0)