|
89 | 89 | ## RFC: Default value coercion (20m, Benjie)
|
90 | 90 |
|
91 | 91 | - Benjie: I was looking through the spec….there’s some interesting things that go with default value. [Explains the PR]. This feels like a bug in the spec.
|
92 |
| -- Lee: I agree this doesn’t seem quite right. There’s a reason why default values don’t go through cooresion: It introduces the ability to create this pattern. 1: wanted to make sure it’s statically nullable (you don’t have to execute anything to know). 2: you want to be able to read the query. If you see default values and it looks like an an empty object, you might be confused when you see it actually has a number in it cos it had a default. There’s a twist - either we have to repeat ourselves, or do something like you proposed. You mentioned you could pass in number 7 and that’s what you get. But you can’t because there’s a validation rule, and 7 != 3. Empty object does pass validation because the number field is empty. It’s a bug, because it’s empty but deemed as a valid type, so it’s missing a step. |
| 92 | +- Lee: I agree this doesn’t seem quite right. There’s a reason why default values don’t go through coercion: It introduces the ability to create this pattern. 1: wanted to make sure it’s statically nullable (you don’t have to execute anything to know). 2: you want to be able to read the query. If you see default values and it looks like an an empty object, you might be confused when you see it actually has a number in it cos it had a default. There’s a twist - either we have to repeat ourselves, or do something like you proposed. You mentioned you could pass in number 7 and that’s what you get. But you can’t because there’s a validation rule, and 7 != 3. Empty object does pass validation because the number field is empty. It’s a bug, because it’s empty but deemed as a valid type, so it’s missing a step. |
93 | 93 | - Maybe all values should require manual compression at runtime. Downside is default changes, forgets to update types….maintenance risk. Idk if it’s better, but wanted to toss out alternative solutions
|
94 | 94 | - Benjie: What you said about seeing what the default would be from a user point of view....i’d like to challenge that! I don’t think it would be confusing. Effectively saying if you don’t specify this, same as
|
95 | 95 | - What happens if I specify it but only give one of the properties?
|
|
105 | 105 | - Matt (chat): This also brings up a devex issue: there’s no way for a client to have a variable for an argument, where that variable results in “use the server default value”.
|
106 | 106 | - The only way to implement that is to allow the argument to be null, then have the server decide at execution time that null is the same as some default value.
|
107 | 107 | - Matt: We almost never use schema level default values. Actual code on the server decides what the default is if you pass in a null. Default values assume server tells you what default will be.
|
108 |
| -- Lee: Two algorithms for coercing. Issue only changs coercing of variables, but we should double check there aren’t differences across both algorithms. |
| 108 | +- Lee: Two algorithms for coercing. Issue only changes coercing of variables, but we should double check there aren’t differences across both algorithms. |
109 | 109 | - Benjie: Covers both argument values and variable values. Argument values is really the one that needs fixing.
|
110 | 110 | - Lee: You have change in both places. This is probably the right path forward. We should fix this quickly. As matt says, breaks the contract between client and server. Also potential security / stability issue, if you can crash a server that doesn’t handle NPEs.
|
111 | 111 | - Benjie: @ivan this would be a bigger change to graphql spec than this would be.
|
112 | 112 | - Andras: What does introspection return for this?
|
113 |
| -- Benjie: empty object. This isn’t really a valid schema. Default value doesn’t get cooreced. Up to the schema writer to make sure it’s the case. |
| 113 | +- Benjie: empty object. This isn’t really a valid schema. Default value doesn’t get coerced. Up to the schema writer to make sure it’s the case. |
114 | 114 | - Lee: Introspection is doing the right thing here.
|
115 | 115 | - Lee: Marking as stage 1, seems directionally good.
|
116 | 116 | - Ivan: [https://github.com/graphql/graphql-spec/pull/701](https://github.com/graphql/graphql-spec/pull/701)
|
|
0 commit comments