Naming collision with fragments and scalars.

[scottasoutherland]

Bug report

In our schema, we have a scalar named Text which is mapped to String.

We also have a fragment that looks something like

fragment event on SomeEvent { 
     items {
        title
        text { 
            ... on Type1 {
                ...field1
            } 
            ... on Type2 {
                ..field2
            }
        } 
     }
}

where text is a field with a Union Type of Type1 & Type2 and title is of the scalar type Text.

In the generated code, we're getting both title and text as type Text. And this is because there is a type added on the fragment of Text for the type of object that the field text is. If you make your FileOutput.operations be absolute, the scalar is namespaced as SchemaName.Text and then it works correctly, however that is not an option for us as it's the incorrect setup for us. We are able to work around this issue by renaming the field from text to something else in the fragment.

Versions

Please fill in the versions you're currently using:

• apollo-ios SDK version: 1.0.3
• Xcode version: 14.1
• Swift version: 5.7
• Package manager: cocoapods 1.11.3

Steps to reproduce

Create a schema with a type Text. Create a union type. Use the union type as a field named text. create a fragment that uses both the scalar type and a the field named text. Generate the code using SchemaTypesFileOutput.moduleType.embeddedInTarget, and FileOutput.operations.inSchemaModule

Further details

As mentioned, you can workaround this by renaming the field of the Union type.

[calvincestari]

Thanks for creating the issue @scottasoutherland.

As mentioned, you can workaround this by renaming the field of the Union type.

Did you use a Field Alias for this or did you actually change the schema

In designing the new code generation engine we opted to place responsibility for these kinds of cases into the hands of the engineer, where it was possible, i.e.: using a Field Alias is possible since you craft the queries and fragments but changing a schema is likely not possible in many cases.

I'll see if there is anything we can do re. the existing namespaces to help but it may come down to needing to use a Field Alias to resolve. I understand that is a tedious task if there are many places that would need an alias. As the code generation engine evolves though we're looking to add features such as field policies that would make something like that a much easier 'global' change. Just some insight into what our thoughts were and what we're thinking of for future code generation.

[scottasoutherland]

I used a field alias

[scottasoutherland]

The reason i'd say this is a bug is this is not a collision with two different types defined in the schema, its a collision between a schema type name and a generated type name which we have no control over. The generation just happens to use the name of the field.

[Austinpayne]

I recently hit this as well. Here is a minimum reproducer that exhibits the namespace collision: https://github.com/Austinpayne/apollo-ios-reproducers/tree/bug/colliding-types

[calvincestari]

The reason i'd say this is a bug is this is not a collision with two different types defined in the schema, its a collision between a schema type name and a generated type name which we have no control over. The generation just happens to use the name of the field.

You control the name of the generated type through the field name; granted that's an implementation detail most people should never have to know but most projects also don't encounter this problem. The ability to use a Field Alias aligns with our design decision to make engineers responsible for resolving this instead of additional complexity in the code generation engine. We're happy to revisit any design decision if there is enough community feedback.

[calvincestari]

Thanks for the sample project @Austinpayne.

I'm still going to look for another way we can resolve this without having to change the generated type names.

[PatrickDanino]

Just wanted to follow up on this. The examples/discussion seems to have focused on generated types, but the issue correctly points out this also applies to scalars.

I don't see a way to work around that. For example, if you have a scalar for URL, the generator is creating a type alias of type String that conflicts with Foundation's URL type. This means that any code which references your model will then have to be changed to special-case URL, which in our case is hundreds if not thousands of places only for this one type.

I don't see the reason why these type aliases are needed at all. Sure, they might make your code look more like the original schema, but having a type alias for URL here is superfluous and if anything, makes moving to the new code generator a huge pain.

[calvincestari]

Thanks @PatrickDanino, is https://community.apollographql.com/t/ios-swift-codegen-scalars-conflicting-with-foundation-types/5181 from you too? We can track progress in this issue rather than the community post.

[PatrickDanino]

Yes - the more I looked into this, the more I came to conclude this wasn't something that could easily be worked around.

[vapor-pawelw]

Having the same problem as @calvincestari even since 0.x when I actually used custom scalars with the codegen CLI flag & then I had my own scripts to rename them in the generated files. This was a huge headache though and now this became a problem again and the structure is different. It would be great to get some option to map the GraphQL custom scalar type names when generating code to something that wouldn't collide with Foundation types

[AnthonyMDev]

I'm going to make all references to custom scalars in the generated models use the schema namespace to avoid the conflict (Not just when FileOutput.operations is .absolute).

@vapor-pawelw Will this resolve your primary issue as well?

It would be great to get some option to map the GraphQL custom scalar type names when generating code to something that wouldn't collide with Foundation types

I agree, this is something we want to add in the future, just got a lot of features on our Roadmap right now!

