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

Changing enum on schema from required to optional breaks builds #3455

Closed
wow-such-amazing opened this issue Oct 3, 2024 · 3 comments
Closed
Labels
bug Generally incorrect behavior needs investigation

Comments

@wow-such-amazing
Copy link
Contributor

wow-such-amazing commented Oct 3, 2024

Summary

Hello Apollo Team!

Recently our backend team released a change to make a property optional on the input type instead of it being required. This is considered as a non-breaking change schema-wise as we lift of one the requirements.

However, such change is not compatible with the generated code that we have. One could argue that we should update the app to support new schema. However that's an unexpected extra work. Plus in our case this breaks our build pipeline because during the build we fetch the schema to ensure we always use a correct one. Again, one could argue that this is unnecessary, but overall this doesn't discard the underlying problem in my opinion.

Version

1.15.1

Steps to reproduce the behavior

Schema before:

AddressInput {
  type: AddressEnum!
}

Schema after:

AddressInput {
  type: AddressEnum
}

As you can see, property became optional. And because of that we have this build error that type can't be resolved:
373386224-c3a4ca38-9a99-47e3-98f4-9daa9d6e0d3f
To make it work we need to re-write the code as the following:
373386473-f3daa28a-c2f8-466a-a75a-a37200467166
From what I know this doesn't raise an issue for Android builds and for Frontend builds in our team.

Logs

No response

Anything else?

No response

@wow-such-amazing wow-such-amazing added bug Generally incorrect behavior needs investigation labels Oct 3, 2024
@AnthonyMDev
Copy link
Contributor

Hi @wow-such-amazing, thanks for the input here! We've thought a lot about this issue in the past, and haven't really been able to come up with any sort of solution that fixes this. Swift enums are kind of difficult to work with, and while GraphQLNullable is great for ensuring correctness and expressivity of your nulls, it exposes some awkward cases like this.

In my mind, backwards compatibility requires that existing applications out in production won't break when a schema change occurs. Which is currently the case. But I don't think there is a way to prevent you from needing to change your code when recompiling against the updated schema.

You're right, this probably isn't an issue for the other teams. Unfortunately, that is due to the nature of enums in Swift. We tried to make this work as well as possible. We used the ExpressibleBy___Literal protocols, so that if your input value was a String, or an Int, it would just work. But for custom types like Enums, there is just no way to make this happen in Swift.

I wish there was a better answer for you. We've had to weigh many trade offs when designing Apollo iOS, and this is one of the concession we knew we had to make when designing the way to deal with null input values. GraphQLNullable isn't perfect, but we still feel it's an improvement on what we had in Apollo iOS 0.x, which was doubly nested optionals (AddressEnum??) that didn't make it clear why you would set the input to nil or .some(nil) and also had a bunch of odd syntactic quirks that were far less explicit and expressive.

@AnthonyMDev AnthonyMDev closed this as not planned Won't fix, can't repro, duplicate, stale Oct 4, 2024
Copy link
Contributor

github-actions bot commented Oct 4, 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 iOS usage and allow us to serve you better.

@wow-such-amazing
Copy link
Contributor Author

Hey @AnthonyMDev, thanks for the reply! I understand the reasoning and personally I don't have a strong opinion which approach is better, but wanted to double check with you since such issues block our backend team sometimes to release changes on the schema. Thanks again for the swift answer and clarification 🙏

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Generally incorrect behavior needs investigation
Projects
None yet
Development

No branches or pull requests

2 participants