Ensure @external does not generate resolver types

- Handle external directive when part or whole type is marked
- Add changeset
- Add test cases for @provides and @external

Author: eddeee888

.changeset/twenty-planets-complain.md

@@ -0,0 +1,7 @@
+---
+'@graphql-codegen/visitor-plugin-common': patch
+'@graphql-codegen/typescript-resolvers': patch
+'@graphql-codegen/plugin-helpers': patch
+---
+
+Fix fields or object types marked with @external being wrongly generated

packages/plugins/other/visitor-plugin-common/src/base-resolvers-visitor.ts

@@ -1707,7 +1707,11 @@ export class BaseResolversVisitor<
     return `Partial<${argsType}>`;
   }
 
-  ObjectTypeDefinition(node: ObjectTypeDefinitionNode): string {
+  ObjectTypeDefinition(node: ObjectTypeDefinitionNode): string | null {
+    if (this._federation.skipObjectType({ node })) {
+      return null;
+    }
+
     const declarationKind = 'type';
     const name = this.convertName(node, {
       suffix: this.config.resolverTypeSuffix,
@@ -1728,15 +1732,17 @@ export class BaseResolversVisitor<
       return false;
     })();
 
-    const fieldsContent = (node.fields as unknown as FieldDefinitionPrintFn[]).map(f => {
-      return f(
-        typeName,
-        (rootType === 'query' && this.config.avoidOptionals.query) ||
-          (rootType === 'mutation' && this.config.avoidOptionals.mutation) ||
-          (rootType === 'subscription' && this.config.avoidOptionals.subscription) ||
-          (rootType === false && this.config.avoidOptionals.resolvers)
-      ).value;
-    });
+    const fieldsContent = (node.fields as unknown as FieldDefinitionPrintFn[])
+      .map(f => {
+        return f(
+          typeName,
+          (rootType === 'query' && this.config.avoidOptionals.query) ||
+            (rootType === 'mutation' && this.config.avoidOptionals.mutation) ||
+            (rootType === 'subscription' && this.config.avoidOptionals.subscription) ||
+            (rootType === false && this.config.avoidOptionals.resolvers)
+        ).value;
+      })
+      .filter(v => v);
 
     if (!rootType && this._parsedSchemaMeta.typesWithIsTypeOf[typeName]) {
       fieldsContent.push(
@@ -1748,6 +1754,10 @@ export class BaseResolversVisitor<
       );
     }
 
+    if (fieldsContent.length === 0) {
+      return null;
+    }
+
     const genericTypes: string[] = [
       `ContextType = ${this.config.contextType.type}`,
       this.transformParentGenericType(parentType),

packages/plugins/typescript/resolvers/tests/ts-resolvers.federation.spec.ts

@@ -496,20 +496,46 @@ describe('TypeScript Resolvers Plugin + Apollo Federation', () => {
     `);
   });
 
-  it('should skip to generate resolvers of fields with @external directive', async () => {
+  it('should skip to generate resolvers of fields or object types with @external directive', async () => {
     const federatedSchema = /* GraphQL */ `
       type Query {
         users: [User]
       }
 
       type Book {
         author: User @provides(fields: "name")
+        editor: User @provides(fields: "company { taxCode }")
       }
 
       type User @key(fields: "id") {
         id: ID!
         name: String @external
         username: String @external
+        address: Address
+        dateOfBirth: DateOfBirth
+        placeOfBirth: PlaceOfBirth
+        company: Company
+      }
+
+      type Address {
+        street: String! @external
+        zip: String!
+      }
+
+      type DateOfBirth {
+        day: Int! @external
+        month: Int! @external
+        year: Int! @external
+      }
+
+      type PlaceOfBirth @external {
+        city: String!
+        country: String!
+      }
+
+      type Company @external {
+        name: String!
+        taxCode: String!
       }
     `;
 
@@ -520,7 +546,8 @@ describe('TypeScript Resolvers Plugin + Apollo Federation', () => {
       },
     });
 
-    // UserResolver should not have a resolver function of name field
+    // UserResolvers should not have `username` resolver because it is marked with `@external`
+    // UserResolvers should have `name` resolver because whilst it is marked with `@external`, it is provided by `Book.author`
     expect(content).toBeSimilarStringTo(`
       export type UserResolvers<ContextType = any, ParentType extends ResolversParentTypes['User'] = ResolversParentTypes['User'], FederationType extends FederationTypes['User'] = FederationTypes['User']> = {
         __resolveReference?: ReferenceResolver<Maybe<ResolversTypes['User']>,
@@ -530,6 +557,26 @@ describe('TypeScript Resolvers Plugin + Apollo Federation', () => {
         name?: Resolver<Maybe<ResolversTypes['String']>, ParentType, ContextType>;
       };
     `);
+
+    // AddressResolvers should only have fields not marked with @external
+    expect(content).toBeSimilarStringTo(`
+      export type AddressResolvers<ContextType = any, ParentType extends ResolversParentTypes['Address'] = ResolversParentTypes['Address']> = {
+        zip?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
+      };
+    `);
+
+    // CompanyResolvers should only have taxCode resolver because it is part of the `@provides` directive in `Book.editor`
+    expect(content).toBeSimilarStringTo(`
+      export type CompanyResolvers<ContextType = any, ParentType extends ResolversParentTypes['Company'] = ResolversParentTypes['Company']> = {
+        taxCode?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
+      };
+    `);
+
+    // DateOfBirthResolvers should not be generated because there is no field not marked with @external
+    expect(content).not.toBeSimilarStringTo('export type DateOfBirthResolvers');
+
+    // PlaceOfBirthResolvers should not be generated because the type is marked with @external
+    expect(content).not.toBeSimilarStringTo('export type PlaceOfBirthResolvers');
   });
 
   it('should not include _FieldSet scalar', async () => {

packages/utils/plugins-helpers/src/federation.ts

@@ -265,16 +265,20 @@ export class ApolloFederation {
     return this.enabled && name === '_FieldSet';
   }
 
+  skipObjectType({ node }: { node: ObjectTypeDefinitionNode }): boolean {
+    if (!this.enabled) {
+      return false;
+    }
+
+    return this.isExternal(node);
+  }
+
   /**
    * Decides if field should not be generated
    * @param data
    */
   skipField({ fieldNode, parentType }: { fieldNode: FieldDefinitionNode; parentType: GraphQLNamedType }): boolean {
-    if (
-      !this.enabled ||
-      !(isObjectType(parentType) && !isInterfaceType(parentType)) ||
-      !checkTypeFederationDetails(parentType, this.schema)
-    ) {
+    if (!this.enabled || !(isObjectType(parentType) && !isInterfaceType(parentType))) {
       return false;
     }
 
@@ -349,7 +353,7 @@ export class ApolloFederation {
     return this.isExternal(fieldNode) && !this.hasProvides(objectType, fieldNode);
   }
 
-  private isExternal(node: FieldDefinitionNode): boolean {
+  private isExternal(node: FieldDefinitionNode | ObjectTypeDefinitionNode): boolean {
     return getDirectivesByName('external', node).length > 0;
   }
 
@@ -481,7 +485,14 @@ function getDirectivesByName(
     astNode = node;
   }
 
-  return astNode?.directives?.filter(d => d.name.value === name) || [];
+  return (
+    astNode?.directives?.filter(d => {
+      // A ObjectTypeDefinitionNode's directive looks like `{ kind: 'Directive', name: 'external', arguments: [] }`
+      // However, other directives looks like `{ kind: 'Directive', name: { kind: 'Name', value: 'external' }, arguments: [] }`
+      // Therefore, we need to check for both `d.name.value` and d.name
+      return d.name.value === name || (d.name as unknown as string) === name;
+    }) || []
+  );
 }
 
 function extractReferenceSelectionSet(directive: DirectiveNode): ReferenceSelectionSet {