diff --git a/.changeset/twenty-planets-complain.md b/.changeset/twenty-planets-complain.md new file mode 100644 index 00000000000..4d9a34fa45c --- /dev/null +++ b/.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 diff --git a/packages/plugins/other/visitor-plugin-common/src/base-resolvers-visitor.ts b/packages/plugins/other/visitor-plugin-common/src/base-resolvers-visitor.ts index d44f422058c..cf842c12435 100644 --- a/packages/plugins/other/visitor-plugin-common/src/base-resolvers-visitor.ts +++ b/packages/plugins/other/visitor-plugin-common/src/base-resolvers-visitor.ts @@ -83,9 +83,14 @@ export interface ParsedResolversConfig extends ParsedConfig { } type FieldDefinitionPrintFn = ( - parentName: string, + parent: { + node: ObjectTypeDefinitionNode | InterfaceTypeDefinitionNode; + typeName: string; + }, avoidResolverOptionals: boolean ) => { value: string | null; meta: { federation?: { isResolveReference: boolean } } }; +export type FieldDefinitionResult = [{ node: FieldDefinitionNode }, FieldDefinitionPrintFn]; + export interface RootResolver { content: string; generatedResolverTypes: { @@ -1519,123 +1524,130 @@ export class BaseResolversVisitor< return `ParentType extends ${parentType} = ${parentType}`; } - FieldDefinition(node: FieldDefinitionNode, key: string | number, parent: any): FieldDefinitionPrintFn { + FieldDefinition(node: FieldDefinitionNode, key: string | number, parent: any): FieldDefinitionResult { const hasArguments = node.arguments && node.arguments.length > 0; const declarationKind = 'type'; - return (parentName, avoidResolverOptionals) => { - const original: FieldDefinitionNode = parent[key]; - const parentType = this.schema.getType(parentName); - const meta: ReturnType['meta'] = {}; - - if (this._federation.skipField({ fieldNode: original, parentType })) { - return { value: null, meta }; - } + return [ + { node }, + (parentNode, avoidResolverOptionals) => { + const parentName = parentNode.typeName; + + const original: FieldDefinitionNode = parent[key]; + const parentType = this.schema.getType(parentName); + const meta: ReturnType['meta'] = {}; + + const fieldsToGenerate = this._federation.findFieldNodesToGenerate({ node: parentNode.node }); + const shouldGenerateField = fieldsToGenerate.some(field => field.name === node.name); + if (!shouldGenerateField) { + return { value: null, meta }; + } - const contextType = this.getContextType(parentName, node); - - let argsType = hasArguments - ? this.convertName( - parentName + - (this.config.addUnderscoreToArgsType ? '_' : '') + - this.convertName(node.name, { - useTypesPrefix: false, - useTypesSuffix: false, - }) + - 'Args', - { - useTypesPrefix: true, - }, - true - ) - : null; + const contextType = this.getContextType(parentName, node); + + let argsType = hasArguments + ? this.convertName( + parentName + + (this.config.addUnderscoreToArgsType ? '_' : '') + + this.convertName(node.name, { + useTypesPrefix: false, + useTypesSuffix: false, + }) + + 'Args', + { + useTypesPrefix: true, + }, + true + ) + : null; + + const avoidInputsOptionals = this.config.avoidOptionals.inputValue; + + if (argsType !== null) { + const argsToForceRequire = original.arguments.filter( + arg => !!arg.defaultValue || arg.type.kind === 'NonNullType' + ); + + if (argsToForceRequire.length > 0) { + argsType = this.applyRequireFields(argsType, argsToForceRequire); + } else if (original.arguments.length > 0 && avoidInputsOptionals !== true) { + argsType = this.applyOptionalFields(argsType, original.arguments); + } + } - const avoidInputsOptionals = this.config.avoidOptionals.inputValue; + const parentTypeSignature = this._federation.transformFieldParentType({ + fieldNode: original, + parentType, + parentTypeSignature: this.getParentTypeForSignature(node), + federationTypeSignature: 'FederationType', + }); - if (argsType !== null) { - const argsToForceRequire = original.arguments.filter( - arg => !!arg.defaultValue || arg.type.kind === 'NonNullType' - ); + const { mappedTypeKey, resolverType } = ((): { mappedTypeKey: string; resolverType: string } => { + const baseType = getBaseTypeNode(original.type); + const realType = baseType.name.value; + const typeToUse = this.getTypeToUse(realType); + /** + * Turns GraphQL type to TypeScript types (`mappedType`) e.g. + * - String! -> ResolversTypes['String']> + * - String -> Maybe + * - [String] -> Maybe>> + * - [String!]! -> Array + */ + const mappedType = this._variablesTransformer.wrapAstTypeWithModifiers(typeToUse, original.type); + + const subscriptionType = this._schema.getSubscriptionType(); + const isSubscriptionType = subscriptionType && subscriptionType.name === parentName; + + if (isSubscriptionType) { + return { + mappedTypeKey: `${mappedType}, "${node.name}"`, + resolverType: 'SubscriptionResolver', + }; + } - if (argsToForceRequire.length > 0) { - argsType = this.applyRequireFields(argsType, argsToForceRequire); - } else if (original.arguments.length > 0 && avoidInputsOptionals !== true) { - argsType = this.applyOptionalFields(argsType, original.arguments); - } - } + const directiveMappings = + node.directives + ?.map(directive => this._directiveResolverMappings[directive.name as any]) + .filter(Boolean) + .reverse() ?? []; - const parentTypeSignature = this._federation.transformFieldParentType({ - fieldNode: original, - parentType, - parentTypeSignature: this.getParentTypeForSignature(node), - federationTypeSignature: 'FederationType', - }); - - const { mappedTypeKey, resolverType } = ((): { mappedTypeKey: string; resolverType: string } => { - const baseType = getBaseTypeNode(original.type); - const realType = baseType.name.value; - const typeToUse = this.getTypeToUse(realType); - /** - * Turns GraphQL type to TypeScript types (`mappedType`) e.g. - * - String! -> ResolversTypes['String']> - * - String -> Maybe - * - [String] -> Maybe>> - * - [String!]! -> Array - */ - const mappedType = this._variablesTransformer.wrapAstTypeWithModifiers(typeToUse, original.type); - - const subscriptionType = this._schema.getSubscriptionType(); - const isSubscriptionType = subscriptionType && subscriptionType.name === parentName; - - if (isSubscriptionType) { return { - mappedTypeKey: `${mappedType}, "${node.name}"`, - resolverType: 'SubscriptionResolver', + mappedTypeKey: mappedType, + resolverType: directiveMappings[0] ?? 'Resolver', }; - } + })(); + + const signature: { + name: string; + modifier: string; + type: string; + genericTypes: string[]; + } = { + name: node.name as any, + modifier: avoidResolverOptionals ? '' : '?', + type: resolverType, + genericTypes: [mappedTypeKey, parentTypeSignature, contextType, argsType].filter(f => f), + }; - const directiveMappings = - node.directives - ?.map(directive => this._directiveResolverMappings[directive.name as any]) - .filter(Boolean) - .reverse() ?? []; + if (this._federation.isResolveReferenceField(node)) { + if (!this._federation.getMeta()[parentType.name].hasResolveReference) { + return { value: '', meta }; + } + signature.type = 'ReferenceResolver'; + signature.genericTypes = [mappedTypeKey, parentTypeSignature, contextType]; + meta.federation = { isResolveReference: true }; + } return { - mappedTypeKey: mappedType, - resolverType: directiveMappings[0] ?? 'Resolver', + value: indent( + `${signature.name}${signature.modifier}: ${signature.type}<${signature.genericTypes.join( + ', ' + )}>${this.getPunctuation(declarationKind)}` + ), + meta, }; - })(); - - const signature: { - name: string; - modifier: string; - type: string; - genericTypes: string[]; - } = { - name: node.name as any, - modifier: avoidResolverOptionals ? '' : '?', - type: resolverType, - genericTypes: [mappedTypeKey, parentTypeSignature, contextType, argsType].filter(f => f), - }; - - if (this._federation.isResolveReferenceField(node)) { - if (!this._federation.getMeta()[parentType.name].hasResolveReference) { - return { value: '', meta }; - } - signature.type = 'ReferenceResolver'; - signature.genericTypes = [mappedTypeKey, parentTypeSignature, contextType]; - meta.federation = { isResolveReference: true }; - } - - return { - value: indent( - `${signature.name}${signature.modifier}: ${signature.type}<${signature.genericTypes.join( - ', ' - )}>${this.getPunctuation(declarationKind)}` - ), - meta, - }; - }; + }, + ]; } private getFieldContextType(parentName: string, node: FieldDefinitionNode): string { @@ -1707,7 +1719,12 @@ export class BaseResolversVisitor< return `Partial<${argsType}>`; } - ObjectTypeDefinition(node: ObjectTypeDefinitionNode): string { + ObjectTypeDefinition(node: ObjectTypeDefinitionNode): string | null { + const fieldsToGenerate = this._federation.findFieldNodesToGenerate({ node }); + if (fieldsToGenerate.length === 0) { + return null; + } + const declarationKind = 'type'; const name = this.convertName(node, { suffix: this.config.resolverTypeSuffix, @@ -1728,15 +1745,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 FieldDefinitionResult[]) + .map(([_, f]) => { + return f( + { node, 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 +1767,10 @@ export class BaseResolversVisitor< ); } + if (fieldsContent.length === 0) { + return null; + } + const genericTypes: string[] = [ `ContextType = ${this.config.contextType.type}`, this.transformParentGenericType(parentType), @@ -1958,8 +1981,8 @@ export class BaseResolversVisitor< // An Interface in Federation may have the additional __resolveReference resolver, if resolvable. // So, we filter out the normal fields declared on the Interface and add the __resolveReference resolver. - const fields = (node.fields as unknown as FieldDefinitionPrintFn[]).map(f => - f(typeName, this.config.avoidOptionals.resolvers) + const fields = (node.fields as unknown as FieldDefinitionResult[]).map(([_, f]) => + f({ node, typeName }, this.config.avoidOptionals.resolvers) ); for (const field of fields) { if (field.meta.federation?.isResolveReference) { diff --git a/packages/plugins/typescript/resolvers/tests/ts-resolvers.federation.spec.ts b/packages/plugins/typescript/resolvers/tests/ts-resolvers.federation.spec.ts index 57f1c665bec..b749a97db14 100644 --- a/packages/plugins/typescript/resolvers/tests/ts-resolvers.federation.spec.ts +++ b/packages/plugins/typescript/resolvers/tests/ts-resolvers.federation.spec.ts @@ -496,7 +496,7 @@ 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] @@ -504,12 +504,38 @@ describe('TypeScript Resolvers Plugin + Apollo Federation', () => { 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 = { __resolveReference?: ReferenceResolver, @@ -528,8 +555,32 @@ describe('TypeScript Resolvers Plugin + Apollo Federation', () => { & GraphQLRecursivePick ), ContextType>; id?: Resolver; name?: Resolver, ParentType, ContextType>; + address?: Resolver, ParentType, ContextType>; + dateOfBirth?: Resolver, ParentType, ContextType>; + placeOfBirth?: Resolver, ParentType, ContextType>; + company?: Resolver, ParentType, ContextType>; + }; + `); + + // AddressResolvers should only have fields not marked with @external + expect(content).toBeSimilarStringTo(` + export type AddressResolvers = { + zip?: Resolver; }; `); + + // FIXME: CompanyResolvers should only have taxCode resolver because it is part of the `@provides` directive in `Book.editor` + // expect(content).toBeSimilarStringTo(` + // export type CompanyResolvers = { + // taxCode?: Resolver; + // }; + // `); + + // DateOfBirthResolvers should not be generated because every field is 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 () => { diff --git a/packages/utils/plugins-helpers/src/federation.ts b/packages/utils/plugins-helpers/src/federation.ts index 4c6ce6ee5ef..60a9ddd4f81 100644 --- a/packages/utils/plugins-helpers/src/federation.ts +++ b/packages/utils/plugins-helpers/src/federation.ts @@ -1,4 +1,5 @@ import { astFromInterfaceType, astFromObjectType, getRootTypeNames, MapperKind, mapSchema } from '@graphql-tools/utils'; +import type { FieldDefinitionResult } from '@graphql-codegen/visitor-plugin-common'; import { DefinitionNode, DirectiveNode, @@ -8,6 +9,7 @@ import { GraphQLNamedType, GraphQLObjectType, GraphQLSchema, + InterfaceTypeDefinitionNode, isInterfaceType, isObjectType, ObjectTypeDefinitionNode, @@ -266,19 +268,58 @@ export class ApolloFederation { } /** - * Decides if field should not be generated - * @param data + * findFieldNodesToGenerate + * @description Function to find field nodes to generate. + * In a normal setup, all fields must be generated. + * However, in a Federatin setup, a field should not be generated if: + * - The field is marked as `@external` and there is no `@provides` path to the field + * - The parent object is marked as `@external` and there is no `@provides` path to the field */ - skipField({ fieldNode, parentType }: { fieldNode: FieldDefinitionNode; parentType: GraphQLNamedType }): boolean { - if ( - !this.enabled || - !(isObjectType(parentType) && !isInterfaceType(parentType)) || - !checkTypeFederationDetails(parentType, this.schema) - ) { - return false; + findFieldNodesToGenerate({ + node, + }: { + node: ObjectTypeDefinitionNode | InterfaceTypeDefinitionNode; + }): readonly FieldDefinitionNode[] { + if (!this.enabled) { + return node.fields || []; + } + + const parentType = this.schema.getType(node.name as any as string); + if (!(isObjectType(parentType) && !isInterfaceType(parentType))) { + return []; + } + + const fieldNodes = node.fields as unknown as FieldDefinitionResult[]; + + // If the object is marked with @external, fields to generate are those with `@provides` + if (this.isExternal(node)) { + const fieldNodesWithProvides = fieldNodes.reduce((acc, [fieldDef]) => { + if (this.hasProvides(parentType, fieldDef.node.name as any as string)) { + acc.push(fieldDef.node); + } + return acc; + }, []); + return fieldNodesWithProvides; } - return this.isExternalAndNotProvided(fieldNode, parentType); + // If the object is not marked with @external, fields to generate are: + // - the fields without `@external` + // - the `@external` fields with `@provides` + const fieldNodesWithoutExternalOrHasProvides = fieldNodes.reduce((acc, [fieldDef]) => { + if (!this.isExternal(fieldDef.node)) { + acc.push(fieldDef.node); + return acc; + } + + if (this.isExternal(fieldDef.node) && this.hasProvides(parentType, fieldDef.node.name as any as string)) { + acc.push(fieldDef.node); + return acc; + } + + return acc; + }, []); + + return fieldNodesWithoutExternalOrHasProvides; } isResolveReferenceField(fieldNode: FieldDefinitionNode): boolean { @@ -345,19 +386,15 @@ export class ApolloFederation { return this.meta; } - private isExternalAndNotProvided(fieldNode: FieldDefinitionNode, objectType: GraphQLObjectType): boolean { - return this.isExternal(fieldNode) && !this.hasProvides(objectType, fieldNode); - } - - private isExternal(node: FieldDefinitionNode): boolean { + private isExternal(node: FieldDefinitionNode | ObjectTypeDefinitionNode | InterfaceTypeDefinitionNode): boolean { return getDirectivesByName('external', node).length > 0; } - private hasProvides(objectType: ObjectTypeDefinitionNode | GraphQLObjectType, node: FieldDefinitionNode): boolean { + private hasProvides(objectType: ObjectTypeDefinitionNode | GraphQLObjectType, fieldName: string): boolean { const fields = this.providesMap[isObjectType(objectType) ? objectType.name : objectType.name.value]; if (fields?.length) { - return fields.includes(node.name.value); + return fields.includes(fieldName); } return false; @@ -410,7 +447,7 @@ export class ApolloFederation { for (const field of Object.values(objectType.getFields())) { const provides = getDirectivesByName('provides', field.astNode) .map(extractReferenceSelectionSet) - .reduce((prev, curr) => [...prev, ...Object.keys(curr)], []); + .reduce((prev, curr) => [...prev, ...Object.keys(curr)], []); // FIXME: this is not taking into account nested selection sets e.g. `company { taxCode }` const ofType = getBaseType(field.type); providesMap[ofType.name] ||= []; @@ -471,17 +508,24 @@ function checkTypeFederationDetails( */ function getDirectivesByName( name: string, - node: ObjectTypeDefinitionNode | GraphQLObjectType | FieldDefinitionNode + node: ObjectTypeDefinitionNode | GraphQLObjectType | FieldDefinitionNode | InterfaceTypeDefinitionNode ): readonly DirectiveNode[] { - let astNode: ObjectTypeDefinitionNode | FieldDefinitionNode; + let astNode: ObjectTypeDefinitionNode | FieldDefinitionNode | InterfaceTypeDefinitionNode; - if (isObjectType(node)) { + if (isObjectType(node) || isInterfaceType(node)) { astNode = node.astNode; } else { 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 {