Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[resolvers] Ensure __isTypeof is only generated for implementing types (of Interfaces) or Union members #10283

Open
wants to merge 6 commits into
base: federation-fixes
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 5 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions .changeset/angry-lamps-notice.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
'@graphql-codegen/visitor-plugin-common': major
'@graphql-codegen/typescript-resolvers': major
'@graphql-codegen/plugin-helpers': major
---

BREAKING CHANGES: Do not generate \_\_isTypeOf for non-implementing types or non-union members
3 changes: 0 additions & 3 deletions dev-test/modules/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -220,7 +220,6 @@ export type ArticleResolvers<
id?: Resolver<ResolversTypes['ID'], ParentType, ContextType>;
text?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
title?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
};

export type CreditCardResolvers<
Expand All @@ -241,7 +240,6 @@ export type DonationResolvers<
id?: Resolver<ResolversTypes['ID'], ParentType, ContextType>;
recipient?: Resolver<ResolversTypes['User'], ParentType, ContextType>;
sender?: Resolver<ResolversTypes['User'], ParentType, ContextType>;
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
};

export type MutationResolvers<
Expand Down Expand Up @@ -298,7 +296,6 @@ export type UserResolvers<
id?: Resolver<ResolversTypes['ID'], ParentType, ContextType>;
lastName?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
paymentOptions?: Resolver<Maybe<Array<ResolversTypes['PaymentOption']>>, ParentType, ContextType>;
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
};

export type Resolvers<ContextType = any> = {
Expand Down
2 changes: 0 additions & 2 deletions dev-test/subpath-import/result.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -144,7 +144,6 @@ export type UserResolvers<
name?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
password?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
updatedAt?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
};

export type MutationResolvers<
Expand All @@ -157,7 +156,6 @@ export type MutationResolvers<
FiedContextType,
RequireFields<MutationCreateUserArgs, 'email' | 'name' | 'password'>
>;
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
};

export type Resolvers<ContextType = TestContext> = {
Expand Down
5 changes: 0 additions & 5 deletions dev-test/test-schema/resolvers-federation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -166,15 +166,13 @@ export type AddressResolvers<
city?: Resolver<Maybe<ResolversTypes['String']>, ParentType, ContextType>;
lines?: Resolver<ResolversTypes['Lines'], ParentType, ContextType>;
state?: Resolver<Maybe<ResolversTypes['String']>, ParentType, ContextType>;
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
};

export type BookResolvers<
ContextType = any,
ParentType extends ResolversParentTypes['Book'] = ResolversParentTypes['Book']
> = {
id?: Resolver<ResolversTypes['ID'], ParentType, ContextType>;
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
};

export type LinesResolvers<
Expand All @@ -183,7 +181,6 @@ export type LinesResolvers<
> = {
line1?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
line2?: Resolver<Maybe<ResolversTypes['String']>, ParentType, ContextType>;
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
};

export type QueryResolvers<
Expand Down Expand Up @@ -216,8 +213,6 @@ export type UserResolvers<
GraphQLRecursivePick<FederationType, { address: { city: true; lines: { line2: true } } }>,
ContextType
>;

__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
};

export type Resolvers<ContextType = any> = {
Expand Down
2 changes: 0 additions & 2 deletions dev-test/test-schema/resolvers-root.ts
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,6 @@ export type QueryResolvers<
ParentType extends ResolversParentTypes['Query'] = ResolversParentTypes['Query']
> = {
someDummyField?: Resolver<ResolversTypes['Int'], ParentType, ContextType>;
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
};

export type QueryRootResolvers<
Expand Down Expand Up @@ -172,7 +171,6 @@ export type UserResolvers<
email?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
id?: Resolver<ResolversTypes['Int'], ParentType, ContextType>;
name?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
};

export type Resolvers<ContextType = any> = {
Expand Down
1 change: 0 additions & 1 deletion dev-test/test-schema/resolvers-stitching.ts
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,6 @@ export type UserResolvers<
email?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
id?: Resolver<ResolversTypes['Int'], ParentType, ContextType>;
name?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
};

export type Resolvers<ContextType = any> = {
Expand Down
1 change: 0 additions & 1 deletion dev-test/test-schema/resolvers-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -148,7 +148,6 @@ export type UserResolvers<
email?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
id?: Resolver<ResolversTypes['Int'], ParentType, ContextType>;
name?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
};

export type Resolvers<ContextType = any> = {
Expand Down
1 change: 0 additions & 1 deletion dev-test/test-schema/typings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,6 @@ export type UserResolvers<
email?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
id?: Resolver<ResolversTypes['Int'], ParentType, ContextType>;
name?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
};

export type Resolvers<ContextType = any> = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,11 @@ import {
DirectiveDefinitionNode,
EnumTypeDefinitionNode,
FieldDefinitionNode,
GraphQLInterfaceType,
GraphQLNamedType,
GraphQLObjectType,
GraphQLSchema,
GraphQLUnionType,
InputValueDefinitionNode,
InterfaceTypeDefinitionNode,
isEnumType,
Expand All @@ -33,8 +35,6 @@ import {
ConvertOptions,
DeclarationKind,
EnumValuesMap,
type NormalizedGenerateInternalResolversIfNeededConfig,
type GenerateInternalResolversIfNeededConfig,
NormalizedAvoidOptionalsConfig,
NormalizedScalarsMap,
ParsedEnumValuesMap,
Expand Down Expand Up @@ -77,7 +77,6 @@ export interface ParsedResolversConfig extends ParsedConfig {
resolverTypeSuffix: string;
allResolversTypeName: string;
internalResolversPrefix: string;
generateInternalResolversIfNeeded: NormalizedGenerateInternalResolversIfNeededConfig;
directiveResolverMappings: Record<string, string>;
resolversNonOptionalTypename: ResolversNonOptionalTypenameConfig;
avoidCheckingAbstractTypesRecursively: boolean;
Expand Down Expand Up @@ -584,15 +583,6 @@ 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 {}
* @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
*/
generateInternalResolversIfNeeded?: GenerateInternalResolversIfNeededConfig;
/**
* @description Makes `__typename` of resolver mappings non-optional without affecting the base types.
* @default false
Expand Down Expand Up @@ -671,6 +661,31 @@ export class BaseResolversVisitor<
baseGeneratedTypename?: string;
};
} = {};
protected _parsedSchemaMeta: {
types: {
interface: Record<
string,
{
type: GraphQLInterfaceType;
implementingTypes: Record<string, GraphQLObjectType>;
}
>;
union: Record<
string,
{
type: GraphQLUnionType;
unionMembers: Record<string, GraphQLObjectType>;
}
>;
};
typesWithIsTypeOf: Record<string, true>;
} = {
types: {
interface: {},
union: {},
},
typesWithIsTypeOf: {},
};
protected _collectedDirectiveResolvers: { [key: string]: string } = {};
protected _variablesTransformer: OperationVariablesToObject;
protected _usedMappers: { [key: string]: boolean } = {};
Expand All @@ -679,7 +694,6 @@ export class BaseResolversVisitor<
protected _hasReferencedResolversUnionTypes = false;
protected _hasReferencedResolversInterfaceTypes = false;
protected _resolversUnionTypes: Record<string, string> = {};
protected _resolversUnionParentTypes: Record<string, string> = {};
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is no longer used, so we can remove it now.

protected _resolversInterfaceTypes: Record<string, string> = {};
protected _rootTypeNames = new Set<string>();
protected _globalDeclarations = new Set<string>();
Expand Down Expand Up @@ -745,6 +759,11 @@ export class BaseResolversVisitor<
this.config.namespacedImportName
);

// 1. Parse schema meta at the start once,
// so we can use it in subsequent generate functions
this.parseSchemaMeta();

// 2. Generate types for resolvers
this._resolversTypes = this.createResolversFields({
applyWrapper: type => this.applyResolverTypeWrapper(type),
clearWrapper: type => this.clearResolverTypeWrapper(type),
Expand Down Expand Up @@ -1006,7 +1025,7 @@ export class BaseResolversVisitor<
const { unionMember, excludeTypes } = this.config.resolversNonOptionalTypename;
res[typeName] = this.getAbstractMembersType({
typeName,
memberTypes: schemaType.getTypes(),
memberTypes: Object.values(this._parsedSchemaMeta.types.union[schemaType.name].unionMembers),
isTypenameNonOptional: unionMember && !excludeTypes?.includes(typeName),
});
}
Expand All @@ -1028,24 +1047,11 @@ export class BaseResolversVisitor<
const schemaType = allSchemaTypes[typeName];

if (isInterfaceType(schemaType)) {
const allTypesMap = this._schema.getTypeMap();
const implementingTypes: GraphQLObjectType[] = [];

for (const graphqlType of Object.values(allTypesMap)) {
if (graphqlType instanceof GraphQLObjectType) {
const allInterfaces = graphqlType.getInterfaces();

if (allInterfaces.some(int => int.name === schemaType.name)) {
implementingTypes.push(graphqlType);
}
}
}

const { interfaceImplementingType, excludeTypes } = this.config.resolversNonOptionalTypename;

res[typeName] = this.getAbstractMembersType({
typeName,
memberTypes: implementingTypes,
memberTypes: Object.values(this._parsedSchemaMeta.types.interface[schemaType.name].implementingTypes),
isTypenameNonOptional: interfaceImplementingType && !excludeTypes?.includes(typeName),
});
}
Expand Down Expand Up @@ -1584,6 +1590,46 @@ export class BaseResolversVisitor<
return contextType;
}

private parseSchemaMeta(): void {
const allSchemaTypes = this._schema.getTypeMap();
const typeNames = this._federation.filterTypeNames(Object.keys(allSchemaTypes));

for (const typeName of typeNames) {
const schemaType = allSchemaTypes[typeName];

if (isUnionType(schemaType)) {
this._parsedSchemaMeta.types.union[schemaType.name] = {
type: schemaType,
unionMembers: {},
};

const unionMemberTypes = schemaType.getTypes();
for (const type of unionMemberTypes) {
this._parsedSchemaMeta.types.union[schemaType.name].unionMembers[type.name] = type;
this._parsedSchemaMeta.typesWithIsTypeOf[type.name] = true;
}
}

if (isInterfaceType(schemaType)) {
this._parsedSchemaMeta.types.interface[schemaType.name] = {
type: schemaType,
implementingTypes: {},
};

for (const graphqlType of Object.values(allSchemaTypes)) {
if (graphqlType instanceof GraphQLObjectType) {
const allInterfaces = graphqlType.getInterfaces();

if (allInterfaces.some(int => int.name === schemaType.name)) {
this._parsedSchemaMeta.types.interface[schemaType.name].implementingTypes[graphqlType.name] = graphqlType;
this._parsedSchemaMeta.typesWithIsTypeOf[graphqlType.name] = true;
}
}
}
}
}
}

protected applyRequireFields(argsType: string, fields: InputValueDefinitionNode[]): string {
this._globalDeclarations.add(REQUIRE_FIELDS_TYPE);
return `RequireFields<${argsType}, ${fields.map(f => `'${f.name.value}'`).join(' | ')}>`;
Expand Down Expand Up @@ -1624,7 +1670,7 @@ export class BaseResolversVisitor<
).value;
});

if (!rootType) {
if (!rootType && this._parsedSchemaMeta.typesWithIsTypeOf[typeName]) {
fieldsContent.push(
indent(
`${
Expand Down
3 changes: 0 additions & 3 deletions packages/plugins/other/visitor-plugin-common/src/types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,3 @@ export interface CustomDirectivesConfig {
*/
apolloUnmask?: boolean;
}

export interface GenerateInternalResolversIfNeededConfig {}
export type NormalizedGenerateInternalResolversIfNeededConfig = Required<GenerateInternalResolversIfNeededConfig>;
Original file line number Diff line number Diff line change
Expand Up @@ -181,7 +181,6 @@ describe('TypeScript Resolvers Plugin + Apollo Federation - Interface', () => {
export type PersonNameResolvers<ContextType = any, ParentType extends ResolversParentTypes['PersonName'] = ResolversParentTypes['PersonName']> = {
first?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
last?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
};
export type Resolvers<ContextType = any> = {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,13 +147,11 @@ describe('TypeScript Resolvers Plugin + Apollo Federation - mappers', () => {
__resolveReference?: ReferenceResolver<Maybe<ResolversTypes['User']>, { __typename: 'User' } & GraphQLRecursivePick<FederationType, {"id":true}>, ContextType>;
id?: Resolver<ResolversTypes['ID'], ParentType, ContextType>;
name?: Resolver<Maybe<ResolversTypes['String']>, ParentType, ContextType>;
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
};

export type UserProfileResolvers<ContextType = any, ParentType extends ResolversParentTypes['UserProfile'] = ResolversParentTypes['UserProfile']> = {
id?: Resolver<ResolversTypes['ID'], ParentType, ContextType>;
user?: Resolver<ResolversTypes['User'], ParentType, ContextType>;
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
};

export type Resolvers<ContextType = any> = {
Expand Down
Loading
Loading