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

CodegenCLI - File name collision if Fragment has the same name as Type #2598

Closed
p4checo opened this issue Oct 21, 2022 · 16 comments
Closed

CodegenCLI - File name collision if Fragment has the same name as Type #2598

p4checo opened this issue Oct 21, 2022 · 16 comments
Labels
bug Generally incorrect behavior codegen Issues related to or arising from code generation

Comments

@p4checo
Copy link
Contributor

p4checo commented Oct 21, 2022

Bug report

In our setup we define fragments with the same name as the schema types, e.g.:

fragment ProductImage on ProductImage {
    extraLarge
    large
    medium
    small
}

While this wasn't an issue with the previous single-file codegen, in the current multi-file codegen of Apollo 1.0.x this is causing an issue because two files with the same ProductImage.graphql.swift name are generated (one in /Schema/Objects and another in /Fragments). This causes compilation fail with an error like this:

error: filename "ProductImage.graphql.swift" used twice: '(...)/GraphQL/Generated/Schema/Objects/ProductImage.graphql.swift' and '(...)/GraphQL/Generated/Fragments/ProductImage.graphql.swift'
note: filenames are used to distinguish private declarations with the same name

The generated types themselves are properly namespaced so they don't collide, it's only the filenames themselves cause the problem:

Fragment

public extension GraphQL {
  struct ProductImage: GraphQL.SelectionSet, Fragment {
(...)

Object

public extension GraphQL.Objects {
  static let ProductImage = Object(
    typename: "ProductImage",
    implementedInterfaces: []
  )
}

While we can rename all our fragments to not have the same name as the type, we feel that this could also be fixed at the codegen phase, by adding a Fragment suffix to the file (e.g. Foo.Fragment.graphql.swift) instead of relying only on the folder structure (which can possibly cause other similar collisions). Would it make sense? Could also be added as a parameter of the config.

Versions

Please fill in the versions you're currently using:

  • apollo-ios SDK version: 1.0.1
  • Xcode version: 14.0 (14A309)
  • Swift version: 5.7
  • Package manager: 5.7

Steps to reproduce

  • Create a fragment with the same name as the type
  • Run codegen
@calvincestari
Copy link
Member

Thanks for the bug report @p4checo. I'd prefer not to add yet another configuration option, there are already too many. I'd rather opt for a consistent "Fragment" suffix on the filename. I'll see if there are any other options.

@calvincestari calvincestari added bug Generally incorrect behavior codegen Issues related to or arising from code generation labels Oct 24, 2022
@calvincestari calvincestari added this to the Release 1.0.3 milestone Oct 24, 2022
@SzymonMatysik
Copy link

Hey,
I've also run into this issue. Is "Fragment" suffix solution available in configuration?
Cheers

@calvincestari
Copy link
Member

@SzymonMatysik, no fix available yet. The work is slated for the 1.1 release, we're currently working on issues for the next patch release - 1.0.6.

@jostster
Copy link

I am also having this issue after migrating to the new codegen cli now that we switched from cocoapods to SPM. Is there any work around so we don't have to rename 50+ files after generating them?

@calvincestari
Copy link
Member

calvincestari commented Jun 1, 2023

Hi @jostster - the only options currently are:

  1. rename the fragments, I realize this may not always be possible in larger orgs where the named fragments may not be under your direct control.
  2. rename the generated files, since all named fragments are generated into a single folder within the schema module output location a script to rename every file in that folder may ease the burden and become something that could be automated.

@jostster
Copy link

jostster commented Jun 1, 2023

Yea for now I created a bash script to generate then rename all the files. Is there a flag or option to have all the generated files be combined into a single file? Since they are generated and shouldn't be edited this could also ease the burden and help larger projects not have a bunch of different generated files.

@calvincestari
Copy link
Member

Is there a flag or option to have all the generated files be combined into a single file?

No, the legacy (0.x) versions used to have that option but it was removed in 1.0 to support modular architectures. It was also a poor performance option re. Xcode when it was an unwieldily large file.

@AnthonyMDev
Copy link
Contributor

This will be fixed by #3283.

@AnthonyMDev AnthonyMDev removed their assignment May 17, 2024
@pm-dev
Copy link

pm-dev commented Jan 7, 2025

Any update here?

@calvincestari
Copy link
Member

@pm-dev, the schema type renaming feature was released in 1.13.0. That will allow you to rename the conflicting type and in turn have it use a different file name.

Copy link
Contributor

github-actions bot commented Jan 7, 2025

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.

@pm-dev
Copy link

pm-dev commented Jan 7, 2025

@calvincestari custom naming doesn't solve this issue. When fragments are named the same thing as the type, the file names conflict. Resolving this via custom naming means manually adding an entry for each fragment, which at that point might as well just change the name of the fragment in the .graphql file. So it's not really a solution. I suggest this issue is reopened, unless you want to explicitly forbid fragments to have the same name as the type, which seems silly for the client to make such an opinionated rule.

@calvincestari
Copy link
Member

@calvincestari custom naming doesn't solve this issue. When fragments are named the same thing as the type, the file names conflict.

Renaming the type should result in a different file name. If that's not happening it's probably a bug and worthy of creating an issue. Ideally Xcode would understand they're at a different path and this problem wouldn't exist.

@pm-dev
Copy link

pm-dev commented Jan 7, 2025

@calvincestari sorry if I was unclear. Forcing users to rename is not a solution to this issue.

@AnthonyMDev
Copy link
Contributor

We didn't implement this because it wasn't going to be a good experience for people who aren't running into this problem. If you have a ProductDetails fragment that doesn't have a filename conflict because the type is just Product, you don't actually want it to be named ProductDetailsFragment. More verbosity in adding Fragment to the end of every fragment is not a desired behavior in 99% of situations.

"Forcing users to rename their fragments" only happens during the initial migration to 1.0. If you were creating a new project from scratch, you would encounter this issue once and just name your fragments differently going forward. This is one of the many pain points with the 1.0 migration. But we didn't want to add a "feature" that actually isn't desired behavior usually forever to slight ease the migration path here.

@pm-dev
Copy link

pm-dev commented Jan 9, 2025

If you have a ProductDetails fragment that doesn't have a filename conflict because the type is just Product, you don't actually want it to be named ProductDetailsFragment

There's no need to rename the fragment, just the filename. The original proposal here was to name fragment files ProductDetails.Fragment.graphql.swift but the fragment inside would still be named ProductDetails. Seems simple and harmless and would avoid this issue but lmk if I'm missing something.

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
Development

No branches or pull requests

7 participants