Skip to content

Allow generation of for case‐sensitive types #1308

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

Closed
yeikel opened this issue Aug 16, 2023 · 4 comments
Closed

Allow generation of for case‐sensitive types #1308

yeikel opened this issue Aug 16, 2023 · 4 comments

Comments

@yeikel
Copy link
Contributor

yeikel commented Aug 16, 2023

Is your feature request related to a problem? Please describe.

This is a follow up for #651

As per the spec :

Names
Name
/[_A-Za-z][_0-9A-Za-z]*/
GraphQL Documents are full of named things: operations, fields, arguments, types, directives, fragments, and variables. All names must follow the same grammatical form.

Names in GraphQL are case‐sensitive. That is to say name, Name, and NAME all refer to different names. Underscores are significant, which means other_name and othername are two different names.

Names in GraphQL are limited to this ASCII subset of possible characters to support interoperation with as many other systems as possible.

While I agree with this comment :

#651 (comment)

The Graphql spec allows it and in my humble opinion the plugin should follow the spec

Additionally, as this is allowed, it is seen in the wild and changing existing production schemas is not always possible

Currently, what the plugin produces is this :

Caused by: java.nio.file.FileAlreadyExistsException: File already exists: /target/generated-sources/graphql/model/Hierarchy.java
at com.kobylynskyi.graphql.codegen.generators.FreeMarkerTemplateFilesCreator.create (FreeMarkerTemplateFilesCreator.java:44)
at com.kobylynskyi.graphql.codegen.generators.impl.InputGenerator.generate (InputGenerator.java:42)
at com.kobylynskyi.graphql.codegen.generators.impl.InputGenerator.generate (InputGenerator.java:34)
at com.kobylynskyi.graphql.codegen.GraphQLCodegen.processDefinitions (GraphQLCodegen.java:175)
at com.kobylynskyi.graphql.codegen.GraphQLCodegen.generate (GraphQLCodegen.java:151)
at io.github.kobylynskyi.graphql.codegen.GraphQLCodegenMojo.execute (GraphQLCodegenMojo.java:325)

Example type definition to replicate the problem :

type Hierarchy {

}

type hierarchy {

}

Describe the solution you'd like

As this is mostly limited by case sensitive filesystems, we can add an option to customize the package of a given type

For example, in addition to packageName, we can have

<customTypePackage>
<hierarchy>my.custom.package</hierarchy>
</customTypePackage>

Describe alternatives you've considered

A combination of #1307 #1297 might work where the user will use #1307 to define a custom type for one the duplicates in a different package and them use #1297 to skip the generation

@yeikel yeikel changed the title Allow generation of for case sensitive types Allow generation of for case‐sensitive types Aug 16, 2023
@kobylynskyi
Copy link
Owner

What you can do today: split types with the same name into different .graphqls files and create separate build configurations with different output folders. In that way your file system will not complain about duplicate files.

@yeikel
Copy link
Contributor Author

yeikel commented Aug 16, 2023

What you can do today: split types with the same name into different .graphqls files and create separate build configurations with different output folders. In that way your file system will not complain about duplicate files.

Sadly, I cannot do this because I also need the full graphql schema in one go

@kobylynskyi
Copy link
Owner

Ok.
But introducing a custom package logic for specific types isn't a good idea either, because it will be very complicated to manage imports of inter-dependent types that have different packages.
So I am closing the ticket as "Won't Fix".

@kobylynskyi kobylynskyi closed this as not planned Won't fix, can't repro, duplicate, stale Aug 16, 2023
@yeikel
Copy link
Contributor Author

yeikel commented Aug 16, 2023

Ok.
But introducing a custom package logic for specific types isn't a good idea either, because it will be very complicated to manage imports of inter-dependent types that have different packages.
So I am closing the ticket as "Won't Fix".

I understand

My argument still stands because this is supported by the spec, but it's understandable if you aren't looking to support everything that the spec supports

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants