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

Make Codegen Namespacing Optional #3164

Closed
Mordil opened this issue Jul 31, 2023 · 15 comments
Closed

Make Codegen Namespacing Optional #3164

Mordil opened this issue Jul 31, 2023 · 15 comments
Labels
feature New addition or enhancement to existing solutions

Comments

@Mordil
Copy link
Contributor

Mordil commented Jul 31, 2023

Use case

The Codegen API tries to support many architecture use cases, and in some it's possible to have name collisions if the generated code happens to be in the same module.

This is resolved by wrapping the code in a namespace enum, which is configurable.

However, this is completely unnecessary in some architectures, where all the generated code is encapsulated in a single Swift module - the Swift module acts as the namespace.

This is particularly painful for projects that are upgrading from the previous codegen to the new, where thousands of lines of code are needing to be updated to append the namespace, with no other changes to the type reference.

Describe the solution you'd like

In the Codegen API, namespace should be entirely optional.

@Mordil Mordil added the feature New addition or enhancement to existing solutions label Jul 31, 2023
@calvincestari
Copy link
Member

Hi @Mordil, this is already available with the other module type. I've attached a sample project that demonstrates the configuration.

Archive.zip

@Mordil
Copy link
Contributor Author

Mordil commented Aug 1, 2023

@calvincestari Then documentation needs to be updated as this is not clearly explained and actually seems counter-intuitive from the symbol docs on the enum options.

The other type mentions that schemaNamespace is required and that the option is more for things like Cocoapods (when I'm using a SwiftPM target to host this code)

and embeddedInTarget mentions manually including files into the project

@calvincestari
Copy link
Member

The other type mentions that schemaNamespace is required

schemaNamespace is always required as part of the configuration since we use it to fully qualify the referenced type names.

and that the option is more for things like Cocoapods (when I'm using a SwiftPM target to host this code)

Yes I can see how mention of CocoaPods specifically confuses the issue. I can clean that up.

and embeddedInTarget mentions manually including files into the project

I'll include details on this for other too.

@Mordil
Copy link
Contributor Author

Mordil commented Aug 1, 2023

What about for the use case where I want all of my schema types to be defined in one module, but all of my operations are defined in downstream modules, but I don't want to namespace the schema types in the sole module?

What configuration combination should I use?

- schema module
  - operationSubsetModule
  - operationSubsetModule

@Mordil
Copy link
Contributor Author

Mordil commented Aug 1, 2023

For context in this other use case,

we want a top level schema module that has all of the "currency" types of the schema, as public, and then we want each submodule to encapsulate the operations as internal, only exposing the currency types to our app that imports these operation modules.

What way can we setup the configuration to have the submodules import the schema module, and control over their access modifiers?

@calvincestari
Copy link
Member

I think you should still be able to use a schema module type of other, eliminating the enum namespace. Combined with an operations output of relative(subpath:accessModifier:), which would require you to place the operation definitions in each of the downstream modules.

we want a top level schema module that has all of the "currency" types of the schema, as public, and then we want each submodule to encapsulate the operations as internal, only exposing the currency types to our app that imports these operation modules.

I think the above configuration should satisfy that.

What way can we setup the configuration to have the submodules import the schema module, and control over their access modifiers?

1.2.0 introduced the accessModifier property to all relevant output configurations. I believe for swiftPackageManager and other all schema types are public.

@Mordil
Copy link
Contributor Author

Mordil commented Aug 1, 2023

Thank you, this helps immensely simplify the upgrade path we were facing

@calvincestari
Copy link
Member

Glad to help. The configuration is quite flexible but difficult to fully explain it's capabilities in documentation without concrete examples that could never cover every possible configuration.

@guillianbalisi
Copy link

In the Archive.zip example, where does the CodegenNamespacingOptional type come from?

@calvincestari
Copy link
Member

calvincestari commented Aug 1, 2023

In the Archive.zip example, where does the CodegenNamespacingOptional type come from?

It's the name of the project the files get added to, or the name of the SPM package, or the name of the pod.

@Mordil
Copy link
Contributor Author

Mordil commented Aug 1, 2023

What about for the use case where I want all of my schema types to be defined in one module, but all of my operations are defined in downstream modules, but I don't want to namespace the schema types in the sole module?

What configuration combination should I use?

- schema module
  - operationSubsetModule
  - operationSubsetModule

I've noticed with this configuration there's a bug that stops it from compiling - when generating the schema types for the submodules it's adding the namespace to the SchemaMetadata.graphql.swift file - but it shouldn't because that file exists in the module that shares the same name as the "namespace", causing compilation to fail

@guillianbalisi
Copy link

It's the name of the project the files get added to, or the name of the SPM package, or the name of the pod.

If I am trying to integrate Apollo into a project that uses its own build system that doesn't necessarily fit into those 3 things, trying to specify this namespace doesn't work. It will produce an error cannot find type 'MyNamespace' in scope. To get it working I have to manually delete the use of MyNamespace, e.g. MyNamespace.SelectionSet -> SelectionSet. Making codegen namespacing optional would let me avoid that.

@Mordil
Copy link
Contributor Author

Mordil commented Aug 1, 2023

What about for the use case where I want all of my schema types to be defined in one module, but all of my operations are defined in downstream modules, but I don't want to namespace the schema types in the sole module?
What configuration combination should I use?

- schema module
  - operationSubsetModule
  - operationSubsetModule

I've noticed with this configuration there's a bug that stops it from compiling - when generating the schema types for the submodules it's adding the namespace to the SchemaMetadata.graphql.swift file - but it shouldn't because that file exists in the module that shares the same name as the "namespace", causing compilation to fail

Never mind on this... after spending more time debugging, I had realized this schema module still contained the autogenerated struct that re-uses the module's name.

@calvincestari
Copy link
Member

calvincestari commented Aug 1, 2023

If I am trying to integrate Apollo into a project that uses its own build system that doesn't necessarily fit into those 3 things, trying to specify this namespace doesn't work.

@guillianbalisi we try to be flexible as possible but there will be bounds to what we have built so far. Can you share a few more details about the custom build system and what it creates in terms of modules, etc.?

@guillianbalisi
Copy link

Using Bazel with bazel-ios rules (apple_framework) to define modules, and building modules using bazel build MyModule. Unfortunately I don't have a simple reproducible example, but I recognize that this is a pretty unordinary use case.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New addition or enhancement to existing solutions
Projects
None yet
Development

No branches or pull requests

3 participants