graphql-code-generator-community icon indicating copy to clipboard operation
graphql-code-generator-community copied to clipboard

Codegen c-sharp generate duplicate class definition on Response

Open eddeee888 opened this issue 1 year ago • 1 comments

Which packages are impacted by your issue?

@graphql-codegen/c-sharp

Describe the bug

Which packages are impacted by your issue?

No response

Describe the bug

When im generated c-sharp types from graphql schema i got multiple class generation for same Response. Exemple, :

public class GearSetupTypeSelection {
        
          [JsonProperty("id")]
          public string id { get; set; }
          
          public class ItemTypeSelection {
          
            [JsonProperty("id")]
            public string id { get; set; }
            
          }
          
          [JsonProperty("primaryWeapon")]
          public ItemTypeSelection primaryWeapon { get; set; }
          
          public class ItemTypeSelection {
          
            [JsonProperty("id")]
            public string id { get; set; }
            
          }
          
          [JsonProperty("secondaryWeapon")]
          public ItemTypeSelection secondaryWeapon { get; set; }
          
          public class ItemTypeSelection {
          
            [JsonProperty("id")]
            public string id { get; set; }
            
          }
          
          [JsonProperty("classItem")]
          public ItemTypeSelection classItem { get; set; }
}

I just want to have one public class ItemTypeSelection { ... } My graphql got fields like : primaryWeapon { ...ItemFields } secondaryWeapon { ...ItemFields} classItem {...ItemFields}

My nodejs codegen config is :

