Skip to content

[resolvers][federation] Fix fields or types being wrong generated when marked with @external #10287

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

Open
wants to merge 12 commits into
base: federation-fixes
Choose a base branch
from
7 changes: 7 additions & 0 deletions .changeset/twenty-planets-complain.md
Original file line number Diff line number Diff line change
@@ -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
1 change: 0 additions & 1 deletion dev-test/test-schema/resolvers-federation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -210,7 +210,6 @@ export type UserResolvers<
GraphQLRecursivePick<FederationType, { address: { city: true; lines: { line2: true } } }>,
ContextType
>;

email?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
};

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -82,8 +82,13 @@ export interface ParsedResolversConfig extends ParsedConfig {
avoidCheckingAbstractTypesRecursively: boolean;
}

export interface FieldDefinitionResult {
node: FieldDefinitionNode;
printContent: FieldDefinitionPrintFn;
}

type FieldDefinitionPrintFn = (
parentName: string,
parentNode: ObjectTypeDefinitionNode | InterfaceTypeDefinitionNode,
avoidResolverOptionals: boolean
) => { value: string | null; meta: { federation?: { isResolveReference: boolean } } };
export interface RootResolver {
Expand Down Expand Up @@ -1463,6 +1468,9 @@ export class BaseResolversVisitor<
return '';
}

// FIXME: this Name method causes a lot of type inconsistencies
// because the type of nodes no longer matches the `graphql-js` types
// So, we should update this and remove any relevant `as any as string` or `as unknown as string`
Comment on lines +1471 to +1473
Copy link
Collaborator Author

@eddeee888 eddeee888 May 13, 2025

Choose a reason for hiding this comment

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

Drive-by put a FIXME here, so we remember why we need to do as any as string in a lot of places, so we can fix it later.

Name(node: NameNode): string {
return node.value;
}
Expand Down Expand Up @@ -1519,122 +1527,133 @@ 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) => {
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 function is used to print field definition. We need more details when we handle fields.

So, this now returns the original node, etc. as part of the result object. We might return more things in the future.

const original: FieldDefinitionNode = parent[key];
const parentType = this.schema.getType(parentName);
const meta: ReturnType<FieldDefinitionPrintFn>['meta'] = {};
const original: FieldDefinitionNode = parent[key];

if (this._federation.skipField({ fieldNode: original, parentType })) {
return { value: null, meta };
}
return {
node: original,
printContent: (parentNode, avoidResolverOptionals) => {
const parentName = parentNode.name as unknown as string;
const parentType = this.schema.getType(parentName);
const meta: ReturnType<FieldDefinitionPrintFn>['meta'] = {};
const typeName = node.name as unknown as string;

const fieldsToGenerate = this._federation.findFieldNodesToGenerate({ node: parentNode });
const shouldGenerateField =
fieldsToGenerate.some(field => field.name.value === typeName) ||
this._federation.isResolveReferenceField(node);

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(typeName, {
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<ResolversTypes['String']>
* - [String] -> Maybe<Array<Maybe<ResolversTypes['String']>>>
* - [String!]! -> Array<ResolversTypes['String']>
*/
const mappedType = this._variablesTransformer.wrapAstTypeWithModifiers(typeToUse, original.type);

const subscriptionType = this._schema.getSubscriptionType();
const isSubscriptionType = subscriptionType && subscriptionType.name === parentName;

if (isSubscriptionType) {
return {
mappedTypeKey: `${mappedType}, "${typeName}"`,
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<ResolversTypes['String']>
* - [String] -> Maybe<Array<Maybe<ResolversTypes['String']>>>
* - [String!]! -> Array<ResolversTypes['String']>
*/
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: typeName,
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,
};
},
};
}

Expand Down Expand Up @@ -1707,12 +1726,17 @@ export class BaseResolversVisitor<
return `Partial<${argsType}>`;
}

ObjectTypeDefinition(node: ObjectTypeDefinitionNode): string {
ObjectTypeDefinition(node: ObjectTypeDefinitionNode): string | null {
const typeName = node.name as unknown as string;
const fieldsToGenerate = this._federation.findFieldNodesToGenerate({ node });
if (fieldsToGenerate.length === 0) {
return null;
}

const declarationKind = 'type';
const name = this.convertName(node, {
suffix: this.config.resolverTypeSuffix,
});
const typeName = node.name as any as string;
const parentType = this.getParentTypeToUse(typeName);

const rootType = ((): false | 'query' | 'mutation' | 'subscription' => {
Expand All @@ -1728,15 +1752,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(({ printContent }) => {
return printContent(
node,
(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(
Expand All @@ -1748,6 +1774,10 @@ export class BaseResolversVisitor<
);
}

if (fieldsContent.length === 0) {
return null;
}

const genericTypes: string[] = [
`ContextType = ${this.config.contextType.type}`,
this.transformParentGenericType(parentType),
Expand Down Expand Up @@ -1958,8 +1988,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(({ printContent }) =>
printContent(node, this.config.avoidOptionals.resolvers)
);
for (const field of fields) {
if (field.meta.federation?.isResolveReference) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,6 @@ describe('customDirectives.sematicNonNull', () => {
nonNullableListWithNonNullableItemLevel0?: Resolver<Array<ResolversTypes['String']>, ParentType, ContextType>;
nonNullableListWithNonNullableItemLevel1?: Resolver<Array<ResolversTypes['String']>, ParentType, ContextType>;
nonNullableListWithNonNullableItemBothLevels?: Resolver<Array<ResolversTypes['String']>, ParentType, ContextType>;
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
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 a new test from master.
__isTypeOf is not required here after this PR in the feature branch: #10283

};
`);
});
Expand Down
Loading
Loading