Avoid optionals on fields influences variables

[simPod]

Describe the bug

Even though avoidOptionals.inputValue is false, variable types are generated without optionals.

To Reproduce

1. My GraphQL schema:

type Query {
    a(filter: [A!]): String
}

2. My GraphQL operations:

query a($filter: [A!]) {
    a(filter: $filter)
}

3. My codegen.yml config file:

avoidOptionals:
  field: true
  object: true

export type AQueryVariables = Exact<{
  filter: Maybe<Array<A>>;
}>;

https://codesandbox.io/s/stupefied-paper-vjrm1?file=/types.ts

Expected behavior

It's still part of my InputType so this is expected

export type AQueryVariables = Exact<{
  filter?: Maybe<Array<A>>;
}>;

Environment:

• OS:
  "@graphql-codegen/add": "^2.0.1",
  "@graphql-codegen/cli": "^1.17.8",
  "@graphql-codegen/fragment-matcher": "^1.17.8",
  "@graphql-codegen/typescript": "^1.17.9",
  "@graphql-codegen/typescript-operations": "^1.17.8",
  "@graphql-codegen/typescript-react-apollo": "^2.0.6",
• NodeJS: 14

[simPod]

@dotansimha where this could be possible fixed? Can look at it but have no idea to start honestly.

[Rio6]

Looks like it's the problem with typescript-resolvers plugin. It has this function:

    formatRootResolver(schemaTypeName, resolverType, declarationKind) {
        return `${schemaTypeName}${this.config.avoidOptionals ? '' : '?'}: ${resolverType}${this.getPunctuation(declarationKind)}`;
    }

Which assumes avoidOptionals to be boolean type only. I made this simple patch to test it

diff --git a/node_modules/@graphql-codegen/typescript-resolvers/index.cjs.js b/node_modules/@graphql-codegen/typescript-resolvers/index.cjs.js
index a08c96d..60fb56c 100644
--- a/node_modules/@graphql-codegen/typescript-resolvers/index.cjs.js
+++ b/node_modules/@graphql-codegen/typescript-resolvers/index.cjs.js
@@ -38,7 +38,7 @@ class TypeScriptResolversVisitor extends visitorPluginCommon.BaseResolversVisito
         return `ParentType extends ${parentType} = ${parentType}`;
     }
     formatRootResolver(schemaTypeName, resolverType, declarationKind) {
-        return `${schemaTypeName}${this.config.avoidOptionals ? '' : '?'}: ${resolverType}${this.getPunctuation(declarationKind)}`;
+        return `${schemaTypeName}${this.config.avoidOptionals && this.config.avoidOptionals.resolver ? '' : '?'}: ${resolverType}${this.getPunctuation(declarationKind)}`;
     }
     clearOptional(str) {
         if (str.startsWith('Maybe')) {

And it looks like that was it.

EDIT: nevermind, I was having a different issue than yours.

[simPod]

So the code that controls the behaviour starts here:

graphql-code-generator/packages/plugins/other/visitor-plugin-common/src/variables-to-object.ts

Lines 82 to 85 in f0b5ea5

	const hasDefaultValue = variable.defaultValue != null && typeof variable.defaultValue !== 'undefined';
	const isNonNullType = variable.type.kind === Kind.NON_NULL_TYPE;
	const formattedFieldString = this.formatFieldString(fieldName, isNonNullType, hasDefaultValue);

Not sure if it needs some change but adding a variable default value helps in my case

query a($filter: [A!] = null) {
    a(filter: $filter)
}

https://codesandbox.io/s/pensive-shaw-fw7tf?file=/types.ts

[Urigo]

I'm not adding a lot here but I'm just labeling it according to our new Contribution Guide and issue flow.

It looks like you already made it into stage 1 thanks to your reproduction, thank you for that!

Would be great if someone could help progress the issues through the stages

In order to move it to the next step, do you think you could create a failing test with the same configuration?
Also check why there isn't already one.
Probably check the tests here: https://github.com/dotansimha/graphql-code-generator/blob/f0b5ea53af313e95d59cf2b0e01ee6b048fc141a/packages/plugins/typescript/operations/tests/ts-documents.spec.ts

Thank you and sorry that this comment is not a complete solution (yet)

[simPod]

@Urigo thanks. I'might have the test in my local history.

But I wonder if it's something that should be fixed. Like I don't have such knowledge of GQL spec so I can tell.

I can achieve the proper behaviour by adding a default value. Should input fields be marked as optional if they have no default value but are nullable? If so, I'll provide the test.

[neiker]

In my case the solution was to set object to false.

  avoidOptionals: 
    field: true 
    object: false
    inputValue: false
    defaultValue: false

[laurisvan]

I think we've got plenty of data here, but just wanted to confirm that input types incorrectly get their optionality removed, even with the following settings:

      avoidOptionals:
        field: true
        inputValue: false
        object: false
        defaultValue: false

 export type AcceptFeedback = {
   __typename?: 'AcceptFeedback';
-  note?: Maybe<Scalars['String']>;
-  npsScore?: Maybe<Scalars['Int']>;
+  note: Maybe<Scalars['String']>;
+  npsScore: Maybe<Scalars['Int']>;
 };
 
 export type AcceptFeedbackInput = {
@@ -56,16 +56,16 @@ export type AcceptFeedbackInput = {
 export type Account = {
   __typename?: 'Account';
   createdAt: Scalars['DateTime'];
-  deletedAt?: Maybe<Scalars['DateTime']>;
+  deletedAt: Maybe<Scalars['DateTime']>;
   id: Scalars['ID'];
   /** Login username */
   login: Scalars['String'];
   /** Phone number for sms login */
-  phone?: Maybe<Scalars['String']>;
+  phone: Maybe<Scalars['String']>;
   /** Generated, long-lived JWT access token for refreshing the actual token */
-  refreshToken?: Maybe<Scalars['String']>;
+  refreshToken: Maybe<Scalars['String']>;
   /** Generated JWT access token - not part of the data model */
-  token?: Maybe<Scalars['String']>;
+  token: Maybe<Scalars['String']>;
   updatedAt: Scalars['DateTime'];
   user: User;
 };

This feels like an issue that I would like to try fixing. I have not contributed to this project yet, so @dotansimha @charlypoly please hint me if this is an issue that is beyond a "simple first issue" to work on for a person that does know JavaScript and large projects fairly well but does not have a clue on the internals of this project.