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

Generate a Kotlin enum without the UNKNOWN__ case #6243

Open
joshuakcockrell opened this issue Nov 6, 2024 · 8 comments · Fixed by #6248
Open

Generate a Kotlin enum without the UNKNOWN__ case #6243

joshuakcockrell opened this issue Nov 6, 2024 · 8 comments · Fixed by #6248
Labels
⌛ Waiting for info More information is required ✨ Type: Feature
Milestone

Comments

@joshuakcockrell
Copy link

joshuakcockrell commented Nov 6, 2024

Use case

Currently, when statements need to match .UNKNOWN__ every time they're used in a codebase.

when(color) {
  BLUEBERRY -> TODO()
  CANDY -> TODO()
  __UNKNOWN -> TODO()
} 

This means I need to think through the UNKNOWN fallback several times.

If the Apollo codegen provided an enum that didn't contain this case, I could instead write

when(color.unwrap()) {
  BLUEBERRY -> TODO()
  CANDY -> TODO()
} 

Yes, nothing's currently stopping me from writing an unwrap() extension, but I still need to sprinkle __UNKNOWN throughout my codebase to get the when statements to compile.

This is how Apollo iOS works, btw. GraphQLEnum.unknown is instead a wrapper enum around the core enum. On first glance, you would think Kotlin's approach is nicer because you don't need to match .case(...), but every single when statement needs a default case to handle the .UNKNOWN__, whereas iOS I can just create an unwrapper at the top level of my project to handle .unknown once.
Screenshot 2024-11-06 at 2 51 19 PM

Describe the solution you'd like

Screenshot_2024-11-06_at_2 29 15_PM

Apollo Kotlin already generates knownEntries List. If it just provided this exact thing in enum format I could write my own project level unwrap() and my individual when statements would never need to worry about .UNKNOWN__.

More context

Discord thread

@joshuakcockrell
Copy link
Author

joshuakcockrell commented Nov 6, 2024

For the record, after using both iOS & Kotlin Apollo libraries, I do like Apollo Kotlin's enum approach better as a starting point. Seeing Getting Started examples that have .case(..) is kinda gross on iOS. But I'd say Apollo iOS wins for the power use cases once you figure out how to write your own .unwrap() extensions.

Ideally, Apollo Kotlin could just codegen a knownEntries enum and that's the best of both worlds.

@martinbonnin
Copy link
Contributor

@joshuakcockrell an implementation is available at #6248. Let us know what you think

Copy link
Contributor

github-actions bot commented Nov 8, 2024

Do you have any feedback for the maintainers? Please tell us by taking a one-minute survey. Your responses will help us understand Apollo Kotlin usage and allow us to serve you better.

@BoD BoD added this to the 4.1.1 milestone Nov 8, 2024
@joshuakcockrell
Copy link
Author

Wow, that's a crazy fast turnaround on this! I'm excited to test this out. Will let you know how it goes.

@martinbonnin
Copy link
Contributor

Damn, I didn't mean to close this issue so fast, forgot that merging the PR would close the issue 🤦
In all cases, let us know how it goes, it's available in the SNAPSHOTs now.

@martinbonnin martinbonnin reopened this Nov 11, 2024
@martinbonnin
Copy link
Contributor

@joshuakcockrell did you get a chance to test this? Just want to make sure it's working for you while while it's still hot.

@joshuakcockrell
Copy link
Author

@martinbonnin I've been having difficulty getting the snapshot working. I got 4.1.1-SNAPSHOT working but couldn't see any difference in the generated code. I'll try again today.

@martinbonnin
Copy link
Contributor

Hi @joshuakcockrell any update here? Would be nice to get confirmation it fits your use case before we cut a release.

@BoD BoD added the ⌛ Waiting for info More information is required label Dec 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
⌛ Waiting for info More information is required ✨ Type: Feature
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants