@Skip, @include directives for objects (#5326)

dotansimha · Jan 25, 2021 · 142b32b · 142b32b
1 parent d10803a
commit 142b32b
Show file tree

Hide file tree

Showing 7 changed files with 157 additions and 64 deletions.
diff --git a/.changeset/seven-papayas-sit.md b/.changeset/seven-papayas-sit.md
@@ -0,0 +1,6 @@
+---
+'@graphql-codegen/flow-operations': patch
+'@graphql-codegen/typescript-operations': patch
+---
+
+@skip, @include directives resolve to optional fields
diff --git a/.changeset/thirty-waves-glow.md b/.changeset/thirty-waves-glow.md
@@ -0,0 +1,7 @@
+---
+'@graphql-codegen/flow-operations': patch
+'@graphql-codegen/visitor-plugin-common': patch
+'@graphql-codegen/typescript-operations': patch
+---
+
+Better support for @skip/@include directives with complex selection sets
diff --git a/packages/plugins/flow/operations/src/visitor.ts b/packages/plugins/flow/operations/src/visitor.ts
@@ -66,8 +66,12 @@ export class FlowDocumentsVisitor extends BaseDocumentsVisitor<FlowDocumentsPlug
     const wrapOptional = (type: string) => `?${type}`;
 
     const useFlowReadOnlyTypes = this.config.useFlowReadOnlyTypes;
-    const formatNamedField = (name: string, type: GraphQLOutputType | GraphQLNamedType | null): string => {
-      const optional = !!type && !isNonNullType(type);
+    const formatNamedField = (
+      name: string,
+      type: GraphQLOutputType | GraphQLNamedType | null,
+      isConditional = false
+    ): string => {
+      const optional = (!!type && !isNonNullType(type)) || isConditional;
       return `${useFlowReadOnlyTypes ? '+' : ''}${name}${optional ? '?' : ''}`;
     };
 

diff --git a/packages/plugins/flow/operations/tests/flow-documents.spec.ts b/packages/plugins/flow/operations/tests/flow-documents.spec.ts
@@ -1220,17 +1220,23 @@ describe('Flow Operations Plugin', () => {
 
         type User {
           id: String!
-          name: String
-          address: String!
+          name: String!
+          address: Address!
+        }
+
+        type Address {
+          city: String!
         }
       `);
 
       const ast = parse(/* GraphQL */ `
-        query user($showAddress: Boolean!) {
+        query user($showAddress: Boolean!, $showName: Boolean!) {
           user {
             id
-            name
-            address @include(if: $showAddress)
+            name @include(if: $showName)
+            address @include(if: $showAddress) {
+              city
+            }
           }
         }
       `);
@@ -1251,9 +1257,64 @@ describe('Flow Operations Plugin', () => {
       expect(result).toBeSimilarStringTo(`
       export type UserQueryVariables = {
         showAddress: $ElementType<Scalars, 'Boolean'>,
+        showName: $ElementType<Scalars, 'Boolean'>,
       };
+      export type UserQuery = { user: ({
+            ...$MakeOptional<$Pick<User, { id: *, name: * }>, { name: * }>,
+          ...{ address?: ?$Pick<Address, { city: * }> }
+        }) };
+      `);
+
+      validateFlow(result);
+    });
+
+    it('@skip, @include should resolve to optional on preResolveTypes', async () => {
+      const schema1 = buildSchema(/* GraphQL */ `
+        type Query {
+          user: User!
+        }
+        type User {
+          id: String!
+          name: String!
+          address: Address!
+        }
+        type Address {
+          city: String!
+        }
+      `);
+
+      const ast = parse(/* GraphQL */ `
+        query user($showAddress: Boolean!, $showName: Boolean!) {
+          user {
+            id
+            name @include(if: $showName)
+            address @include(if: $showAddress) {
+              city
+            }
+          }
+        }
+      `);
 
-      export type UserQuery = { user: $MakeOptional<$Pick<User, { id: *, name?: *, address: * }>, { address: * }> };
+      const result = mergeOutputs([
+        await plugin(
+          schema1,
+          [
+            {
+              location: '',
+              document: ast,
+            },
+          ],
+          { preResolveTypes: true },
+          { outputFile: '' }
+        ),
+      ]);
+
+      expect(result).toBeSimilarStringTo(`
+      export type UserQueryVariables = {
+        showAddress: $ElementType<Scalars, 'Boolean'>,
+        showName: $ElementType<Scalars, 'Boolean'>,
+      };
+      export type UserQuery = { __typename?: 'Query', user: { __typename?: 'User', id: string, name?: ?string, address?: ?{ __typename?: 'Address', city: string } } };
       `);
 
       validateFlow(result);

diff --git a/packages/plugins/other/visitor-plugin-common/src/selection-set-to-object.ts b/packages/plugins/other/visitor-plugin-common/src/selection-set-to-object.ts
@@ -372,14 +372,15 @@ export class SelectionSetToObject<Config extends ParsedDocumentsConfig = ParsedD
     for (const { field, selectedFieldType } of linkFieldSelectionSets.values()) {
       const realSelectedFieldType = getBaseType(selectedFieldType as any);
       const selectionSet = this.createNext(realSelectedFieldType, field.selectionSet);
+      const isConditional = hasConditionalDirectives(field.directives);
 
       linkFields.push({
         alias: field.alias ? this._processor.config.formatNamedField(field.alias.value, selectedFieldType) : undefined,
-        name: this._processor.config.formatNamedField(field.name.value, selectedFieldType),
+        name: this._processor.config.formatNamedField(field.name.value, selectedFieldType, isConditional),
         type: realSelectedFieldType.name,
         selectionSet: this._processor.config.wrapTypeWithModifiers(
           selectionSet.transformSelectionSet().split(`\n`).join(`\n  `),
-          selectedFieldType
+          isConditional ? realSelectedFieldType : selectedFieldType
         ),
       });
     }

diff --git a/packages/plugins/typescript/operations/src/visitor.ts b/packages/plugins/typescript/operations/src/visitor.ts
@@ -51,8 +51,12 @@ export class TypeScriptDocumentsVisitor extends BaseDocumentsVisitor<
       return `${listModifier}<${type}>`;
     };
 
-    const formatNamedField = (name: string, type: GraphQLOutputType | GraphQLNamedType | null): string => {
-      const optional = !this.config.avoidOptionals.field && !!type && !isNonNullType(type);
+    const formatNamedField = (
+      name: string,
+      type: GraphQLOutputType | GraphQLNamedType | null,
+      isConditional = false
+    ): string => {
+      const optional = isConditional || (!this.config.avoidOptionals.field && !!type && !isNonNullType(type));
       return (this.config.immutableTypes ? `readonly ${name}` : name) + (optional ? '?' : '');
     };
 

diff --git a/packages/plugins/typescript/operations/tests/ts-documents.spec.ts b/packages/plugins/typescript/operations/tests/ts-documents.spec.ts
@@ -4813,56 +4813,7 @@ function test(q: GetEntityBrandDataQuery): void {
       export type UserQuery = { __typename?: 'Query', user: { __typename?: 'User', name: string, address?: Maybe<string> } };`);
     });
 
-    // it('objects with @skip, @include should pre resolve into optional', async () => {
-    //   const schema = buildSchema(/* GraphQL */ `
-    //     type Query {
-    //       user: User!
-    //     }
-
-    //     type User {
-    //       id: String!
-    //       name: String!
-    //       address: Address!
-    //     }
-
-    //     type Address {
-    //       city: String!
-    //     }
-    //   `);
-
-    //   const fragment = parse(/* GraphQL */ `
-    //     query user($showAddress: Boolean!, $showName: Boolean!) {
-    //       user {
-    //         id
-    //         name @include(if: $showName)
-    //         address @include(if: $showAddress) {
-    //           city
-    //         }
-    //       }
-    //     }
-    //   `);
-
-    //   const { content } = await plugin(
-    //     schema,
-    //     [{ location: '', document: fragment }],
-    //     {
-    //       preResolveTypes: true,
-    //     },
-    //     {
-    //       outputFile: 'graphql.ts',
-    //     }
-    //   );
-
-    //   expect(content).toBeSimilarStringTo(`
-    //   export type UserQueryVariables = Exact<{
-    //     showAddress: Scalars['Boolean'];
-    //     showName: Scalars['Boolean'];
-    //   }>;
-
-    //   export type UserQuery = { __typename?: 'Query', user: { __typename?: 'User', id: string, name?: Maybe<string>, address?: Maybe<{ __typename?: 'Address', city: string}> } };`);
-    // });
-
-    it('fileds with @skip, @include should make container resolve into MakeOptional type', async () => {
+    it('objects with @skip, @include should pre resolve into optional', async () => {
       const schema = buildSchema(/* GraphQL */ `
         type Query {
           user: User!
@@ -4871,14 +4822,68 @@ function test(q: GetEntityBrandDataQuery): void {
         type User {
           id: String!
           name: String!
+          address: Address!
+        }
+
+        type Address {
+          city: String!
         }
       `);
 
       const fragment = parse(/* GraphQL */ `
-        query user($showName: Boolean!) {
+        query user($showAddress: Boolean!, $showName: Boolean!) {
           user {
             id
             name @include(if: $showName)
+            address @include(if: $showAddress) {
+              city
+            }
+          }
+        }
+      `);
+
+      const { content } = await plugin(
+        schema,
+        [{ location: '', document: fragment }],
+        {
+          preResolveTypes: true,
+        },
+        {
+          outputFile: 'graphql.ts',
+        }
+      );
+
+      expect(content).toBeSimilarStringTo(`
+      export type UserQueryVariables = Exact<{
+        showAddress: Scalars['Boolean'];
+        showName: Scalars['Boolean'];
+      }>;
+      export type UserQuery = { __typename?: 'Query', user: { __typename?: 'User', id: string, name?: Maybe<string>, address?: Maybe<{ __typename?: 'Address', city: string }> } };`);
+    });
+
+    it('fields with @skip, @include should make container resolve into MakeOptional type', async () => {
+      const schema = buildSchema(/* GraphQL */ `
+        type Query {
+          user: User!
+        }
+        type User {
+          id: String!
+          name: String!
+          address: Address!
+        }
+        type Address {
+          city: String!
+        }
+      `);
+
+      const fragment = parse(/* GraphQL */ `
+        query user($showAddress: Boolean!, $showName: Boolean!) {
+          user {
+            id
+            name @include(if: $showName)
+            address @include(if: $showAddress) {
+              city
+            }
           }
         }
       `);
@@ -4894,6 +4899,7 @@ function test(q: GetEntityBrandDataQuery): void {
 
       expect(content).toBeSimilarStringTo(`
       export type UserQueryVariables = Exact<{
+        showAddress: Scalars['Boolean'];
         showName: Scalars['Boolean'];
       }>;
   
@@ -4902,7 +4908,11 @@ function test(q: GetEntityBrandDataQuery): void {
         { __typename?: 'Query' }
         & { user: (
           { __typename?: 'User' }
-          & MakeOptional<Pick<User, 'id' | 'name'>, 'name'>
+          & MakeOptional<Pick<User, 'id' | 'name'>, 'name'>      
+          & { address?: Maybe<(
+            { __typename?: 'Address' }
+            & Pick<Address, 'city'>
+          )> }
         ) }
       );`);
     });