config: {
          namespaceName: 'Eva.GG.Client',
          typesafeOperation: true,
          defaultScalarType: 'object',
        },`

Your Example Website or App

havent

Steps to Reproduce the Bug or Issue

Have multiple identicals fragment on a query or mutation

Expected behavior

Just have one class definition

Screenshots or Videos

No response

Platform

  • OS: macOS
  • NodeJS: 20.0.0
  • graphql version: 12.0.11
  • @graphql-codegen/* version(s): 3.1.0

Codegen Config File

import type { CodegenConfig } from '@graphql-codegen/cli';

const config: CodegenConfig = {
  schema:
    'libs/shared/graphql/util-generator/src/lib/apps/backend/game/schema.graphql',
  documents:
    'libs/shared/graphql/util-generator/src/lib/apps/backend/game/operations/**/*.graphql',
  generates: {
    'libs/shared/graphql/util-generator/src/lib/apps/backend/game/output/Generated-v2.cs':
      {
        plugins: ['c-sharp-operations'],
        config: {
          namespaceName: 'Eva.GG.Client',
          typesafeOperation: true,
          defaultScalarType: 'object',
        },
      },
  },
};
export default config;

Additional context

No response

eddeee888 avatar Nov 23 '24 22:11 eddeee888

Hi @NicolasTarbouriech, I've transferred your issue here as this is the repo for the plugin/s in question

eddeee888 avatar Nov 23 '24 22:11 eddeee888

I have patched our local install to solve this. Note this patch also includes a patch for the enum values to ensure they match the graphql schema.

diff --git a/node_modules/@graphql-codegen/c-sharp-operations/cjs/visitor.js b/node_modules/@graphql-codegen/c-sharp-operations/cjs/visitor.js
index 10f91e5..46de4a2 100644
--- a/node_modules/@graphql-codegen/c-sharp-operations/cjs/visitor.js
+++ b/node_modules/@graphql-codegen/c-sharp-operations/cjs/visitor.js
@@ -216,7 +216,16 @@ class CSharpOperationsVisitor extends visitor_plugin_common_1.ClientSideBaseVisi
                         `public ${responseTypeName} ${propertyName} { get; set; }`,
                     ].join('\n') + '\n');
                 }
-                const selectionBaseTypeName = `${responseType.baseType.type}Selection`;
+                
+                // #105: Use <property_name>Selection instead of <type>Selection so that multiple
+                // properties of the same underlying GQL type at any level does not generate multiple
+                // *Selection classes of the same name at that level. This way we can also avoid keyword
+                // escaping with @ as PascalCasing the typename will ensure we won't make a type name that
+                // is a reserved keyword (no keywords in C# start with a captialized letter)
+                let selectionBaseTypeName = `${node.name.value}Selection`;
+                // Ensure PascalCase
+                selectionBaseTypeName = selectionBaseTypeName.charAt(0).toUpperCase() + selectionBaseTypeName.slice(1);
+
                 const selectionType = Object.assign(new c_sharp_common_1.CSharpFieldType(responseType), {
                     baseType: { type: selectionBaseTypeName },
                 });
@@ -433,7 +442,7 @@ ${this._getOperationMethod(node)}
             .access('public')
             .asKind('enum')
             .withName((0, c_sharp_common_1.convertSafeName)(this.convertName(node.name)))
-            .withBlock((0, visitor_plugin_common_1.indentMultiline)((_a = node.values) === null || _a === void 0 ? void 0 : _a.map(v => this._parsedConfig.memberNamingFunction(v.name.value)).join(',\n'))).string;
+            .withBlock((0, visitor_plugin_common_1.indentMultiline)((_a = node.values) === null || _a === void 0 ? void 0 : _a.map(v => v.name.value).join(',\n'))).string;
         return (0, visitor_plugin_common_1.indentMultiline)(enumDefinition, 2);
     }
 }
diff --git a/node_modules/@graphql-codegen/c-sharp-operations/esm/visitor.js b/node_modules/@graphql-codegen/c-sharp-operations/esm/visitor.js
index 374c0b1..4eff65b 100644
--- a/node_modules/@graphql-codegen/c-sharp-operations/esm/visitor.js
+++ b/node_modules/@graphql-codegen/c-sharp-operations/esm/visitor.js
@@ -1,8 +1,8 @@
-import autoBind from 'auto-bind';
-import { isEnumType, isInputObjectType, isScalarType, Kind, print, visit, } from 'graphql';
 import { C_SHARP_SCALARS, convertSafeName, CSharpDeclarationBlock, CSharpFieldType, getListInnerTypeNode, getListTypeDepth, getListTypeField, getMemberNamingFunction, isValueType, wrapFieldType, } from '@graphql-codegen/c-sharp-common';
 import { getCachedDocumentNodeFromSchema } from '@graphql-codegen/plugin-helpers';
 import { buildScalarsFromConfig, ClientSideBaseVisitor, DocumentMode, getBaseTypeNode, indent, indentMultiline, } from '@graphql-codegen/visitor-plugin-common';
+import autoBind from 'auto-bind';
+import { isEnumType, isInputObjectType, isScalarType, Kind, print, visit, } from 'graphql';
 const defaultSuffix = 'GQL';
 const R_NAME = /name:\s*"([^"]+)"/;
 function R_DEF(directive) {
@@ -212,7 +212,16 @@ export class CSharpOperationsVisitor extends ClientSideBaseVisitor {
                         `public ${responseTypeName} ${propertyName} { get; set; }`,
                     ].join('\n') + '\n');
                 }
-                const selectionBaseTypeName = `${responseType.baseType.type}Selection`;
+                
+                // #105: Use <property_name>Selection instead of <type>Selection so that multiple
+                // properties of the same underlying GQL type at any level does not generate multiple
+                // *Selection classes of the same name at that level. This way we can also avoid keyword
+                // escaping with @ as PascalCasing the typename will ensure we won't make a type name that
+                // is a reserved keyword (no keywords in C# start with a captialized letter)
+                let selectionBaseTypeName = `${node.name.value}Selection`;
+                // Ensure PascalCase
+                selectionBaseTypeName = selectionBaseTypeName.charAt(0).toUpperCase() + selectionBaseTypeName.slice(1);
+
                 const selectionType = Object.assign(new CSharpFieldType(responseType), {
                     baseType: { type: selectionBaseTypeName },
                 });
@@ -429,7 +438,7 @@ ${this._getOperationMethod(node)}
             .access('public')
             .asKind('enum')
             .withName(convertSafeName(this.convertName(node.name)))
-            .withBlock(indentMultiline((_a = node.values) === null || _a === void 0 ? void 0 : _a.map(v => this._parsedConfig.memberNamingFunction(v.name.value)).join(',\n'))).string;
+            .withBlock(indentMultiline((_a = node.values) === null || _a === void 0 ? void 0 : _a.map(v => v.name.value).join(',\n'))).string;
         return indentMultiline(enumDefinition, 2);
     }
 }

ocdi avatar Jul 01 '25 06:07 ocdi

Thanks @ocdi , do you mind sending a PR? 🙂

eddeee888 avatar Jul 05 '25 10:07 eddeee888

@eddeee888 thanks for the prod to create a PR - I went a little ham and created 3, to address issues I'm facing. 😄 The enum values one #1162 was what the other patch above was handling, but hopefully much better/more reliable.

ocdi avatar Jul 06 '25 09:07 ocdi

Thanks @ocdi! these are the alpha versions for the changes:

@graphql-codegen/c-sharp@5.1.2-alpha-20250708131948-f774f9a3830bac8b8e2c04c75b38360a9cef5e91
@graphql-codegen/c-sharp-common@1.2.0-alpha-20250708131948-f774f9a3830bac8b8e2c04c75b38360a9cef5e91
@graphql-codegen/c-sharp-operations@3.2.0-alpha-20250708131948-f774f9a3830bac8b8e2c04c75b38360a9cef5e91

Please let me know if these work, and we can release some time this week 🙏

eddeee888 avatar Jul 08 '25 13:07 eddeee888

Here's the new alpha versions:

@graphql-codegen/c-sharp@5.1.2-alpha-20250711120712-cc88892bd8c50576777af3089da0184b3ea07c17
@graphql-codegen/c-sharp-common@1.2.0-alpha-20250711120712-cc88892bd8c50576777af3089da0184b3ea07c17
@graphql-codegen/c-sharp-operations@3.2.0-alpha-20250711120712-cc88892bd8c50576777af3089da0184b3ea07c17

Please let me know if these work better @ocdi ! These should have the changes from: https://github.com/dotansimha/graphql-code-generator-community/pull/1158

eddeee888 avatar Jul 11 '25 14:07 eddeee888

Thanks! I'll test them over the weekend

ocdi avatar Jul 11 '25 14:07 ocdi

Everything works, however I tried it on a project that didn't have this duplicate fragment issue, therefore was generating with the old class names. I didn't realise how annoying it is to change - I'm wondering if we should bump the c-sharp-operations to a major/breaking change. Another possibility is I do try to preserve the old names, perhaps just keep track and generate class1, 2, 3.

I also should update the pascal case code to properly do that for the classes, I have some queries that were like process_form which created the class Process_formSelection, which is less than ideal.

ocdi avatar Jul 12 '25 04:07 ocdi

I made my own local builds of these and I am really happy with the last PR I just submitted. The project that had this issue is changing a lot but, but I feel for the greater good this is a much better approach. I've now applied this to two of my real projects and the code builds successfully.

ocdi avatar Jul 12 '25 05:07 ocdi

Thank you @ocdi ! Here are the new alpha versions if you want to try it out:

@graphql-codegen/c-sharp@5.1.2-alpha-20250711120712-cc88892bd8c50576777af3089da0184b3ea07c17
@graphql-codegen/c-sharp-common@1.2.0-alpha-20250711120712-cc88892bd8c50576777af3089da0184b3ea07c17
@graphql-codegen/c-sharp-operations@3.2.0-alpha-20250711120712-cc88892bd8c50576777af3089da0184b3ea07c17

eddeee888 avatar Jul 14 '25 12:07 eddeee888

The above comment appears to be the previous release. I looked up on npm the most recent version 3.2.0-alpha-20250714221436-59459010845cfa987870492f930ae2a054f1888e and tested that. For my use cases it all worked as expected.

ocdi avatar Jul 15 '25 01:07 ocdi

Oops sorry about that! These changes are released here: https://github.com/dotansimha/graphql-code-generator-community/releases/tag/release-1752671186180

Thank you for driving this forward @ocdi !

eddeee888 avatar Jul 16 '25 13:07 eddeee888