Apollo 1.0 Migration Feedback

[CraigSiemens]

We've been spending a couple days trying to update our project to use Apollo 1.0 but unfortunately it hasn't really been smooth sailing. For now we're probably going to pause our migration to 1.0 because the benefits don't yet outweigh the complexity in upgrading.

Also, sorry for the wall of text. I hope it's better than creating multiple issues for now since there's some overlap between the suggestions.

Our Project Structure

We have ~90 modules in our project with ~50 of them containing .graphql files requiring codegen. Most of those modules represent a self contained feature of the application with UI, models, and API code it needs. We try to keep the Apollo types isolated to the API layer and map them to our own model types before passing them onto the UI or other modules.

Each of those modules has a build phase to run the codegen when any of the graphql files are changed (using input xcfilelists). We have a swift package that is using ApolloCodegenLib. It runs codegen generating the single file in a temporary directory, makes some changes to the file, then moves it to the location in the project only if it's content is different so Xcode doesn't think it has to rebuild the file.

We also have a base API module that creates the interceptors, ApolloClient, and Scalar type definitions for all the modules to use.

Issues and Suggestions

The new codegen seems to be trying to help out with a projects module structure, but it feels like it makes the codegen options more complicated and less flexible than it could be. I’ve been trying to infer the supported structures from various options but it’s pretty complicated cause it feels like there’s hidden interdependencies between some of them.

For our project we’d just need a way to set the

• input and output paths for files
• optionally additional imports to add to those files

That would still allow having convenience inits for the options built on top of those while still allowing customization when needed.

---


schemaName would be clearer if it was called namespace, which was the old name in the codegen. That seems to be what it’s mostly used for. The current name makes it seem like it’s related to schemaFile and schemaTypes when it isn’t exactly referring to the same things. I think it's also sometimes used as as the name of a module and imported in operations.

If it's expected the schema types and operations can be written to separate locations, it'd probably be simpler if each could customize the namespace each uses and, with the suggestion above, directly control what import statements are added.

---

We’d like to limit the exposure of graphql types within our project so it’d be nice if there was an option to control the ACL rather than everything being public. The old codegen had an option for it but I believe it only worked with the experimental swift codegen, which I guess the new one replaced.

We previously had our script do a find and replace of public to internal. This was easy enough when only one file was generated, but now that there’s multiple it be more complicated.

---

cocoapodsCompatibleImportStatements could have a name that isn’t dependent on a specific decency manager. Even though we don’t use cocoapods, I feel like we may have to use this option for other reasons.

With the suggestion above adding better control over import statements, this could potentially be replaced by a convenience init, or a constant that can be passed for the imports to use.

---

It'd be nice to have an option to remove the @_exported on the imports. Again with our module structure we don’t want to be exposing Apollo to places that don’t need it. We also try to avoid using anything prefixed with underscores if we don’t have to.

Having control over the imports added would also solve this.

---

The custom pattern matching on GraphQLEnum can’t be used while also making a switch statement exhaustive. A default case is always required so the compiler can’t automatically detect when a new case is added. That makes it very easy for a developer to miss that a new case was added and be aware that the app should add handling for the new case.

While the old codegen creating __unknown always looked a little weird, it allowed a simple way to create exhaustive switch statements with the case names directly. It also allowed Xcode to autocomplete all the switch statement with all cases or add missing cases. Having to add case .case(.someEnumCase) for every case to get an exhaustive switch isn't a great experience.

[calvincestari]

Hi @CraigSiemens :wave:

We've been spending a couple days trying to update our project to use Apollo 1.0 but unfortunately it hasn't really been smooth sailing. For now we're probably going to pause our migration to 1.0 because the benefits don't yet outweigh the complexity in upgrading.

Let's see if we can ease some of those concerns and get you back on track to adopting 1.x sooner rather than later.

We have ~90 modules in our project with ~50 of them containing .graphql files requiring codegen. Most of those modules represent a self contained feature of the application with UI, models, and API code it needs. We try to keep the Apollo types isolated to the API layer and map them to our own model types before passing them onto the UI or other modules.

This doesn't sound like a particularly unique configuration and I would be surprised if there isn't an Apollo codegen configuration that doesn't work for your project.

Each of those modules has a build phase to run the codegen when any of the graphql files are changed (using input xcfilelists). We have a swift package that is using ApolloCodegenLib. It runs codegen generating the single file in a temporary directory, makes some changes to the file, then moves it to the location in the project only if it's content is different so Xcode doesn't think it has to rebuild the file.

You should still be able to do this although no longer with just a single output file. Unless there is any specific logic in your swift package for the code generation you should be able to achieve the same with the CLI + configuration file.

The new codegen seems to be trying to help out with a projects module structure, but it feels like it makes the codegen options more complicated and less flexible than it could be. I’ve been trying to infer the supported structures from various options but it’s pretty complicated cause it feels like there’s hidden interdependencies between some of them.

There is additional complication because 0.x had no support for modularization. I think there is a lot of perceived complexity in the configuration but we can certainly improve our documentation of that and consider configuration changes. In the next few days we will be adding a migration guide and a module type-specific guide, which should further explain those.

For our project we’d just need a way to set the

• input and output paths for files
• optionally additional imports to add to those files

• The input/output paths can be satisfied now with the current configuration struct.
• Additional imports are not possible at the moment but we are aware that some users would like the ability to augment those. One potential solution to this is to allow a custom pass over the generated code before writing to files.

schemaName would be clearer if it was called namespace, which was the old name in the codegen. That seems to be what it’s mostly used for. The current name makes it seem like it’s related to schemaFile and schemaTypes when it isn’t exactly referring to the same things.

I agree, and we have https://github.com/apollographql/apollo-ios/issues/2642 to address this.

If it's expected the schema types and operations can be written to separate locations, it'd probably be simpler if each could customize the namespace each uses

They should be thought of as separate 'groups' of output, which is why they each have a configuration option.

• When the schema types 'group' is placed into it's own module (.swiftPackageManager or .other), then --schema-name is used as the name of that module, therefore becoming the namespace.
• When the schema types 'group' is placed into an existing target (.embeddedInTarget(name:)), then --schema-name is used to create a caseless enum which is used as the namespace to avoid any potential name clashes.
• When the operation files are put in the schema types module (.inSchemaModule), then it assumes that namespace.
• When the operation files are placed outside of the schema types module (.relative(subpath:) or .absolute(path:)), they take on the namespace of the module they are manually placed into.

We’d like to limit the exposure of graphql types within our project so it’d be nice if there was an option to control the ACL rather than everything being public. The old codegen had an option for it but I believe it only worked with the experimental swift codegen, which I guess the new one replaced.

We previously had our script do a find and replace of public to internal. This was easy enough when only one file was generated, but now that there’s multiple it be more complicated.

This request is coming from a few places now and we've got https://github.com/apollographql/apollo-ios/issues/2630 to track it's progress. In hindsight it's something we should have carried into 1.0 initially.

cocoapodsCompatibleImportStatements could have a name that isn’t dependent on a specific decency manager. Even though we don’t use cocoapods, I feel like we may have to use this option for other reasons.

This is specific to CocoaPods because of the way it aggregates and exposes the imported dependent Apollo modules by the pod name. Access to ApolloAPI needs to done through import Apollo when using CocoaPods rather than the traditional import ApolloAPI which is what's generated when using SPM. That special case import statement would have no use outside of a CocoaPods environment.

It'd be nice to have an option to remove the @_exported on the imports. Again with our module structure we don’t want to be exposing Apollo to places that don’t need it. We also try to avoid using anything prefixed with underscores if we don’t have to.

We assume responsibility for using the underscored attributes and will adapt to any changes in their availability. We're aware of the potential risks and are comfortable with them.

The custom pattern matching on GraphQLEnum can’t be used while also making a switch statement exhaustive. A default case is always required so the compiler can’t automatically detect when a new case is added. That makes it very easy for a developer to miss that a new case was added and be aware that the app should add handling for the new case.

I'll come back to you on this one.

[calvincestari]

We have ~90 modules in our project with ~50 of them containing .graphql files requiring codegen. Most of those modules represent a self contained feature of the application with UI, models, and API code it needs. We try to keep the Apollo types isolated to the API layer and map them to our own model types before passing them onto the UI or other modules.

Each of those modules has a build phase to run the codegen when any of the graphql files are changed (using input xcfilelists). We have a swift package that is using ApolloCodegenLib. It runs codegen generating the single file in a temporary directory, makes some changes to the file, then moves it to the location in the project only if it's content is different so Xcode doesn't think it has to rebuild the file.