@PatrickDanino I feel like I'm not 100% grasping what problem you are having. Have you read the documentation on Custom Scalars we have?

Particularly, the example of UUID would help with your URL issue. You can just change the typealias URL = String to typealias URL = Foundation.URL and then make Foundation.URL conform to the CustomScalarType protocol. At that point, there should be no conflict, as they are the same type.

That might clear up some things for you. If you are still unhappy with how this is working, we should discuss it more.

[vapor-pawelw]

I'm going to make all references to custom scalars in the generated models use the schema namespace to avoid the conflict (Not just when FileOutput.operations is .absolute).

@vapor-pawelw Will this resolve your primary issue as well?

It would be great to get some option to map the GraphQL custom scalar type names when generating code to something that wouldn't collide with Foundation types

I agree, this is something we want to add in the future, just got a lot of features on our Roadmap right now!

@PatrickDanino I feel like I'm not 100% grasping what problem you are having. Have you read the documentation on Custom Scalars we have?

Particularly, the example of UUID would help with your URL issue. You can just change the typealias URL = String to typealias URL = Foundation.URL and then make Foundation.URL conform to the CustomScalarType protocol. At that point, there should be no conflict, as they are the same type.

That might clear up some things for you. If you are still unhappy with how this is working, we should discuss it more.

I've ended up putting the schema in a swift package so it's working fine, the collision problem is now a non issue as it only occurs when importing the schema package so I just need to specify Foundation.Date explicitly to not collide with Date custom scalar. It would probably be a problem if I tried to put the generated code in executable target as I did with 0.x versions, because I would have the collisions in all files belonging to the target.

Thank you for the assistance though, and would be great to see scalar name mapping in the future :)

[PatrickDanino]

I'm going to make all references to custom scalars in the generated models use the schema namespace to avoid the conflict (Not just when FileOutput.operations is .absolute).

@vapor-pawelw Will this resolve your primary issue as well?

It would be great to get some option to map the GraphQL custom scalar type names when generating code to something that wouldn't collide with Foundation types

I agree, this is something we want to add in the future, just got a lot of features on our Roadmap right now!

@PatrickDanino I feel like I'm not 100% grasping what problem you are having. Have you read the documentation on Custom Scalars we have?

Particularly, the example of UUID would help with your URL issue. You can just change the typealias URL = String to typealias URL = Foundation.URL and then make Foundation.URL conform to the CustomScalarType protocol. At that point, there should be no conflict, as they are the same type.

That might clear up some things for you. If you are still unhappy with how this is working, we should discuss it more.

Before proceeding, I want to clarify the premise of my question after comparing what the docs claim should be generated with what I actually see.

The docs in your response above show something like this:

public extension MySchema {
  typealias URL = String
}

What I actually see get generated in v1.0.5 is this, emphasizing the lack of namespace:

import ApolloAPI

/// The type to represent a URL for some resource. Format is as follows:
///
/// scheme:[userinfo@]host[:port]path[?query][#fragment]
///
/// https://en.wikipedia.org/wiki/URL
public typealias URL = String

So it would seem the crux of the issue I am seeing, in addition to the lack of using fully qualified type references, is the additional lack of namespace.

[AnthonyMDev]

@PatrickDanino, that means you are generating your schema types with a configuration that expects you to put them in a separate module, rather than embedding them directly in your application target. In this case, we don't use the namespace extension because the namespace is the name of your schema module. But that shouldn't really matter here, because they are namespaced inside your schema module.

I see from your previous post (before you edited it), that you had this example problem:

# Contains Foundation.URL
import Foundation

# Contains `typealias URL = Foundation.String`
import MyApolloGenerated

class Test {
  # This will generate a compiler error
  let url: URL
}

In that case, yes you would have an ambiguous type lookup, because our schema module typealias is pointing to string, so you do have two different distinct types.

If, in your schema module, you changed the typealias to point to Foundation.URL, then your code will compile just fine though, as the typealias is pointing to the same type. I've tested this and the Swift compiler is smart enough to understand that there is no type ambiguity here.

You should really consider converting your custom scalars from the raw String into the URL type and changing your typealias here.

For the case where you don't want to use Foundation.URL, we do have plans to allow the client to override the generated names of custom scalars from your schema in the future. It's just not something we have gotten around to yet.

[dafurman]

For the case where you don't want to use Foundation.URL, we do have plans to allow the client to override the generated names of custom scalars from your schema in the future. It's just not something we have gotten around to yet.

@AnthonyMDev Sorry for reopening conversation in an old issue, but I just stumbled upon this while looking to see if there was any way I could do exactly what you're discussing here. This would be a wonderful addition! Is there an open issue / milestone where this is being tracked, so I can reference it in my code?

Edit: Never mind, I worked out my particular issue with typealiasing, though I am admittedly still curious about the potential of renaming custom scalars. Might be a nice alternative for some more complex use cases.