Disable Int32 (and any similar) CodeGen types

[PatrickDanino]

Summary

While I understand that Int32 may technically be "more correct", this causes huge amounts of headaches when trying to use these types in real world projects. This is particularly true of cases where we already have thousands of other types that depend on Int, and converting each of those would be a headache.

The docs seem to imply that this wasn't meant to be final, but it appears to have been included without the option of turning it off. Can we switch it back or at least make this configurable in the code generator?

Version

2.0

Steps to reproduce the behavior

• Generate any type that uses Int

Logs

Anything else?

Docs

[calvincestari]

Hi @PatrickDanino, I remember the discussions we had amongst our team for this change after feedback and the burden you're expressing was specifically raised.

You're correct that v2.0.0-alpha-2 implemented that change for all generated Int fields but that was rolled back slightly in v2.0.0-beta-1 so that only input object variables are generated as Int32. Are you seeing Int32 generated in models that do not have input variables?

[PatrickDanino]

Hi @PatrickDanino, I remember the discussions we had amongst our team for this change after feedback and the burden you're expressing was specifically raised.

You're correct that v2.0.0-alpha-2 implemented that change for all generated Int fields but that was rolled back slightly in v2.0.0-beta-1 so that only input object variables are generated as Int32. Are you seeing Int32 generated in models that do not have input variables?

I did track down this code in the generator and it does appear to be limited to input types, but even those input queries are in the hundreds for us (i.e. we specify things like limit, first, etc. in nearly all our queries). At least for our use cases, I can't fathom a scenario where we'd get close to Int32 limits, so the change doesn't help us much.

I understand not all projects are as big as ours though. I guess we could try to work around this but for now it was simpler for me to disable this altogether as it seems like a distraction from the more important work of migrating to v2. As it stands, we're already having to not only migrate many of our older calls to Async, migrate lots of non-trivial code that used v1 functionality (like Apollo cancellables), migrate all our interceptors, but also migrate entire modules to Swift 6 since we were blocked from doing so. That's already a ton of changes.

All that to say that I understand where you guys are coming from, but this feels like it could have waited or have been a config option you introduced that at least would have given us the option to disable this for now while we deal with the rest.

[AnthonyMDev]

Thanks for the feedback @PatrickDanino. I'm sorry to hear the migration is really large for you guys. Its expected for the major version change to require a migration, and Apollo is a pretty core piece of infrastructure, so it's not too surprising, if there are other specific pain points (in addition to this one about Int32) that you've encountered, I'd be interested to hear about them and if there are things we can do to mitigate them.

---

There was discussion around the Int32 change before we made the choice for 2.0. I did explore the option of making this a configuration option, but that would have added a lot of complexity to the code gen engine and the complexity of the change would have caused us to have to bump back the release date.

Even if we started to build this as a config option now, I anticipate by the time we got a release out, you'd be done with your migration... I don't think that we want people to be using Int long-term, so adding it after people have migrated feels like moving backwards.

The docs seem to imply that this wasn't meant to be final

What does this refer to? If there is something in the docs indicating that, I need to update them!

This discussion has caused me to realize that I forgot make a section about the Int32 change in the migration guide. I'll add that now.

[PatrickDanino]

What does this refer to? If there is something in the docs indicating that, I need to update them!

I was referring to this blurb:

While we would like to keep this new behavior, if user feedback is negative, this may be reverted or made into an optional behavior based on a codegen configuration option.

That is closer to what I would have expected - an optional behaviour of codegen.

I can see the argument for wanting to support Int32, even wanting to change the default eventually, but it would be nice to have made this optional (opt-in) given it's still unclear who exactly this benefits. Not all of our changes are Apollo per se, but I'm sure we aren't the only ones who have been blocked from migrating to Swift 6 due to an Apollo dependency, and there's a lot of other things we need to worry about getting right without having to add on an Int32 change. Hope that makes sense!

For now I disabled the Int32 generation through a codegen change to unblock myself, so it's not the end of the world.

[AnthonyMDev]

Got it. Thanks for the feedback. I'd like to try to keep this from being opt-in if possible. We try to reduce the number of codegen config options as much as possible, since it's already so complicated. And we'd like people to migrate over to this behavior.

Glad you were able to modify codegen to unblock yourself for now. I hope that once you've finished the rest of your migration you are open to going back and migrating to Int32 at some point in the near future.

I don't plan on implementing this as an option currently, so I'm going to close this issue. I'm open to reconsidering if we get a lot of feedback from multiple people who want the option to disable Int32 usage. Particularly if people give me a good use case for when they would need this disabled permanently not just temporarily for migration purposes.

[github-actions]

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.