-
Notifications
You must be signed in to change notification settings - Fork 734
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
Apollo iOS users: we want to interview you! #3197
Comments
Hello! We're currently in the process of integrating Apollo with our iOS and Android apps, as well as embedding Apollo's Explorer editor in our staff UI for our engineers to work with. I'm pretty busy/don't have time for an interview, but since you're soliciting feedback I figured replying here would be good? Some thoughts, in no particular order: Positives:
Opportunities:
CLI in general is annoying. CLI with a bunch of arguments I have to keep in a text file? More annoying. CLI with a bunch of arguments that also loads data from a JSON with a bunch of config options I have no way to easily discover? Really annoying. I mean I figured it out, it's setup now, and frankly I don't want to ever touch it again. Not the best first-impression. I'm sure people will disagree, this is just my 2c.
As of now we're on track to ship Apollo on millions of devices in the next few weeks, and we couldn't be more excited about it as an Engineering team. All the best! |
Thanks so much for the feedback @RamblinWreck77! It truly means a lot and will help us improve 🙏🏻 the team will look into these suggestions. Are you able to share more about your use case / team / app? Feel free to email if you would prefer not to post it publicly: jeff.auriemma [at] apollographql [dot] com |
Plus one to this! In particular regarding SQLite.swift. It seems to produce a nasty SPM bug where our only resolution was to fork Apollo to remove the problematic bit of SQLite ourselves. It's particularly frustrating when my team has no need for any parts of Apollo that are dependent on SQLite. |
I attempted the migration to 1.0 almost a year ago and gave up. I'm starting on my second attempt today. If Apollo made a tutorial video on the migration process that would have been super helpful. |
This migration was incredibly painful for me as well. Fortunately some bits of the change were reverted / made more flexible (immutable properties on generated fragments), but it still took about half a year on and off to finally get to the end of this for our large codebase, which was time that could have been spent on...anything else. The PR that will update Apollo is still massive and hard to adequately review because of the changes that needed to be made at once, and we're bracing ourselves for a potential need to hotfix the release that this PR goes into. It's a really concerning change that puts our business at risk. #2678 (comment) really perfectly hits the nail on the head, particularly this bit:
Please, please, please in the future try to split breaking changes into smaller chunks that can be addressed one at a time, when that might be possible to do. And to be clear, I absolutely respect the massive amount of effort that clearly went into the 1.0 release, and that sometimes big changes can't always be performed in isolation. |
I just created #3207 if anyone has a sample project with ApolloCodegenConfiguration.TestMockFileOutput.swiftPackage(targetName:) Please send me a link. I'm stuck trying to get my existing mocks migrated. Thanks! |
Thanks @dafurman, @chasemac, and @dafurman! I truly appreciate your candor. @chasemac we'll take a look at that issue you opened ASAP. Please let us know how we can be of further assistance as you continue migrating to 1.0. 0.x to 1.0 required a lot of foundational changes that were backwards-incompatible. Many of these changes were interrelated and needed to happen in tandem. The good news is that we anticipate v2 to largely build upon the foundation that the maintainers have built and are building over the life of v1. So @dafurman, at this early stage we think it’s unlikely that the amount of effort it took users to migrate to 1.0 will be comparable to 1.x to 2.0 (with the caveat that 2.0 has yet to be fully scoped). |
I created a PR to the test app to add tests and upgrade it to version 1.4. Please let me know if I did this correct. Thanks! apollographql/iOSTutorial#10 |
I stumbled upon another issue that lead to a suggestion that would (in my opinion) aid iOS devs adopting Apollo. It seems like a common pattern in any org adopting GraphQL is to release a subset of their API's features on a "v2/v3/v4" API that's GraphQL, as it's simply not realistic to switch everything over at once. So from the mobile side you probably have to deal with split networking implementations for a while, lets call it On iOS there's a lot of arcane/bizarre reasons why having multiple Since Apollo creates it's own What would be great is if I could create a single static URLSession that I configure for my app on-boot, then pass it in for ApolloClient to use (also on-boot), so that my Outside of just migration, an app might still need to interact/make network calls to 3rd party APIs that are not GraphQL and it sure would be nice to be able to dispatch regular calls on the same underlying session. |
Thanks @RamblinWreck77, that's something to take into consideration for 2.0. For now let's keep the conversation of that issue in #2851. |
Another really frustrating thing about the migration to 1.x. I thought I finished it on Friday. I redid all my mocked data with the new mock methods. All my unit tests and UI tests passed. Then I went to create an archive to publish the app and I ran into this error: "Module 'ApolloAPI' was not compiled for testing. It appears that all the new mocks I created to replace my old ones for my SwiftUI previews and units tests aren't available to use because they require the 'ApolloTestSupport' module. I wish I knew that at the beginning before I finished all of this. That's why I asked for a video showing someone migrating a project. And a sample project that has unit tests and UI tests that use mock data. I am super frustrated right now. There's a chance iOS 17 comes out tomorrow and I'm not ready because this is taking way too long. |
My team did the same thing. I am now working on our second attempt at migration, one year after the first attempt, and it is Sisyphean. Not only is there another year's worth of stuff to migrate, infrastructure included, but every day that passes means more pre-1.0 code is written that needs to be migrated due to the breaking changes. Each rebase is more painful than the last. And that's not to mention the extreme risk of merging a feature branch with a high-six-figure diff, and the extraordinary challenge and time-sink of getting it reviewed even piecemeal. It is still unclear if the migration is actually going to be mergeable at any point or if we'll have to cut our losses and just accept running a pre-1.0 version. At the same time, I understand the reasoning behind the changes and I agree with them in principle. But unfortunately making a change of this scale all at once is close to being infeasible for any codebase of a sufficient size, in my opinion. I guess my feedback is that I really hope that in the future, Apollo will consider making breaking changes like this in a way that is less all-or-nothing and more akin to the incremental nature of moving from objc to swift or uikit to swiftui, which is much more compatible with large codebases. Keeping the old generated files as-is and having any new files be generated with the new API, for example, would make migration far more manageable. My second piece of feedback is that this migration has subjectively felt like it was more painful than it needed to be. Perhaps I was just looking in the wrong places, but the little documentation I was able to find was incomprehensive and left me fumbling around in the dark trying to piece things together. There have been many occasions where something took hours of trial and error which could have taken minutes in retrospect, had there been documentation to tell me how something works. |
Thanks for the feedback @anavetotwitch - could you please point me to current documentation which could be improved or missing documentation that would have helped you. Any guidance to help make the migration better supported would be great. |
@anavetotwitch there's a way to not have to recreate all your sample data from scratch. It's just not documented very well. But that would have been a huge help to me if I knew that before I migrated a few weeks ago. |
@calvincestari The biggest stumbling block for me was mocking. The only relevant page I was able to find was https://www.apollographql.com/docs/ios/testing/test-mocks/, which did not go into sufficient detail for my needs. The fact that generated files and generated mock files aren't 1:1 made it confusing in the beginning to figure out how to migrate our mocking infrastructure to Apollo. For the first few queries/mutations, I found a Could just be pebkac, or an imperfect schema, but that was a frustrating experience for me and resulted in a significant amount of wasted time. I realize looking back at that page that @chasemac Can you please elaborate? This sounds like it would be helpful. |
@anavetotwitch Add selectionSetInitializers to your codegen config file. They don't really have it documented and I can't comment on GitHub from my work computer. |
@anavetotwitch here's an example of my config. I have my codegen in a swift package. I wasted a week on mocks and got most of it working and then my project couldn't archive so I had to delete it all and then I found selectionSetInitializers in some old GitHub issues. |
This is due to a DocC bug in the API reference docs we haven't figured out yet. You can see it's present in the docc archive but not being rendered for some reason.
The best way to think about this is that the test mocks are for schema-based mocking whereas the selection set initializers are better for model-based mocking.
|
@chasemac Thank you! @calvincestari Yep that makes sense, just took a while to figure out |
#3197 (comment) If this is just a library for helping generate mock object, it would be really helpful if it can be used in non-test target. Since in SwiftUI development, we would add preview with mock object, if the mock object can only be used in Test target, the preview can not benefit from the code completion from ApolloTestSupport, so we have to use dictionary to mock the apollo model or use custom data type instead |
@bingyi-ad Please see this comment, it sounds like what you want is the selection set initializers. |
Thanks so much everyone! To those of you who reached out, we appreciate having had the opportunity to speak with you. Your feedback has been truly helpful and will make a difference. We'll also be trying to get more feedback in the future through other means like surveys as well. For folks who have been discussing specific topics in this thread, please do feel free to open an Issue with specifics and/or direct your comments to the relevant Issue(s) in the repo 🙏🏻 |
Question
Hi all 👋🏻 Jeff Auriemma here, I'm the Engineering Manager serving the client library maintainers here at @apollographql. If you use Apollo iOS, I want to talk to you 👀 We want to build the absolute best open-source GraphQL client SDK possible, and no one knows what you need better than, well, you! If you can spare 45-60 minutes to help make our open-source software better, we'd be so grateful. Feel free to email me at jeff.auriemma [at] apollographql [dot] com or find me on Discord
@jeff.auriemma
and we'll figure out a date and time. Thanks so much for supporting our open-source software!The text was updated successfully, but these errors were encountered: