Apollo-ios: Separate ApolloCodegenLib

Created on 14 Apr 2020  路  3Comments  路  Source: apollographql/apollo-ios

I think it would be better to use Apollo with SPM if _ApolloCodegenLib_ was a separate repo.
I read #848 but I disagree with that being easier to test is a reason good enough to keep them together.

It's kind of annoying to have to clone _SQLite_ and _Starscream_ if you are not using it, but for those targets I can accept that the maintenance cost of keeping them separate would not be that trivial. But _ApolloCodegenLib_ is a completely separate product that the _Apollo_ product does not, and should not, have direct dependency on.
Right now if you add Apollo to your SPM project it adds 6 dependencies: _Apollo_, _SQLite_, _Starscream_, _Stencil_, _PathKit_ and _Spectre_. And I doubt that won't be the need for more in the future.

From what I've seem the SwiftCodegen is not complete and will still take sometime, so there is also the fact that many won't even be using it. In our project we have the generated code in another repo, so even after complete we won't need the code generation in the main project.

SPM improvements should help with that, but from what I understood from the evolution proposals
https://github.com/apple/swift-evolution/blob/master/proposals/0226-package-manager-target-based-dep-resolution.md
https://github.com/apple/swift-evolution/blob/master/proposals/0273-swiftpm-conditional-target-dependencies.md
It still will be cloned and affect the dependency resolution.

In summary, _ApolloCodegenLib_ is adding an unnecessary overhead for resolution/cloning, that probably will only get worse in the near future.

codegen question

All 3 comments

I definitely agree there shouldn't be a direct dependency of the Apollo lib on ApolloCodegenLib, that's 100% in the design here. ApolloCodegenLib actually won't even work on anything other than macOS.

I also agree that it's very annoying that if you're not using a particular target, its dependencies still get resolved by SPM. My reading of SE-226 is that they do _intend_ for this to eventually stop resolving unused dependencies, but it's not yet fully implemented.

And you are correct, Swift Codegen is definitely still WIP and gonna be a hot minute before it's ready.

All that said, though, I did consider having the codegen lib in a separate repo when I was initially designing this. Here's why I decided against that:

Realistically, each version of the iOS library is tied to a version of the code generation. This is the case now with the JS generator, which is why we set things up to use specific version of the CLI per version. This will continue to be the case with the Swift code generation, because any improvements we make to the generated code may require corresponding updates to the handling code in the SDK.

It's not a direct dependency, so you don't need to import all the stuff from one to the other, but it's immensely more difficult to work on either in a vacuum. In the past when I've had things that are highly dependent on each other in different repos, it becomes really easy to break the hell out of everything when things unintentionally get out of sync.

Having these libraries that are not direct _dependencies_ but still highly _dependent on each others' behavior_ in the same repo vastly increases the likelihood of tests that are designed to catch these issues actually being run all the time.

I hope that explains the decision - please let me know if you have further questions.

I also understood that they intend to improve the dependency resolution even further, but I could not find anything about it being done right now, so I don't think that we should wait for the problem to be solved by SPM.

And I honestly understand your decision, my main concern is that like I said, this is a problem that tends to only get worse as you add more tools to the generator.
I've seen some Packages use some scripts to avoiding propagating Packages used for testing, but I don't think there is a solution for real targets.

Well, I don't think I have any great arguments that could change your mind, you can close the issue if you want. But I hope you at least keep this in mind as the project progresses =)

Yeah - at this point updated codegen is the main thing being added to this lib anytime soon that might introduce any more dependencies, because it's going to be a LOT of rewriting stuff to make it all work.

Gonna close this out but will definitely will try to keep this in mind. Honestly, I'll probably bug the SPM team to see if they can fix it before I'd need to get around to needing it. 馃槆

Was this page helpful?
0 / 5 - 0 ratings

Related issues

ivanhoe picture ivanhoe  路  5Comments

vishal-p picture vishal-p  路  4Comments

ashiemke picture ashiemke  路  5Comments

hiteshborse12 picture hiteshborse12  路  4Comments

wnagrodzki picture wnagrodzki  路  4Comments