Optional arguments still wraped in Some #48
Comments
|
Probably have a similar problem with String vs &str... |
|
Perhaps one of the answers to this is making the query_dsl smarter - take Into<Cow> or similar for Strings, wrap Options in into etc. Rather than trying to come up with some generic solution for all types. Not sure what the approach for custom scalars would be, but at least there’s a trait to work with there - suspect references might just work if I added the right impl. |
|
Some work for this on the into-option branch. |
|
Interesting side effect of supporting |
|
Pushed a first pass of this to 9d8f995 - I need to think further on how to implement it though. Suspect there might be some subtlety (or at the very least bits that can be improved) when it comes to integrating this with the custom scalar/ID work I have in mind (#51) Also wondering if this could help with #46 - though not quite fully formed my thoughts there. |
#### Why do we need this change? Currently, the `cynic_arguments` attribute is pretty strict in what it accepts. - If an argument is `String!` you need to explicitly provide a `String` (a `&str` isn't good enough). - If an argument is `String` you need to explicitly provide an `Option<String>` - none of `String`, '&str`, or `Option<&str>` is acceptable. This leads to some noisy `cynic_argument` attributes. This also causes otherwise backwards compatible GraphQL schema changes to cause compilation errors - if a field was required before and is now optional, you'll have to explicitly add that `Some` to your definition. #### What effects does this change have? This PR adds an `IntoArguments<T>` trait where `T` is the type of the argument. This trait handles converting various arguments into their expected form. I considered using some of the built in traits like `AsRef` or `Into` but they all seemed to come with limitations for what I wanted to do. Fixes #48 #### Note on Lifetimes I spent some time trying to solve #46 at the same time - at one point `IntoArgument` had a `Output` associated type that I could use to control the type being serialized. I used this to borrow `&str` and feed it into a new lifetime on `Argument`. This new lifetime ended up bleeding up to the `Query` struct which made things a bit unergonomic. See note on #46 for more details on that. At the time of writing this alternative implementation can be found on the `into-arguments` branch, though it may not stay around forever. This PR was a partial rebase of that original branch, with some re-work afterwards.
#44 updated the query_dsl to use the builder pattern for specifying arguments. Might be nice to look at unwrapping the Option from optional arguments so that we don't have to do
an_option(Some(xyz))everywhere.Though we probably do want to support people providing
Noneso maybe need to useInto<Option<T>>?The text was updated successfully, but these errors were encountered: