Simple naming causes duplicate Query/Mutation classes

[akinsho]

Bug description
When running artemis code generation, using the simple naming scheme with two queries in two separate files both returning different data. I get a duplicate class name error for the Query.

[SEVERE] artemis:artemis on lib/$lib$:

Two classes were generated with the same name `ClassName(name:r'Query')`
but with different selection set.

Class A
ClassDefinition(name:ClassName(name:r'Query'), properties:[ClassProperty(type:TypeName(name:r'ClientPage'), name:ClassPropertyName(name:r'clients'), isNonNull:true, isResolveType:false)], factoryPoss
ibilities:{}, typeNameField:TypeName(name:r'__typename'), isInput:false)

Class B
ClassDefinition(name:ClassName(name:r'Query'), properties:[ClassProperty(type:TypeName(name:r'String'), name:ClassPropertyName(name:r'__typename'), annotations:[r'override', r'''JsonKey(name: '__type
name')'''], isNonNull:false, isResolveType:true), ClassProperty(type:TypeName(name:r'ClientEventPage'), name:ClassPropertyName(name:r'clientEvents'), isNonNull:true, isResolveType:false)], factoryPos
sibilities:{}, typeNameField:TypeName(name:r'__typename'), isInput:false)

From what I can see with only one query a class called Query is generated representing the single query I have. Once I have another though this causes a conflict. I understand that the simple naming doesn't do any name spacing and just uses the graphql class name but I would have thought in the case of a query or mutation the name I gave my query would be the class name.

e.g.

# file A
query getUser {
# ....
 }

# file B
query getActivity {
# ...
}

In this case I would have expected the class name generated to be GetActivityQuery and GetUserQuery rather than attempting to create 2 Query classes.

It could be that I've misunderstood how the simple strategy works and that it is only valid if you have one query and one mutation but I guessed not. Anyway thanks for your hard work it's made my development much easier in general 🙌🏿 . If you need any information please let me know, I've only added snippets of the output as it's a private work project 😓 . But I don't think the verbose output shows anything more as this happens regardless of what actual query is

Specs
Artemis version: 6.13.1-beta.1

build.yaml:

# Please paste your `build.yaml` file
      artemis:
        options:
          fragments_glob: lib/graphql/fragments/**.graphql
          schema_mapping:
            - output: lib/graphql/queries.dart
              schema: graphql/schema.graphql
              queries_glob: lib/graphql/queries/**.graphql

Artemis output:

# Please paste the output of
$ flutter pub run build_runner build --verbose
#or
$ pub run build_runner build --verbose

GraphQL schema:

# If possible, please paste your GraphQL schema file,
# or a minimum reproducible schema of the bug.

GraphQL query:

# If possible, please paste your GraphQL query file,
# or a minimum reproducible query of the bug.

[vasilich6107]

Hi. You need to give them different names or use another naming schema

[akinsho]

@vasilich6107 when you say different names do you mean inside the graphql file?
so my queries are structured like

query client { # <-- this name
  client(id: ID!) {
    # ... fields
  }
 }

# file B
query clientDetails { # <-- is different from this
 clientDetails {
   # ... fields
 }
}

Or do you mean different names somewhere else?

[vasilich6107]

For some reason it generates two classes with name:r'Query

[vasilich6107]

Could you fill the issue with all data required for reproduction?

GraphQL schema and GraphQL query

https://tppr.me/KZWEv

[akinsho]

@vasilich6107 I managed to narrow it right down to a super minimal queries/schema which I've posted in the link above. If that isn't clear or doesn't work in that tool I can repost them here since I've stripped out anything sensitive

[vasilich6107]

I’ll check.

[vasilich6107]

Check if this fix works for you

  artemis:
    git:
      url: git://github.com/comigor/artemis.git
      ref: simple-fix

[akinsho]

@vasilich6107 That seems to have worked just rebuilt my project and didn't get the error. It now seems to build separate QueryNameAQuery and a QueryNameBQuery 👍🏿

[vasilich6107]

Awesome, I'll prepare a new release later this week

[akinsho]

@vasilich6107 I think I spoke too soon. I'm no longer getting the name clash and separate classes per query are being created but all the subclasses are named with what looks like the default strategy and quite a few of the simply named types that are being referenced aren't being created

[vasilich6107]

Could you add more information?
What code is generated and what expected?

[vasilich6107]

It will be great if you create some demo repo to showcase your issues. I’ll be able to quickly reproduce and test fixes

[akinsho]

@vasilich6107 I just created https://github.com/akinsho/test_artemis which demonstrates the issue. If you pull this down and build the types, then go to the lib/graphq/queries.graphql.dart you'll see that there are simple type names being used without any implementations of those classes

[vasilich6107]

Thanks
I'll check

[vasilich6107]

@akinsho could you reinstall the simple-fix branch and check again?

[akinsho]

@vasilich6107 I think that's the one 👍🏿 just tried it and it seems to have solved the issues

[vasilich6107]

Some thought about namingSchema
Currently we support 3 naming schemas

• simple
• pathedWithFields
• pathedWithTypes

From our experience only pathedWithFields gives more reliable results in terms of avoiding duplications.
I recommend you to try it. You can achieve the same kind of a name shortening by extracting graphql objects to fragments.

Then you'll be able to use them as [FragmentName]Mixin.

Supporting 3 naming schemas adds more complexity and maintenance overhead.
There could be a chance that we remove support of multiple naming schemas in future.

[akinsho]

@vasilich6107 I've tried both of the alternatives but we have a few complex with a lot of fields and sub types. These queries don't really need fragments (since we won't reuse the selections) and the sort of class names that are being generated are huge.
This makes them quite hard to remember and (very subjective) the code much less readable.

I think it would makes things a bit complex for my colleagues to have a bunch of fragments we only ever use once just so we can generate more readable names. I'm happy to have to alias things if they clash but much prefer the simple naming.

I've used type generation for graphql using gqlgen for golang and apollo with TS and they both seem to create readable names 🤷🏿‍♂️ I don't know much about their internals but maybe there might be an applicable solution there. Anyway the solution you provided in the branch seems to fix things. 🤞🏿 you don't remove the strategy anytime soon 😅

[vasilich6107]

The most interesting part will begin when you'll need 2 types with different selection set ;-)

[vasilich6107]

I assume we will come with some kind of an alternative strategy for simple naming in case we decide to discontinue naming schema support)

[akinsho]

The most interesting part will begin when you'll need 2 types with different selection set ;-)

😅 I'm assuming in this case I would be able to alias the query and that should work. I've had to do a similar thing before when using gqlgen and that was fine, especially since the queries are technically different having a different name makes sense.

I assume we will come with some kind of an alternative strategy for simple naming in case we decide to discontinue naming schema support)

👍🏿 I think having some form of simpler naming (even with trade offs) would be much appreciated by other people

[vasilich6107]

@akinsho how it works?
If it is suitable I will merge and release it.

[akinsho]

@vasilich6107 yeah this resolved the duplicate name issue for me 👍🏿 , I ended up switching naming scheme anyway though since I ended up with another I think unrelated error about missing constructors. Don't remember the exact error atm. But yeah I think this is good to go.

As a side note this is what some of the longer names look like

[vasilich6107]

Extract your selection set into a fragment and you’ll get a [FragmentName]Mixin which can be used for type checking.

Something like

fragment ConsultationCreateEventDetails on ConsultationCreateEventDetails {
... your fields goes here
}

Then you’ll get ConsultationCreateEventDetailsMixin

[alexkharech]

Supporting 3 naming schemas adds more complexity and maintenance overhead.
There could be a chance that we remove support of multiple naming schemas in future.

Which scheme is preferable to rely on so that in the future there will be no problems when updating?

[vasilich6107]

@alexkharech pathedWithFields

[vasilich6107]

Fixed in 6.17