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

Add Namespace for ApolloAPI types in Generated Code #2581

Closed
matijakregarGH opened this issue Oct 13, 2022 · 21 comments · Fixed by #2585 or #2679
Closed

Add Namespace for ApolloAPI types in Generated Code #2581

matijakregarGH opened this issue Oct 13, 2022 · 21 comments · Fixed by #2585 or #2679
Assignees
Labels
bug Generally incorrect behavior codegen Issues related to or arising from code generation
Milestone

Comments

@matijakregarGH
Copy link

Feature request

There is a high probability of naming clashes in client app codebase and Apollo types. Name-spacing or prefixing would solve this issue.

Motivation

There is an actual problem that I'm facing.
I wanted to have DocumentType as enum in my app, however all generated operation files contain the following line:
public static let document: DocumentType = .notPersisted( ... that causes a build error.

Proposed solution

There are a few solutions that I can think of:

  1. Specifying the module in this specific case: public static let document: Apollo.DocumentType = .notPersisted( .... This one is more of a quick fix, but it would solve this particular problem.
  2. Using a namespace to prevent other potential clashes.

Outstanding Questions

Perhaps there's another way of getting around this issue. If I'm missing something I'd appreciate further instructions 🙂

@AnthonyMDev
Copy link
Contributor

Oh, that's a good point. We've actually done this for a ton of other things already, we just didn't consider DocumentType. I'll do an audit and make sure we aren't missing any others that I can tell.

I think we will likely go with your option 1 for solving this actually. Keep an eye out for a fix in 1.0.2 soon!

@AnthonyMDev AnthonyMDev added this to the Next Release (1.0.1) milestone Oct 13, 2022
@AnthonyMDev AnthonyMDev changed the title Consider a namespace for Apollo types. Add Namespace for ApolloAPI types in Generated Code Oct 13, 2022
@AnthonyMDev AnthonyMDev self-assigned this Oct 13, 2022
@matijakregarGH
Copy link
Author

I'm sorry to report that the fix still doesn't help with my issue.
The generated files still contain public static let document: DocumentType = .notPersisted(... that causes a build error. I have to manually change it to public static let document: Apollo.DocumentType = .notPersisted( for the app to compile.

Reason being that DocumentType is also an enum in my GraphQL schema.

@calvincestari
Copy link
Member

Thanks for letting us know @matijakregarGH, are you able to put together a sample project demonstrating the build error?

@matijakregarGH
Copy link
Author

I could, but it will take a while. There is a naming clash between DocumentType in the schema and Apollo.DocumentType. Specifying the Apollo module in the generate files solves the clash. See proposed solution 1. above.
If you need me to, I'll provide a sample project when I get back from my vacation (sorry about that).

@calvincestari
Copy link
Member

That's what the fix in #2585 is meant to resolve, hmm. Are you able to share the codegen configuration you're using then I can create a sample project on my end to try replicate.

Are you not using a CocoaPods dependency configuration?

@matijakregarGH
Copy link
Author

Thanks for helping out!

I am using CocoaPods.

This is my codegen config:

{
  "schemaName" : "MySchemaName",
  "options" : {
    "cocoapodsCompatibleImportStatements" : true
  },
  "input" : {
    "operationSearchPaths" : [
      "**/*.graphql"
    ],
    "schemaSearchPaths" : [
      "**/*.graphqls"
    ]
  },
  "output" : {
    "testMocks" : {
      "none" : {
      }
    },
    "schemaTypes" : {
      "path" : "./ProjectName/Resources/GraphQL/Generated",
      "moduleType": {
        "embeddedInTarget": {
          "name": "TargetName"
        }
      }
    },
    "operations" : {
      "inSchemaModule": {}
    }
  }
}

All the generated objects are "wrapped" in enum MySchemaName.

Hope this helps enough ...

@calvincestari
Copy link
Member

I'll see if I can replicate, thanks for the config details.

@prokhorovxo
Copy link

I have the exact same problem :(
I use SPM, I can share the configuration file if you need it, but it is not much different from the one sent by @matijakregarGH

@calvincestari
Copy link
Member

I think I glossed over this when @matijakregarGH first shared the config but looking again I believe the problem is being caused by the embeddedInTarget(name:) module type. That module type is meant to be used when you don't want any module.

If you're using a dependency manager that is not SwiftPM then you want to use the .other module type - see the documentation here. Those import statements are being generated because the operation files are expecting a module to be available of that name. Note that .other does not automate the creation of the pod, it only places the files in the specified location. You are still required to create the pod yourself.

@calvincestari
Copy link
Member

@prokhorovxo I suggest taking a second look at your configuration, along with the documentation, and verify it's set up for the output your project needs. If after that you still cannot resolve the problem then post it here and we can take a look.

@matijakregarGH
Copy link
Author

@calvincestari I have used the embeddedInTarget(name:) module type to get the generated types wrapped in the schemaName enum. This helps a lot with preventing naming clashes and also generated code decoupling. This feature (to me) is one of the main improvements in v1.0.

Is there any other way to achieve this through the config file?

Now I'm having naming clashes with DocumentType that is defined both in the schema and in Apollo Core.
The solution, as mentioned above, is to specify the module for Apollo DocumentType in generated classes:
public static let document: Apollo.DocumentType = .notPersisted(...

@calvincestari
Copy link
Member

@matijakregarGH - are you able to put together a small sample project demonstrating this? It doesn't need to be your full schema, just make sure the relevant types causing the conflict are there to replicate the build errors.

@AnthonyMDev
Copy link
Contributor

I thought we fixed DocumentType? Can you verify you’re on 1.0.3? This was a known bug in 1.0.0.

@matijakregarGH
Copy link
Author

@AnthonyMDev I'm sure I have 1.0.3. @calvincestari I'll prepare a small sample project. Hopefully I can find the time today.

@matijakregarGH
Copy link
Author

A sample project demonstrating the DocumentType naming conflict in the generated operations file(s).

Pods and generated files are included in the repo. The fix mentioned a few times above solves the build issues.
Please let me know if you need anything else.

@tecnobeto
Copy link

tecnobeto commented Nov 17, 2022

I have exactly the same problem as I wrote it about in the Apollo community:
https://community.apollographql.com/t/conflict-documenttype-in-autogeneration-of-schema-code/5006
Version 1.0.3

@calvincestari
Copy link
Member

OK, I've figured out the issue here and got a PR lined up for the fix. Thanks for the sample repo @matijakregarGH!

@calvincestari calvincestari reopened this Nov 18, 2022
@calvincestari calvincestari added bug Generally incorrect behavior codegen Issues related to or arising from code generation and removed needs investigation labels Nov 18, 2022
@calvincestari
Copy link
Member

OK, the fix for this is merged and will go out in 1.0.4. @matijakregarGH, I've regenerated the code in your sample project and confirmed that it works as expected.

@matijakregarGH
Copy link
Author

@calvincestari Thanks for the fix. I can also confirm that it works with 1.0.4. Cheers 🤟

@prokhorovxo
Copy link

@calvincestari thank you, confirm. 🙏

@tahirmt
Copy link
Contributor

tahirmt commented Dec 7, 2022

I have a conflict in my schema with Selection type name. Is it possible to have codegen automatically prefix Apollo. on all Apollo types in codegen so it doesn't conflict without adding a namespace to all of our generated files? That would cause a huge refactor having to add a namespace on all the types we need.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Generally incorrect behavior codegen Issues related to or arising from code generation
Projects
None yet
6 participants