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

convertDefinition failing when name == concatenated namePrefix #305

Open
Ynng opened this issue Jan 26, 2024 · 1 comment
Open

convertDefinition failing when name == concatenated namePrefix #305

Ynng opened this issue Jan 26, 2024 · 1 comment
Labels
bug Something isn't working
Milestone

Comments

@Ynng
Copy link

Ynng commented Jan 26, 2024

Describe the bug

I am encountering "conflicting definition" errors when using specific GraphQL field and type names with genqlient. The error message is as follows:

...../schema.graphql:13: conflicting definition for FooBarA; this can indicate either a genqlient internal error, a conflict between user-specified type-names, or some very tricksy GraphQL field/type names: expected 2 fields, got 1
exit status 1
main.go:9: running "go": exit status 1

To Reproduce
The issue occurs under specific conditions:

  1. The schema includes a union whose name is the concatenation of a parent type's name and its field names in camel case.
  2. The query includes fragments named identically to the types:

For a clearer understanding and a practical demonstration, I prepared a minimal reproducible example here: https://github.com/Ynng/genqlient-bug

Expected behavior
I expect the code generation process to successfully handle these scenarios without encountering conflicting definition errors.

genqlient version
v0.6.0

Additional Information

My guess is that the issue arises when convertDefinition() encounters a name that is the same as namePrefix list concatenated and formatted.

This causes problems down the road for Unions, leading to duplicate "UnionNameImplName" definitions.
I think the issue can be traced back to these two lines:

sharedFields, err := g.convertSelectionSet(

implTyp, err := g.convertDefinition(

These lines inadvertently create two conflicting definitions: one from "UnionName + ImplName" and another from "Union + Name + ImplName."

@Ynng Ynng added the bug Something isn't working label Jan 26, 2024
@benjaminjkraft
Copy link
Collaborator

benjaminjkraft commented Jan 26, 2024

Wow, good find. That sure is a way to construct a collision without any type names that are too weird! I think your analysis of the name is approximately right, but maybe a simpler way to see it is simply that FooBar is both fragment FooBar and the field bar on fragment Foo. (The fragment-interface itself should conflict too, I think we just happen to hit the implementation types first.)

You should be able to work around this with a typename directive. If I remember correctly you want to put that on Foo.bar and then it will become the prefix for all the union members from that branch. (Or maybe it works on the fragment FooBar as well, I forget. If not that's something we could add -- the only reason we might not have is because there was no obvious use.)

Ideally we could avoid this entirely, but I don't see an obvious way short of making basically all our type names, or at least all our fragment type names, uglier/longer. (Kinda a breaking change too.) Perhaps it's time for the collision-detector to add a suffix, although we'd have to make sure it's stable in its ordering. Suggestions welcome!

@benjaminjkraft benjaminjkraft added this to the v1.0 milestone Jan 26, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants