-
Notifications
You must be signed in to change notification settings - Fork 726
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
Add better default initializers for GraphQLNullable<Wrapped> #2729
Comments
Yes please. Current implementation is a disaster for upgrading to 1.X. We do have a beautiful Swift API around everything, with you know, Swift optionals. Why would we need Please just use Swift optional and delete Thanks. |
Totally agree the current API might have been a good idea on paper, but in a sizeable project where enums and optionals are frequently nested this is painful to use. |
|
There is a very clear distinction between a |
Before v1.0, we were able to initialize any generated model directly using the default initializer (e.g., from a struct). How can we generate setters for every generated model? |
@JoeFerrucci, you can track the release/1.1 branch where work is being done to enable selection set initializers. The work is in progress at the moment so that branch may not build. |
To those who it may help, I've created some extension properties to make working with |
I feel like a lot of the 1.0 release has been quite cumbersome to work with like this. For example the GraphQLEnum type is just as awkward to use as the GraphQLNullable. Also the removal of initializers from fragments makes mocking and testing overly complicated (I realize this is now fixed in 1.1.0, but ive had no luck getting the new inits to generate). We have been at it for about a week now trying to migrate from 0.53.0 and it's been extremely painful so far (about 500 compile errors to work through for us). That being said, here is our extension on GraphQLNullable to make it a bit easier to use with optional types. It might help you. Note this assumes that you want to explicitly encode null if you pass in an optional value and it happens to be nil: import ApolloAPI
extension GraphQLNullable {
init(optionalValue value: Wrapped?) {
if let value {
self = .some(value)
} else {
self = .null
}
}
} |
@BrentMifsud, you might like this thread: #2883 |
@JoeFerrucci unfortunately the new config option for generating initializers isn't working for me. There doesn't seem to be any difference in what gets generated regardless if I add that config option or not 🤷🏻♂️ Im currently on apollo 1.1.1 |
lets see your config. |
got it figured out. I needed to move that new config property under |
Thought i'd throw in my take aswell as it allows for imo better readability.
Current example above yields: My take imo allows for a slight readability improvement. Query(param: .optional(someOptionalValue)) |
In practice, I definitely haven't been seeing as much value in the If Swift had the ability to extend Enum literals like we could with This is because in existing apps, we were able to use the raw enum as a plain enum everywhere, with a "natural" default of |
This issue has been discussed exhaustively. We understand the cumbersome nature of |
Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo iOS usage and allow us to serve you better. |
Use case
I recently completed the porting of our model and network layers from v0.x to v1.0. My general feedback is that this was a lot more painful than it needed to be.
Handling nullable types is now essentially broken by design. Even if you wanted to add the concept of
.none
versus.null
(which is in itself confusing and poorly documented), not supporting nullable values out of the box (such asString?
) will force every client of your SDK to either hack something together, write unnecessarily verbose code, potentially pick the wrong option between.none
and.nil
, or all of the above. It was also exceedingly difficult to get array types to workAnother big pain point was forcing every query parameter to specify a value, likely for the same reason. It seems like this should have been configurable from the code generator, where you could either pick one or the other as the default and handle the potentially few exceptions rather than require lots of boilerplate code for every query.
Describe the solution you'd like
In case it helps others, I was able to at least simplify some of our code by writing an extension like this one:
The text was updated successfully, but these errors were encountered: