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