feat: added CaseIterable to EnumType.

[TizianoCoroneo]

In the previous incarnation of the codegen enums implemented CaseIterable. We have some code that relies on this behavior.

This PR adds back CaseIterable to the EnumType protocol.

Update:

I found out (the hard way) that the compiler will not automatically synthesize the allCases requirement of CaseIterable if the enum contains cases with deprecation annotations. I modified the EnumTemplate to generate an allCases computed variable in this case; other enums rely on the automatic synthesis by the compiler.

[netlify]

👷 Deploy request for apollo-ios-docs pending review.

Visit the deploys page to approve it

Name	Link
🔨 Latest commit	853b23d

[AnthonyMDev]

Thanks for the PR!

I found out (the hard way) that the compiler will not automatically synthesize the allCases requirement of CaseIterable if the enum contains cases with deprecation annotations. I modified the EnumTemplate to generate an allCases computed variable in this case; other enums rely on the automatic synthesis by the compiler.

We also found this out the hard way. And tried solving it the exact same way as you! The issue we had with that solution is, if the allCases includes the deprecated cases, then the generated files emit warnings for accessing them. We don't want to give people warnings in their projects from the generated code there...

It's also unclear if users are going to want the deprecated cases included in allCases. I think in most cases they would, but that emits warnings. And if they don't that is also a viable use case. So we decided to just remove CaseIterable and leave that up to users to add themselves via an extension.

I'm not super happy with that. Very willing to consider other options here. We're looking for feedback on this still. Would love your opinion.

[TizianoCoroneo]

I'm not super happy with that. Very willing to consider other options here. We're looking for feedback on this still. Would love your opinion.

It seems the best solution is adding one more options to the codegen:

deprecatedCasesInCaseIterable with options include or exclude.

This option should only be in effect if both deprecated cases and deprecation warnings are included.

In my case I'd prefer to deal with the warnings than not having the deprecated cases, but I understand that this is highly dependent on the specific situation.

If you approve of this, I can add the option tomorrow (CET-wise).

As a side note, very nice work on the codegen 👍. It seems a bit much in some points, but I'm sure you had your reasons. Blog post when? 😄

[calvincestari]

It seems the best solution is adding one more options to the codegen:

deprecatedCasesInCaseIterable with options include or exclude.

This option should only be in effect if both deprecated cases and deprecation warnings are included.

This is what we considered but I'm not a big fan of options that only work in some cases or conflict with others in non-obvious ways.

In my case I'd prefer to deal with the warnings than not having the deprecated cases, but I understand that this is highly dependent on the specific situation.

This comes down to preference, but I'm happy to learn more what the wider community finds valuable. CaseIterable conformance can always be done in an extension but you lose auto-synthesis, which I understand is annoying.

Blog post when? 😄

Soon™ - lol. We've got a long list of documentation and articles still to publish.

[AnthonyMDev]

@calvincestari and I spoke about this in depth. After reflecting on it, we feel that having deprecation warnings on enum cases doesn't make sense.

For fields and input parameters that are deprecated, you are choosing to fetch them in your operations client-side. Deprecations are actionable -- you should remove the field from your operation and implement usage of the alternative for the deprecation (or whatever other appropriate steps there are to take).

For enum cases, just because the case has been deprecated does not mean you should necessarily stop supporting it. Your server may still send you deprecated enum types in a response for objects created in the past. You still need to handle the cases where that deprecated value is in a response object. The only place that the deprecation is actionable on the client is when used as an input value for a parameter of that enum type. Most of the actionable steps here are server-side.

We are going to move the warning from being an @available warning emitted in your Swift project to just being added as a comment in addition to the documentation on enum cases. @available will still be used for deprecated fields and input parameters.

In that case, we can take action on this PR, and will also make the change to the generated warnings for enum case deprecations. I'll get that resolved ASAP and merge this PR.

Thanks @TizianoCoroneo.

[AnthonyMDev]

Closing this in favor of #2579 which includes the fix for this!

[TizianoCoroneo]

Thank you for the quick response! I agree with all of the above 👌