From 18a679babe76a18ce81c108f39e4010e53deb0ce Mon Sep 17 00:00:00 2001 From: Eddy Nguyen Date: Sun, 25 Aug 2024 23:12:20 +1000 Subject: [PATCH] Use _federation to keep track of its meta --- .changeset/fifty-dodos-marry.md | 6 +- .../src/base-resolvers-visitor.ts | 56 ++++--- .../other/visitor-plugin-common/src/types.ts | 5 + .../tests/ts-resolvers.federation.spec.ts | 156 ++++++++++++++++-- .../utils/plugins-helpers/src/federation.ts | 39 +++-- 5 files changed, 214 insertions(+), 48 deletions(-) diff --git a/.changeset/fifty-dodos-marry.md b/.changeset/fifty-dodos-marry.md index c92959bae84..aec4dc48134 100644 --- a/.changeset/fifty-dodos-marry.md +++ b/.changeset/fifty-dodos-marry.md @@ -4,4 +4,8 @@ '@graphql-codegen/plugin-helpers': minor --- -Avoid generating reference resolvers if federation object is not resolvable +Add `generateInternalResolversIfNeeded` option + +This option can be used to generate more correct types for internal resolvers. For example, only generate `__resolveReference` if the federation object has a resolvable `@key`. + +In the future, this option can be extended to support other internal resolvers e.g. `__isTypeOf` is only generated for implementing types and union members. 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 c7118cb3c21..1c7ab1697db 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 @@ -33,6 +33,8 @@ import { ConvertOptions, DeclarationKind, EnumValuesMap, + type NormalizedGenerateInternalResolversIfNeededConfig, + type GenerateInternalResolversIfNeededConfig, NormalizedAvoidOptionalsConfig, NormalizedScalarsMap, ParsedEnumValuesMap, @@ -75,6 +77,7 @@ export interface ParsedResolversConfig extends ParsedConfig { resolverTypeSuffix: string; allResolversTypeName: string; internalResolversPrefix: string; + generateInternalResolversIfNeeded: NormalizedGenerateInternalResolversIfNeededConfig; onlyResolveTypeForInterfaces: boolean; directiveResolverMappings: Record; resolversNonOptionalTypename: ResolversNonOptionalTypenameConfig; @@ -577,6 +580,16 @@ export interface RawResolversConfig extends RawConfig { * If you are using `mercurius-js`, please set this field to empty string for better compatibility. */ internalResolversPrefix?: string; + /** + * @type object + * @default { __resolveReference: false } + * @description If relevant internal resolvers are set to `true`, the resolver type will only be generated if the right conditions are met. + * Enabling this allows a more correct type generation for the resolvers. + * For example: + * - `__isTypeOf` is generated for implementing types and union members + * - `__resolveReference` is generated for federation types that have at least one resolvable `@key` directive + */ + generateInternalResolversIfNeeded?: GenerateInternalResolversIfNeededConfig; /** * @type boolean * @default false @@ -652,7 +665,6 @@ export class BaseResolversVisitor< [key: string]: { typename: string; baseGeneratedTypename?: string; - federation?: { hasResolveReference: boolean }; }; } = {}; protected _collectedDirectiveResolvers: { [key: string]: string } = {}; @@ -669,7 +681,6 @@ export class BaseResolversVisitor< protected _globalDeclarations = new Set(); protected _federation: ApolloFederation; protected _hasScalars = false; - protected _hasFederation = false; protected _fieldContextTypeMap: FieldContextTypeMap; protected _directiveContextTypesMap: FieldContextTypeMap; protected _checkedTypesWithNestedAbstractTypes: Record = {}; @@ -709,6 +720,9 @@ export class BaseResolversVisitor< mappers: transformMappers(rawConfig.mappers || {}, rawConfig.mapperTypeSuffix), scalars: buildScalarsFromConfig(_schema, rawConfig, defaultScalars), internalResolversPrefix: getConfigValue(rawConfig.internalResolversPrefix, '__'), + generateInternalResolversIfNeeded: { + __resolveReference: rawConfig.generateInternalResolversIfNeeded?.__resolveReference ?? false, + }, resolversNonOptionalTypename: normalizeResolversNonOptionalTypename( getConfigValue(rawConfig.resolversNonOptionalTypename, false) ), @@ -1282,7 +1296,7 @@ export class BaseResolversVisitor< } public hasFederation(): boolean { - return this._hasFederation; + return Object.keys(this._federation.getMeta()).length > 0; } public getRootResolver(): RootResolver { @@ -1305,8 +1319,10 @@ export class BaseResolversVisitor< userDefinedTypes[schemaTypeName] = { name: resolverType.baseGeneratedTypename, }; - if (resolverType.federation) { - userDefinedTypes[schemaTypeName].federation = resolverType.federation; + + const federationMeta = this._federation.getMeta()[schemaTypeName]; + if (federationMeta) { + userDefinedTypes[schemaTypeName].federation = federationMeta; } } @@ -1492,9 +1508,20 @@ export class BaseResolversVisitor< }; if (this._federation.isResolveReferenceField(node)) { - this._hasFederation = true; - signature.type = 'ReferenceResolver'; + if (this.config.generateInternalResolversIfNeeded.__resolveReference) { + const federationDetails = checkObjectTypeFederationDetails( + parentType.astNode as ObjectTypeDefinitionNode, + this._schema + ); + + if (!federationDetails || federationDetails.resolvableKeyDirectives.length === 0) { + return ''; + } + signature.modifier = ''; // if a federation type has resolvable @key, then it should be required + } + this._federation.setMeta(parentType.name, { hasResolveReference: true }); + signature.type = 'ReferenceResolver'; if (signature.genericTypes.length >= 3) { signature.genericTypes = signature.genericTypes.slice(0, 3); } @@ -1537,7 +1564,7 @@ export class BaseResolversVisitor< return `Partial<${argsType}>`; } - ObjectTypeDefinition(node: ObjectTypeDefinitionNode, key: number, parent: any): string { + ObjectTypeDefinition(node: ObjectTypeDefinitionNode): string { const declarationKind = 'type'; const name = this.convertName(node, { suffix: this.config.resolverTypeSuffix, @@ -1584,19 +1611,6 @@ export class BaseResolversVisitor< .withName(name, ``) .withBlock(fieldsContent.join('\n')); - this._collectedResolvers[node.name as any] = { - typename: name + '', - baseGeneratedTypename: name, - }; - - if (this.config.federation) { - const originalNode = parent[key] as ObjectTypeDefinitionNode; - const federationDetails = checkObjectTypeFederationDetails(originalNode, this._schema); - this._collectedResolvers[node.name as any].federation = { - hasResolveReference: federationDetails ? federationDetails.resolvableKeyDirectives.length > 0 : false, - }; - } - return block.string; } diff --git a/packages/plugins/other/visitor-plugin-common/src/types.ts b/packages/plugins/other/visitor-plugin-common/src/types.ts index f275fa47af7..a9aa76ce36d 100644 --- a/packages/plugins/other/visitor-plugin-common/src/types.ts +++ b/packages/plugins/other/visitor-plugin-common/src/types.ts @@ -127,3 +127,8 @@ export interface ResolversNonOptionalTypenameConfig { interfaceImplementingType?: boolean; excludeTypes?: string[]; } + +export interface GenerateInternalResolversIfNeededConfig { + __resolveReference?: boolean; +} +export type NormalizedGenerateInternalResolversIfNeededConfig = Required; 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 a70081fd926..ebace366a67 100644 --- a/packages/plugins/typescript/resolvers/tests/ts-resolvers.federation.spec.ts +++ b/packages/plugins/typescript/resolvers/tests/ts-resolvers.federation.spec.ts @@ -24,7 +24,7 @@ function generate({ schema, config }: { schema: string; config: TypeScriptResolv } describe('TypeScript Resolvers Plugin + Apollo Federation', () => { - it('should add __resolveReference to objects that have @key and is resolvable', async () => { + it('should add optional __resolveReference to objects that have @key', async () => { const federatedSchema = /* GraphQL */ ` type Query { allUsers: [User] @@ -83,12 +83,16 @@ describe('TypeScript Resolvers Plugin + Apollo Federation', () => { }, }); - // User should have it expect(content).toBeSimilarStringTo(` - __resolveReference?: ReferenceResolver, { __typename: 'User' } & GraphQLRecursivePick, ContextType>; + export type UserResolvers = { + __resolveReference?: ReferenceResolver, { __typename: 'User' } & GraphQLRecursivePick, ContextType>; + id?: Resolver; + name?: Resolver, ParentType, ContextType>; + username?: Resolver, ParentType, ContextType>; + __isTypeOf?: IsTypeOfResolverFn; + }; `); - // SingleResolvable should have __resolveReference because it has resolvable: true expect(content).toBeSimilarStringTo(` export type SingleResolvableResolvers = { __resolveReference?: ReferenceResolver, { __typename: 'SingleResolvable' } & GraphQLRecursivePick, ContextType>; @@ -97,15 +101,14 @@ describe('TypeScript Resolvers Plugin + Apollo Federation', () => { }; `); - // SingleNonResolvable shouldn't have __resolveReference because it has resolvable: false expect(content).toBeSimilarStringTo(` export type SingleNonResolvableResolvers = { + __resolveReference?: ReferenceResolver, ParentType, ContextType>; id?: Resolver; __isTypeOf?: IsTypeOfResolverFn; }; `); - // AtLeastOneResolvable should have __resolveReference because it at least one resolvable expect(content).toBeSimilarStringTo(` export type AtLeastOneResolvableResolvers = { __resolveReference?: ReferenceResolver, { __typename: 'AtLeastOneResolvable' } & GraphQLRecursivePick, ContextType>; @@ -116,7 +119,6 @@ describe('TypeScript Resolvers Plugin + Apollo Federation', () => { }; `); - // MixedResolvable should have __resolveReference and references for resolvable keys expect(content).toBeSimilarStringTo(` export type MixedResolvableResolvers = { __resolveReference?: ReferenceResolver, { __typename: 'MixedResolvable' } & (GraphQLRecursivePick | GraphQLRecursivePick), ContextType>; @@ -127,9 +129,9 @@ describe('TypeScript Resolvers Plugin + Apollo Federation', () => { }; `); - // MultipleNonResolvableResolvers does not have __resolveReference because all keys are non-resolvable expect(content).toBeSimilarStringTo(` export type MultipleNonResolvableResolvers = { + __resolveReference?: ReferenceResolver, ParentType, ContextType>; id?: Resolver; id2?: Resolver; id3?: Resolver; @@ -137,9 +139,141 @@ describe('TypeScript Resolvers Plugin + Apollo Federation', () => { }; `); - // Book shouldn't because it doesn't have @key - expect(content).not.toBeSimilarStringTo(` - __resolveReference?: ReferenceResolver, { __typename: 'Book' } & GraphQLRecursivePick, ContextType>; + // Book does NOT have __resolveReference because it doesn't have @key + expect(content).toBeSimilarStringTo(` + export type BookResolvers = { + id?: Resolver; + __isTypeOf?: IsTypeOfResolverFn; + }; + `); + }); + + it('generateInternalResolversIfNeeded - should add non-optional __resolveReference to objects that have resolvable @key', async () => { + const federatedSchema = /* GraphQL */ ` + type Query { + allUsers: [User] + } + + type User @key(fields: "id") { + id: ID! + name: String + username: String + } + + type Book { + id: ID! + } + + type SingleResolvable @key(fields: "id", resolvable: true) { + id: ID! + } + + type SingleNonResolvable @key(fields: "id", resolvable: false) { + id: ID! + } + + type AtLeastOneResolvable + @key(fields: "id", resolvable: false) + @key(fields: "id2", resolvable: true) + @key(fields: "id3", resolvable: false) { + id: ID! + id2: ID! + id3: ID! + } + + type MixedResolvable + @key(fields: "id") + @key(fields: "id2", resolvable: true) + @key(fields: "id3", resolvable: false) { + id: ID! + id2: ID! + id3: ID! + } + + type MultipleNonResolvable + @key(fields: "id", resolvable: false) + @key(fields: "id2", resolvable: false) + @key(fields: "id3", resolvable: false) { + id: ID! + id2: ID! + id3: ID! + } + `; + + const content = await generate({ + schema: federatedSchema, + config: { + federation: true, + generateInternalResolversIfNeeded: { __resolveReference: true }, + }, + }); + + // User should have __resolveReference because it has resolvable @key (by default) + expect(content).toBeSimilarStringTo(` + export type UserResolvers = { + __resolveReference: ReferenceResolver, { __typename: 'User' } & GraphQLRecursivePick, ContextType>; + id?: Resolver; + name?: Resolver, ParentType, ContextType>; + username?: Resolver, ParentType, ContextType>; + __isTypeOf?: IsTypeOfResolverFn; + }; + `); + + // SingleResolvable has __resolveReference because it has resolvable: true + expect(content).toBeSimilarStringTo(` + export type SingleResolvableResolvers = { + __resolveReference: ReferenceResolver, { __typename: 'SingleResolvable' } & GraphQLRecursivePick, ContextType>; + id?: Resolver; + __isTypeOf?: IsTypeOfResolverFn; + }; + `); + + // SingleNonResolvable does NOT have __resolveReference because it has resolvable: false + expect(content).toBeSimilarStringTo(` + export type SingleNonResolvableResolvers = { + id?: Resolver; + __isTypeOf?: IsTypeOfResolverFn; + }; + `); + + // AtLeastOneResolvable has __resolveReference because it at least one resolvable + expect(content).toBeSimilarStringTo(` + export type AtLeastOneResolvableResolvers = { + __resolveReference: ReferenceResolver, { __typename: 'AtLeastOneResolvable' } & GraphQLRecursivePick, ContextType>; + id?: Resolver; + id2?: Resolver; + id3?: Resolver; + __isTypeOf?: IsTypeOfResolverFn; + }; + `); + + // MixedResolvable has __resolveReference and references for resolvable keys + expect(content).toBeSimilarStringTo(` + export type MixedResolvableResolvers = { + __resolveReference: ReferenceResolver, { __typename: 'MixedResolvable' } & (GraphQLRecursivePick | GraphQLRecursivePick), ContextType>; + id?: Resolver; + id2?: Resolver; + id3?: Resolver; + __isTypeOf?: IsTypeOfResolverFn; + }; + `); + + // MultipleNonResolvableResolvers does NOT have __resolveReference because all keys are non-resolvable + expect(content).toBeSimilarStringTo(` + export type MultipleNonResolvableResolvers = { + id?: Resolver; + id2?: Resolver; + id3?: Resolver; + __isTypeOf?: IsTypeOfResolverFn; + }; + `); + + // Book does NOT have __resolveReference because it doesn't have @key + expect(content).toBeSimilarStringTo(` + export type BookResolvers = { + id?: Resolver; + __isTypeOf?: IsTypeOfResolverFn; + }; `); }); diff --git a/packages/utils/plugins-helpers/src/federation.ts b/packages/utils/plugins-helpers/src/federation.ts index c64ad3b16df..d77277629e7 100644 --- a/packages/utils/plugins-helpers/src/federation.ts +++ b/packages/utils/plugins-helpers/src/federation.ts @@ -1,7 +1,7 @@ import { astFromObjectType, getRootTypeNames, MapperKind, mapSchema } from '@graphql-tools/utils'; import { - type DirectiveNode, DefinitionNode, + DirectiveNode, FieldDefinitionNode, GraphQLNamedType, GraphQLObjectType, @@ -35,21 +35,18 @@ export const federationSpec = parse(/* GraphQL */ ` export function addFederationReferencesToSchema(schema: GraphQLSchema): GraphQLSchema { return mapSchema(schema, { [MapperKind.OBJECT_TYPE]: type => { - const objectTypeFederationDetails = checkObjectTypeFederationDetails(type, schema); - - if (!objectTypeFederationDetails || objectTypeFederationDetails.resolvableKeyDirectives.length === 0) { - return type; + if (checkObjectTypeFederationDetails(type, schema)) { + const typeConfig = type.toConfig(); + typeConfig.fields = { + [resolveReferenceFieldName]: { + type, + }, + ...typeConfig.fields, + }; + + return new GraphQLObjectType(typeConfig); } - - const typeConfig = type.toConfig(); - typeConfig.fields = { - [resolveReferenceFieldName]: { - type, - }, - ...typeConfig.fields, - }; - - return new GraphQLObjectType(typeConfig); + return type; }, }); } @@ -85,10 +82,15 @@ export function removeFederation(schema: GraphQLSchema): GraphQLSchema { const resolveReferenceFieldName = '__resolveReference'; +interface TypeMeta { + hasResolveReference: boolean; +} + export class ApolloFederation { private enabled = false; private schema: GraphQLSchema; private providesMap: Record; + protected meta: { [typename: string]: TypeMeta } = {}; constructor({ enabled, schema }: { enabled: boolean; schema: GraphQLSchema }) { this.enabled = enabled; @@ -199,6 +201,13 @@ export class ApolloFederation { return parentTypeSignature; } + setMeta(typename: string, update: Partial): void { + this.meta[typename] = { ...(this.meta[typename] || { hasResolveReference: false }), ...update }; + } + getMeta() { + return this.meta; + } + private isExternalAndNotProvided(fieldNode: FieldDefinitionNode, objectType: GraphQLObjectType): boolean { return this.isExternal(fieldNode) && !this.hasProvides(objectType, fieldNode); }