At first pass this sounds like a combination of .swiftPackageManager for output.schemaTypes.moduleType and .relative(subpath:) for output.operations should suffice. I'm not 100% sure whether the relative setting will work for your workflow, I suspect the legacy-style single generated API file caused you to work with it in a specific way.

This keeps the schema types in their own module that can be shared around, with the operation files relative to the modules where they're used. Unfortunately they can't be completely locked down right now because of the lack of an access modifier setting.

[jarrodrobins]

I have also attempted to migrate our app to Apollo 1.0 in the last week before the holidays. I anticipated a lot of breaking changes going into it but I thought it'd likely be a relatively straightforward process. I was wrong.

I apologise in advance if any of this comes across as overly emotive - I know everyone works extremely hard and are trying to do the best I can. But I also suspect there will be many others attempting this migration who will have more than a few under the breath grumbles as they do it, and if my grumbles can be alleviated it's going to make it a heck of a lot easier for someone else to come in later.

This migration is by far the most complicated and involved migration I've done since the 'we changed everything!' release of Swift 3.0. But at least that had fixits!

Our app is a lot smaller than OP's 90 module app - we essentially have just one API module which does a similar thing of mapping GraphQL types to our own types to keep things separate. Arguably it's a pretty simple module, but seeing how difficult a time I've had this week really makes me suspect you're going to end up with a lot of teams sticking with 0.53 for a long, long time.

The fundamental problem

Fundamentally I believe the major problem with this Apollo release is that it does too much. It shouldn't be 1.0, it should be something like 5.0, with each of the many, many breaking changes spread out across releases so we can migrate one part at a time rather than Big Bang it and have no confidence that I'm not breaking anything along the way. I can't test one part of it until I've done the whole thing because of the innumerable compiler issues that need to be fixed, so I can't verify anything in isolation.

Setup issues and getting the generated code to compile

Right off the bat, your migration guide suggests installing a new CLI tool (which, installing from Xcode didn't even work until the 1.0.5 release - it's the first step!), then suggests setting up an entirely new Swift package for the generated files. Yes, you can just pick a directory, but then you lose the namespacing, so you really want to go the package route. Frankly, an architecture change as major as this should be step 10, not step 1, when I just want to get things compiling again.

Once I had my new Swift package, it didn't even compile! So, off to GitHub Issues I go, and then I find this issue. So, then I had to rename all my fragments to add the 'Fragment' suffix to avoid name collisions. We have lot of fragments, so that's going to cause a lot of code changes.

Next step was setting up scalars again, which have a similar, but slightly different API and need to live in the new Swift package now. This is where I was starting to think this release needs to be broken up across several major releases, though to be fair this step was at least relatively straightforward once I worked out what I needed to do. Though I did have to do a couple of silly typealiases like public typealias UUID = Foundation.UUID to get the generated code to compile. Nothing too big, but it does feel like something that'd be fairly common that did require a bunch of investigation.

Finally, the module compiles, so it's time to fix my actual code.

99 compiler errors to go, recompile, 109 compiler errors to go

First up, can I just say how amazingly thankful I and the rest of my team are that fragments are generated in a much better way now (much the same as the Android SDK) - previously we had a lot of problems mapping them to our own types, and this seems to be massively cleaned up, and I was able to delete a lot of boilerplate glue code we had.

Then came the need to put GraphQLNullable and GraphQLEnum all over the place. Here's where in my ideal world we'd have another major Apollo release. Like OP said, having case .case(.someEnumCase) is really not ideal, and fixing this all over my codebase was probably where I spent the majority of my time for a change that honestly felt little more than bikeshedding to me. GraphQLNullable is probably an improvement over the weird double optional stuff that was there before, at least.

Eventually I breathed a sigh of relief that I was able to get the thing to compile. Time to fix the tests!

Tests

This is where I nearly threw my laptop out the window.

To test all our mappings, we have hundreds of tests which directly instantiate the Data objects, and then create our own types from them. In Apollo 1.0, instead of simply being able to instantiate the Data objects directly, all the initialisers have been removed, and I need to work out how to use the new generated mocks! Another big architecture change!

I'll take the migration guide's assertion that this was 'cumbersome, error-prone and fragile' at face value (it worked well enough for my use case, anyway - it's typesafe and easy to do). But I'd humbly suggest that instead of just nuking the old way from orbit that you keep it in and add in deprecation warnings? Apollo 1.0 is already massive enough. That way I can at least migrate across slowly rather than now having to fix hundreds of files in one go.

Other smaller issues

• Docs on exactly how to set up the generated frameworks would be nice. I've tried to add my generated mocks framework to my test target, which then doesn't compile. I can only add it to the module I'm running the tests against, which isn't really what I want to do. Not sure if I'm doing something silly, but I have hundreds of tests to fix before I spend any more time on this.
• Running the codegen keeps resetting my Package file's minimum supported iOS version to 12 (15 is my minimum, and my scalars use some 'newer' methods). I have to keep resetting this manually every time I run it - is there a way to specify it in the config file?
• Previously I was able to get the operation name from GraphQLQuery, which I was using in some testing code for building a mock API client. Now that type doesn't have any exposed vars or methods on it, so I'm using String(describing: query) to get the name (which feels pretty hacky). Is there a better way/can we get the var back?

tldr; I totally get that a major release will have, well, major changes. But that many? I honestly want to pause migration as well but am dreading having to go through this pain again in future so I'm trudging through. At the time of writing I've only fixed one test file, after my 100th I might be of a different view.

[calvincestari]

Right off the bat, your migration guide suggests installing a new CLI tool (which, installing from Xcode didn't even work until the 1.0.5 release - it's the first step!)

The Install CLI tool was introduced in 1.0.4 to address the problems users' were experiencing in Xcode with the previous SPM plugins. That history is all in the changelog.

Yes, you can just pick a directory, but then you lose the namespacing, so you really want to go the package route.

I'm not sure what you mean by "lose the namespacing". If you can share some more detail I'd like to understand the problem better.

When using the .embeddedInTarget(name:) module type the schema types are namespaced within a caseless enum named the the same as the value given for schema-name in the configuration. This is detailed in the documentation for the Embedded in Target option.

Once I had my new Swift package, it didn't even compile! So, off to GitHub Issues I go, and then I find this issue. So, then I had to rename all my fragments to add the 'Fragment' suffix to avoid name collisions. We have lot of fragments, so that's going to cause a lot of code changes.

This has been a common piece of feedback and something we'll be addressing within the next couple releases.

Though I did have to do a couple of silly typealiases like public typealias UUID = Foundation.UUID to get the generated code to compile. Nothing too big, but it does feel like something that'd be fairly common that did require a bunch of investigation.

If you have suggestions, or a PR, for improvements to the Custom scalars portion of the migration guide we'd love to see it.

Like OP said, having case .case(.someEnumCase) is really not ideal, and fixing this all over my codebase was probably where I spent the majority of my time for a change that honestly felt little more than bikeshedding to me. GraphQLNullable is probably an improvement over the weird double optional stuff that was there before, at least.

GraphQLNullable is certainly an improvement over the previous double optional syntax, it's definitely been a contentious change though.

To test all our mappings, we have hundreds of tests which directly instantiate the Data objects, and then create our own types from them. In Apollo 1.0, instead of simply being able to instantiate the Data objects directly, all the initializers have been removed, and I need to work out how to use the new generated mocks! Another big architecture change!

Another option is to build and maintain your own extension for the struct initializer, it may have been easier than modifying all the tests. Plus has the benefit of being able to be used for SwiftUI previews, if your project uses them.

No, having to create the initializers yourself is not an optimal solution. This is another contentious change that we may have to review and workaround.

I've tried to add my generated mocks framework to my test target, which then doesn't compile. I can only add it to the module I'm running the tests against, which isn't really what I want to do. Not sure if I'm doing something silly, but I have hundreds of tests to fix before I spend any more time on this.

What is the setting you're using for test mocks in the codegen configuration?

Running the codegen keeps resetting my Package file's minimum supported iOS version to 12 (15 is my minimum, and my scalars use some 'newer' methods). I have to keep resetting this manually every time I run it - is there a way to specify it in the config file?

Unfortunately not.

Previously I was able to get the operation name from GraphQLQuery, which I was using in some testing code for building a mock API client. Now that type doesn't have any exposed vars or methods on it, so I'm using String(describing: query) to get the name (which feels pretty hacky). Is there a better way/can we get the var back?

There is a property operationName in each generated query class but that is the actual operation name from the GraphQL operation definition. It sounds like you're wanting the class name instead of the operation name? If that's correct then String(describing: query) is the best thing for now.

This migration is by far the most complicated and involved migration I've done since the 'we changed everything!' release of Swift 3.0. But at least that had fixits!

Fundamentally I believe the major problem with this Apollo release is that it does too much. It shouldn't be 1.0, it should be something like 5.0, with each of the many, many breaking changes spread out across releases so we can migrate one part at a time rather than Big Bang it and have no confidence that I'm not breaking anything along the way.

tldr; I totally get that a major release will have, well, major changes. But that many?

In an ideal world Apollo iOS would have a far better version history of getting to a first major release and beyond. Which would lead to the more evolutionary progression you're after. But it doesn't and we're now on the path of overhauling the three major parts to the SDK in each of the next major versions.

• 1.0: code generation
• 2.0: networking stack
• 3.0: caching

Code generation I think will be most disruptive because of the major ways in which the models changed. Networking might be too since the interceptors paradigm is not great. Caching much less because its feature minimal at the moment so 3.0 will mostly be additive.

No, these changes are not easy depending on the complexity of your project. We appreciate the feedback and will take these learnings into the next major releases. We strongly believe that the changes now are building a better foundation on which we can build an even better product.

[AnthonyMDev]

Thanks for all your feedback! @CraigSiemens

Each of those modules has a build phase to run the codegen when any of the graphql files are changed (using input xcfilelists). We have a swift package that is using ApolloCodegenLib. It runs codegen generating the single file in a temporary directory, makes some changes to the file, then moves it to the location in the project only if it's content is different so Xcode doesn't think it has to rebuild the file.

This sounds really awesome! I'd love to get my hands on the infrastructure you've built here and see if we could eventually integrate it into a tool to provide to others.

We also have a base API module that creates the interceptors, ApolloClient, and Scalar type definitions for all the modules to use.

I think the generated group of "schema" files should be included in this module, it generates template files for your scalar types and all of the necessary files that your generated operations need.

The custom pattern matching on GraphQLEnum can’t be used while also making a switch statement exhaustive. A default case is always required so the compiler can’t automatically detect when a new case is added. That makes it very easy for a developer to miss that a new case was added and be aware that the app should add handling for the new case.

Yeah, we are aware of this limitation. I know it's not ideal, but Swift can't deduce switch exhaustivity with the pattern matching operator. We provide it as a convenience, but to do what you are asking, there are a few options.

If you need to be able to handle the unknown cases using their rawValue you can do a nested switch like this:

let enum: GraphQLEnum<MyEnum>
switch enum {
case let .case(value):
  switch value {
    case .myValueA: // ...
    case .myValueB: //...
  }
case let .unknown(rawValue):
  // handle rawValue
}

If you just have a fallback when the value is unknown that doesn't care about the rawValue, or want to omit unknown values completely, use the GraphQLEnum.value property. This will just return nil for unknown cases

let enum: GraphQLEnum<MyEnum>
switch enum.value {
    case .myValueA: // ...
    case .myValueB: //...
    case .none:
      // handle nil case for unknown values
}

Either of these will give you exhaustivity and not require a default case.

[AnthonyMDev]

And thank you as well @jarrodrobins I've got some things to add.

Overall, I do understand your frustration, the 1.0 is a huge release with a lot of fundamental breaking changes. We really re-thought a lot of the underlying principles to get to a point where we had a stable API that we felt we could build on in the future without having to make major breaking changes continually. We decided to go the route of ripping the band-aid off all at once.

While that is going to make the initial migration a lot more work, we felt like it also allowed us to have a much higher quality product. A lot of these little things were tied to each other in ways that are not immediately obvious, and trying to incrementally do a few things at a time, we would have had to make a lot of sacrifices that we felt would have deteriorated the quality of the end product. It also would have probably caused a lot more bugs over time as we handled the edge cases of all the incremental improvements being backwards compatible with the stuff we hadn't fixed (broken?) yet.

To test all our mappings, we have hundreds of tests which directly instantiate the Data objects, and then create our own types from them. In Apollo 1.0, instead of simply being able to instantiate the Data objects directly, all the initializers have been removed, and I need to work out how to use the new generated mocks! Another big architecture change!

Because initializing test mocks with straight JSON data is error-prone, we created the TestMocks framework, but you can still do what you were doing before, the initializer is just a bit more cumbersome.

MySelectionSet(data: DataDict(["MyData": "AsAJSONResponse"], variables: nil)) is the way to do this. A find and replace with a regex over your test targets should be doable here.

Running the codegen keeps resetting my Package file's minimum supported iOS version to 12 (15 is my minimum, and my scalars use some 'newer' methods). I have to keep resetting this manually every time I run it - is there a way to specify it in the config file?'

This seems like a reasonable request! Please file an issue and we'll add it to an upcoming minor release!\

Previously I was able to get the operation name from GraphQLQuery, which I was using in some testing code for building a mock API client. Now that type doesn't have any exposed vars or methods on it, so I'm using String(describing: query) to get the name (which feels pretty hacky). Is there a better way/can we get the var back?

As Calvin stated, operationName is a static var on the type. You should be able to access this on the type just fine. If you're still struggling with this, can you be a bit more specific on the use case in which this is a problem for you?

[jarrodrobins]

Thanks both @calvincestari and @AnthonyMDev for the replies.

I totally understand why you've gone for the big rip-off-the-bandaid release, as rewriting any piece of software will generally necessitate that in some respects, and I do acknowledge making all the changes will end up with the better product even if the initial migration has its share of pain.

The Install CLI tool was introduced in 1.0.4 to address the problems users' were experiencing in Xcode with the previous SPM plugins. That history is all in the changelog.

That's my mistake - I initially tried upgrading to 1.0.0 rather than 1.0.5 to minimise changes, but the docs had also changed. That's on me. D'oh.

I'm not sure what you mean by "lose the namespacing". If you can share some more detail I'd like to understand the problem better.

Despite my typing and pre-holiday brain mush I was actually referring to access modifiers and how all the generated code is public as opposed to internal. Looks like that's planned on being fixed in #2630.

What is the setting you're using for test mocks in the codegen configuration?

I ended up getting the test mocks to compile - think this may have been Xcode playing up. It'd have problems finding ApolloAPI for some reason. Don't ask me what I changed - removing and readding the framework to my test target seemed to work. 🙈

Because initializing test mocks with straight JSON data is error-prone, we created the TestMocks framework, but you can still do what you were doing before, the initializer is just a bit more cumbersome.

MySelectionSet(data: DataDict(["MyData": "AsAJSONResponse"], variables: nil)) is the way to do this. A find and replace with a regex over your test targets should be doable here.

We might be talking about different things here as I wasn't going anywhere near SelectionSet - that would be very error prone, I agree! What I was doing was this -

let response = Generated.MyQuery.Data(
    getCards: .init(
        cards: [
            .init(
                id: UUID(),
                activationStatus: .activated,
                cardType: .physical,
                cardConfig: .make(),
                nameOnCard: "xxx",
                nickname: "yyy",
                createdDate: Date(),
                ...
            )
        ], 
        hasMore: false
    )
)

let mappedObject = response.toMyMappedObject()
// test mapped object values are what I expect

whereas now, I need to do this -

let response = Generated.MyQuery.Data.from(
    Mock<GeneratedMocks.Query>(
        getCards: Mock<GeneratedMocks.PaginatedCards>(
            cards: [
                Mock<GeneratedMocks.Card>(
                    activationStatus: GraphQLEnum.case(.activated),
                    cardType: GraphQLEnum.case(.physical),
                    createdDate: Date(),
                    id: id,
                    nameOnCard: "xxx",
                    nickname: "yyy",
                    ...
                )
            ],
            hasMore: false
        )
    )
)

let mappedObject = response.toMyMappedObject()
// test mapped object values are what I expect

The additional burden here (and which I've spent the last few days on) is that the mocks also have a different ordering for all the parameters! They're now alphabetical as opposed to the order they were put in in the GraphQL file. Xcode is fairly unhelpful with fixits so this was a lot of manual stuffing around putting everything in the correct order. In future it may be helpful to have some sort of configuration option here.

This seems like a reasonable request! Please file an issue and we'll add it to an upcoming minor release!

Will definitely do this, thanks.

As Calvin stated, operationName is a static var on the type.

You can grab it from a concrete type, but not from the generic GraphQLQuery or GraphQLMutation protocols, which was the case previously. My use case is I have a mock GraphQL client in my tests that instead of going off and hitting the network, I can just give it response data which it'll return when it gets a particular query. I'm after the operation name so I can easily record which queries are called and the order. Using String(describing: self) does work but it's a bit hacky.

If you're interested in any metrics, I have just finished our migration and this is the result of my PR with the vast majority of the changes being in my test files. Definitely not a small one!

[AnthonyMDev]

For your test mocks, is all the qualified name spacing required to make your code compile? That's definitely verbose and ugly. Not my intentions. My hope is that this can be reduced down to at least this:

let response = Generated.MyQuery.Data.from(
    Mock<Query>(
        getCards: Mock<PaginatedCards>(
            cards: [
                Mock<Card>(
                    activationStatus: .case(.activated),
                    cardType: .case(.physical),
                    createdDate: Date(),
                    id: id,
                    nameOnCard: "xxx",
                    nickname: "yyy",
                    ...
                )
            ],
            hasMore: false
        )
    )
)

You can grab it from a concrete type, but not from the generic GraphQLQuery or GraphQLMutation protocols, which was the case previously.

Can you use type(of: myQuery).operationName to get the runtime type, or does that not work in your case?

If you're interested in any metrics, I have just finished our migration and this is the result of my PR with the vast majority of the changes being in my test files. Definitely not a small one!

I'd love more insight into why it's such a large addition of lines of code! I'm hearing that your test mocks are a lot more work now, but one of the main goals was to decrease the size of the generated code significantly, so I would assume to see a migration resulting in less lines in most cases.

[AnthonyMDev]

I'm going to close this, just because it's not an concrete action item for us to work on. We are happy to continue the discussion and receive more feedback on this issue. Please feel free to keep commenting! It just doesn't need to be in our open "issues" list.

We're not going to disallow discussions on GitHub issues, but for things that are discussions/feedback, rather than individual actionable feature requests/bugs, it might be more appropriate to use the Apollo Discourse Boards instead of GitHub issues in the future!

[jarrodrobins]

For your test mocks, is all the qualified name spacing required to make your code compile?

Oh, definitely not needed at all, I've just been doing it to make a clear distinction between generated code and any model objects I might have. Agree it's a bit ugly, and probably overkill. I may clean some of it up now that I've got all my code compiling and tests passing now.

Can you use type(of: myQuery).operationName to get the runtime type

Yes I can! Didn't even think of that - works perfectly, thanks.

I'd love more insight into why it's such a large addition of lines of code!

Turns out the big increase in the lines of code was silliness on my behalf - I moved around the Apollo generated code into a new directory (since it's now part of a framework) but forgot to actually delete the old code from the repo. 🤦

Here's a (possibly) more sensible metric, though being down 58k lines sounds fairly suspicious too (maybe Gitlab's getting confused). In terms of actual code in my app it's fairly similar to how it was (smaller in some spots where I've been able to remove some stuff). The number of files changed doesn't surprise me as basically everything touching Apollo needed at least one change (eg a GraphQLNullable/GraphQLEnum at minimum).

[CraigSiemens]

We still haven't had any more time to devote to this but hopefully some of the suggestions will help.

---

Each of those modules has a build phase to run the codegen when any of the graphql files are changed (using input xcfilelists). We have a swift package that is using ApolloCodegenLib. It runs codegen generating the single file in a temporary directory, makes some changes to the file, then moves it to the location in the project only if it's content is different so Xcode doesn't think it has to rebuild the file.

This sounds really awesome! I'd love to get my hands on the infrastructure you've built here and see if we could eventually integrate it into a tool to provide to others.

Here's the really high level version of it.

• Our scheme has a build pre-action script that goes though each of our modules and finds the .graphql files it contains, then writes them out to a xcfilelist.
• Each module had a codegen build phase script
  • they have input and output files to allow Xcode to skip running the script if none of the input files have changed
  • input files are the generated xcfilelist and the schema
  • output file was the generated Apollo file (this'll need to change to something else)
  • it calls a command in our swift package that
  • calls the Apollo codegen with a temp output dir
  • apply our custom changes to those files
    • replace public with internal
    • add some extra imports
    • add SwiftLint disable comments
  • move the temp file to the final location only if the content is different
    • this prevents Xcode from thinking the file has changed causing it be rebuild

[AnthonyMDev]

@CraigSiemens Thanks so much for this overview! I definitely would like to add more tooling like this to our codegen engine in the future!

-apply our custom changes to those files

• replace public with internal
• add some extra imports
• add SwiftLint disable comments

The new codegen engine was written in Swift to make it easy for you to alter the code generation templates! Rather than calling into a separate command that alters the written files, it would be really, really easy for you to fork the codegen engine and modify the templates to include these changes!

All 3 things that you have on that list are features that we do aim to make available via configuration in the future. So if you ever consider make any of these changes in the code gen engine itself, adding them as configuration options would make it feasible for us to accept a PR for these into the main repo!