Don't serialize Option::None for Inputs #262
Conversation
* Specify most dependencies with major.minor version, no patch * New syn versions syn::Visibility doesn't dervive from debug anymore. * Use the Derivative crate to skip the "module_visibility" field for debug out. Signed-off-by: David Graeff <david.graeff@web.de>
structopt uses the first letter for a "short" option. "selected_operation" (with "o") as well as "output_directory" (with "out") claimed "-o" which resulted in: thread 'main' panicked at 'Argument short must be unique -o is already in use Fixed by removing the short option for "selected_operation" and keep "-o" for output directory. Signed-off-by: David Graeff <david.graeff@web.de>
If a camel case query name like "ping_query" is used, the generated code fails. Fixed by using `to_camel_case` for the struct name, while the module name still uses `to_snake_case`. Even for a single word query name like "ping" that should work. Signed-off-by: David Graeff <david.graeff@web.de>
…ak code generation Add integration test (keywords_query.graphql+keywords_schema.graphql) Signed-off-by: David Graeff <david.graeff@web.de>
The constructor method provides a simpler way to initialize a type with
many or only optional fields. Assuming an input type like this:
input CustomerInput {
company: String
customFields: [CustomFieldInput!]
email: String
firstName: String
lastName: String
phoneNumber: String
}
The generated code looks like so:
pub struct CustomerInput {
pub company: Option<String>,
...
}
impl CustomerInput {
pub fn new() -> Self {
Self { company: None, custom_fields: None, ... }
}
}
ALSO: More idomatic rust is generated by generating Option<Box<T>> instead of Box<Option<T>>.
Signed-off-by: David Graeff <david.graeff@web.de>
fixes
Signed-off-by: David Graeff <david.graeff@web.de>
|
Thanks for noticing that, it's indeed a problem! I have about 30 seconds to think about it now so I'll come back to this later, but I can think of cases where we would want the field to be omitted, and other cases where we would want it to be null. In graphql-ruby for example it's significant (server-side). Maybe |
|
This is also a problem in Juniper. Serde can not interpret this out of the box. Eg We'd need a enum with a custom enum Optional<T> {
None,
Null,
Some<T>,
}If the client does get support for this I hope we can coordinate and use the same type. |
|
An own enum type sounds like the correct solution. If "from" and "into" traits are implemented, it might not even break existing user code. |
|
Could you please rebase on |
|
This would actually solve the problem I was trying to articulate here: #364 |
|
I also have a similar issue since omitting a field and passing a null value are not the same. Is there a workaround in the meantime? |
|
Why was this just closed without any comment? @tomhoule |
|
It looks like it was targeting |
I have noticed that currently the library does not configure serde to skip Option::None and send json
nullvalues instead.I'm not sure if it violates the graphql spec (https://graphql.github.io/graphql-spec/draft/#sec-Type-System.Non-Null.Nullable-vs-Optional), but not sending those fields is definitely allowed:
My graphql server however is absolutely not happy about
nullvalues for optional fields.This PR adds the serde attribute to not serialize Option::None.
Requires all my other PRs for the sake of avoiding branch chaos on my side. If the last commit is fine, I'll be happy to extract and rebase it on master.