Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Allow to omit non-nullable variables if they have a defaultValue #2686

Closed
renclav opened this issue Oct 22, 2020 · 7 comments
Closed

Allow to omit non-nullable variables if they have a defaultValue #2686

renclav opened this issue Oct 22, 2020 · 7 comments

Comments

@renclav
Copy link

renclav commented Oct 22, 2020

Is your feature request related to a problem? Please describe.
I would like to define default values for my query variables inside the query definitions defined in .graphql file/s. Currently this is possible, but the default values are ignored in the generated models.

Describe the solution you'd like
The client library follows the spec defined here which would result in kotlin query models being generated which include default values.

Notes:

  1. I use kotlin models and as such, have no awareness of how this may effect java based models.
  2. I assume this would be desired for subscriptions and mutations as well (no experience there unfortunately ).
@martinbonnin martinbonnin self-assigned this Oct 27, 2020
martinbonnin added a commit that referenced this issue Oct 27, 2020
This only works for scalar types and their list/optional variants, not
input object types

closes #2703 and part of #2686
martinbonnin added a commit that referenced this issue Oct 28, 2020
This only works for scalar types and their list/optional variants, not
input object types

closes #2703 and part of #2686
@martinbonnin martinbonnin changed the title Add the ability generate default Query variables Support Input Objects in variables default values Oct 28, 2020
@martinbonnin
Copy link
Contributor

martinbonnin commented Oct 28, 2020

@renclav Most of use cases should work with #2704. This has been merged and will be available in the 2.4.2-SNAPSHOT in ~30 min (ci build). Any chance you can try the snapshots when they're ready? If everything looks good, I'll make a release next Monday.

In all cases, I'm keeping this issue open (with an updated title) for input object default values, which will require more work.

@renclav
Copy link
Author

renclav commented Nov 2, 2020

@martinbonnin 2.4.2-SNAPSHOT works well for me 🥳

@martinbonnin
Copy link
Contributor

Awesome :). Glad to hear that and perfect timing as I'm preparing the 2.4.2 release. It should be live in a few hours

@martinbonnin
Copy link
Contributor

@renclav after some tests, passing the default value as default argument in the model constructor might not be the best way to do it since it will send a GraphQL variable by default, which might be counter intuitive.

Can you elaborate on why you need the default value in the Kotlin models? Passing Input.absent() as was the case before 2.4.2 should still send the default value to your server as part of the GraphQL query. Or did you need the defaultValue somewhere else?

@martinbonnin
Copy link
Contributor

I'm going to close this one as not overriding the GraphQL document default variables seems to be the way to go, even for input objects (see #2741). Please leave a message if you want me to reopen it.

@martinbonnin
Copy link
Contributor

martinbonnin commented Nov 17, 2020

Reopening this issues as there is more to it. While the solution of passing Input.absent() works for variables of nullable types, it doesn't work for variables of non-null type.

query launches($id: String! = "83") {
  launch(id: $id) {site}
}

will generate

// no way to omit `id`
data class LaunchesQuery(val id: String) { ... }

This means there's currently no way to omit the variable and let the GraphQL query document carry the defaultValue. A potential fix is to make variables with default values optional (in the Kotlin acceptation) as the current optional values (in the GraphQL acceptation)

// by default, id will be omitted 
data class LaunchesQuery(val id: Input<String> = Input.absent()) { ... }

@martinbonnin martinbonnin changed the title Support Input Objects in variables default values Allow to omit non-nullable variables if they have a defaultValue Nov 17, 2020
@martinbonnin
Copy link
Contributor

This should now work as expected in 3.0.0-alpha01. The corresponding tests are at https://github.com/apollographql/apollo-android/pull/3247/files

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants