Skip to content

Commit 0b41ef2

Browse files
eddeee888eddeee888
committed
[resolvers][federation] Fix fields or types being wrong generated when marked with @external (#10287)
* 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 * Format and minor text updates * Add comments * Fix __resolveReference not getting generated * Fix test with __isTypeOf when not needed * Fix type cast that results in wrong type * Revert unncessary changes to FieldDefinitionPrintFn * Re-format * Convert to use AST Node instead of GraphQL Type * Update test template * Cache field nodes to generate for processed objects * Put FIXME on base-resolvers-visitor to remove Name method
1 parent 9f1798d commit 0b41ef2

File tree

6 files changed

+294
-152
lines changed

6 files changed

+294
-152
lines changed

‎.changeset/twenty-planets-complain.md

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
---
2+
'@graphql-codegen/visitor-plugin-common': patch
3+
'@graphql-codegen/typescript-resolvers': patch
4+
'@graphql-codegen/plugin-helpers': patch
5+
---
6+
7+
Fix fields or object types marked with @external being wrongly generated

‎dev-test/test-schema/resolvers-federation.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -210,7 +210,6 @@ export type UserResolvers<
210210
GraphQLRecursivePick<FederationType, { address: { city: true; lines: { line2: true } } }>,
211211
ContextType
212212
>;
213-
214213
email?: Resolver<ResolversTypes['String'], ParentType, ContextType>;
215214
};
216215

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

Lines changed: 145 additions & 115 deletions
Original file line numberDiff line numberDiff line change
@@ -82,8 +82,13 @@ export interface ParsedResolversConfig extends ParsedConfig {
8282
avoidCheckingAbstractTypesRecursively: boolean;
8383
}
8484

85+
export interface FieldDefinitionResult {
86+
node: FieldDefinitionNode;
87+
printContent: FieldDefinitionPrintFn;
88+
}
89+
8590
type FieldDefinitionPrintFn = (
86-
parentName: string,
91+
parentNode: ObjectTypeDefinitionNode | InterfaceTypeDefinitionNode,
8792
avoidResolverOptionals: boolean
8893
) => { value: string | null; meta: { federation?: { isResolveReference: boolean } } };
8994
export interface RootResolver {
@@ -1467,6 +1472,9 @@ export class BaseResolversVisitor<
14671472
return '';
14681473
}
14691474

1475+
// FIXME: this Name method causes a lot of type inconsistencies
1476+
// because the type of nodes no longer matches the `graphql-js` types
1477+
// So, we should update this and remove any relevant `as any as string` or `as unknown as string`
14701478
Name(node: NameNode): string {
14711479
return node.value;
14721480
}
@@ -1523,122 +1531,133 @@ export class BaseResolversVisitor<
15231531
return `ParentType extends ${parentType} = ${parentType}`;
15241532
}
15251533

1526-
FieldDefinition(node: FieldDefinitionNode, key: string | number, parent: any): FieldDefinitionPrintFn {
1534+
FieldDefinition(node: FieldDefinitionNode, key: string | number, parent: any): FieldDefinitionResult {
15271535
const hasArguments = node.arguments && node.arguments.length > 0;
15281536
const declarationKind = 'type';
15291537

1530-
return (parentName, avoidResolverOptionals) => {
1531-
const original: FieldDefinitionNode = parent[key];
1532-
const parentType = this.schema.getType(parentName);
1533-
const meta: ReturnType<FieldDefinitionPrintFn>['meta'] = {};
1534-
1535-
if (this._federation.skipField({ fieldNode: original, parentType })) {
1536-
return { value: null, meta };
1537-
}
1538+
const original: FieldDefinitionNode = parent[key];
15381539

1539-
const contextType = this.getContextType(parentName, node);
1540-
1541-
let argsType = hasArguments
1542-
? this.convertName(
1543-
parentName +
1544-
(this.config.addUnderscoreToArgsType ? '_' : '') +
1545-
this.convertName(node.name, {
1546-
useTypesPrefix: false,
1547-
useTypesSuffix: false,
1548-
}) +
1549-
'Args',
1550-
{
1551-
useTypesPrefix: true,
1552-
},
1553-
true
1554-
)
1555-
: null;
1540+
return {
1541+
node: original,
1542+
printContent: (parentNode, avoidResolverOptionals) => {
1543+
const parentName = parentNode.name as unknown as string;
1544+
const parentType = this.schema.getType(parentName);
1545+
const meta: ReturnType<FieldDefinitionPrintFn>['meta'] = {};
1546+
const typeName = node.name as unknown as string;
1547+
1548+
const fieldsToGenerate = this._federation.findFieldNodesToGenerate({ node: parentNode });
1549+
const shouldGenerateField =
1550+
fieldsToGenerate.some(field => field.name.value === typeName) ||
1551+
this._federation.isResolveReferenceField(node);
1552+
1553+
if (!shouldGenerateField) {
1554+
return { value: null, meta };
1555+
}
15561556

1557-
const avoidInputsOptionals = this.config.avoidOptionals.inputValue;
1557+
const contextType = this.getContextType(parentName, node);
1558+
1559+
let argsType = hasArguments
1560+
? this.convertName(
1561+
parentName +
1562+
(this.config.addUnderscoreToArgsType ? '_' : '') +
1563+
this.convertName(typeName, {
1564+
useTypesPrefix: false,
1565+
useTypesSuffix: false,
1566+
}) +
1567+
'Args',
1568+
{
1569+
useTypesPrefix: true,
1570+
},
1571+
true
1572+
)
1573+
: null;
1574+
1575+
const avoidInputsOptionals = this.config.avoidOptionals.inputValue;
1576+
1577+
if (argsType !== null) {
1578+
const argsToForceRequire = original.arguments.filter(
1579+
arg => !!arg.defaultValue || arg.type.kind === 'NonNullType'
1580+
);
1581+
1582+
if (argsToForceRequire.length > 0) {
1583+
argsType = this.applyRequireFields(argsType, argsToForceRequire);
1584+
} else if (original.arguments.length > 0 && avoidInputsOptionals !== true) {
1585+
argsType = this.applyOptionalFields(argsType, original.arguments);
1586+
}
1587+
}
15581588

1559-
if (argsType !== null) {
1560-
const argsToForceRequire = original.arguments.filter(
1561-
arg => !!arg.defaultValue || arg.type.kind === 'NonNullType'
1562-
);
1589+
const parentTypeSignature = this._federation.transformFieldParentType({
1590+
fieldNode: original,
1591+
parentType,
1592+
parentTypeSignature: this.getParentTypeForSignature(node),
1593+
federationTypeSignature: 'FederationType',
1594+
});
15631595

1564-
if (argsToForceRequire.length > 0) {
1565-
argsType = this.applyRequireFields(argsType, argsToForceRequire);
1566-
} else if (original.arguments.length > 0 && avoidInputsOptionals !== true) {
1567-
argsType = this.applyOptionalFields(argsType, original.arguments);
1568-
}
1569-
}
1596+
const { mappedTypeKey, resolverType } = ((): { mappedTypeKey: string; resolverType: string } => {
1597+
const baseType = getBaseTypeNode(original.type);
1598+
const realType = baseType.name.value;
1599+
const typeToUse = this.getTypeToUse(realType);
1600+
/**
1601+
* Turns GraphQL type to TypeScript types (`mappedType`) e.g.
1602+
* - String! -> ResolversTypes['String']>
1603+
* - String -> Maybe<ResolversTypes['String']>
1604+
* - [String] -> Maybe<Array<Maybe<ResolversTypes['String']>>>
1605+
* - [String!]! -> Array<ResolversTypes['String']>
1606+
*/
1607+
const mappedType = this._variablesTransformer.wrapAstTypeWithModifiers(typeToUse, original.type);
1608+
1609+
const subscriptionType = this._schema.getSubscriptionType();
1610+
const isSubscriptionType = subscriptionType && subscriptionType.name === parentName;
1611+
1612+
if (isSubscriptionType) {
1613+
return {
1614+
mappedTypeKey: `${mappedType}, "${typeName}"`,
1615+
resolverType: 'SubscriptionResolver',
1616+
};
1617+
}
15701618

1571-
const parentTypeSignature = this._federation.transformFieldParentType({
1572-
fieldNode: original,
1573-
parentType,
1574-
parentTypeSignature: this.getParentTypeForSignature(node),
1575-
federationTypeSignature: 'FederationType',
1576-
});
1619+
const directiveMappings =
1620+
node.directives
1621+
?.map(directive => this._directiveResolverMappings[directive.name as any])
1622+
.filter(Boolean)
1623+
.reverse() ?? [];
15771624

1578-
const { mappedTypeKey, resolverType } = ((): { mappedTypeKey: string; resolverType: string } => {
1579-
const baseType = getBaseTypeNode(original.type);
1580-
const realType = baseType.name.value;
1581-
const typeToUse = this.getTypeToUse(realType);
1582-
/**
1583-
* Turns GraphQL type to TypeScript types (`mappedType`) e.g.
1584-
* - String! -> ResolversTypes['String']>
1585-
* - String -> Maybe<ResolversTypes['String']>
1586-
* - [String] -> Maybe<Array<Maybe<ResolversTypes['String']>>>
1587-
* - [String!]! -> Array<ResolversTypes['String']>
1588-
*/
1589-
const mappedType = this._variablesTransformer.wrapAstTypeWithModifiers(typeToUse, original.type);
1590-
1591-
const subscriptionType = this._schema.getSubscriptionType();
1592-
const isSubscriptionType = subscriptionType && subscriptionType.name === parentName;
1593-
1594-
if (isSubscriptionType) {
15951625
return {
1596-
mappedTypeKey: `${mappedType}, "${node.name}"`,
1597-
resolverType: 'SubscriptionResolver',
1626+
mappedTypeKey: mappedType,
1627+
resolverType: directiveMappings[0] ?? 'Resolver',
15981628
};
1599-
}
1629+
})();
1630+
1631+
const signature: {
1632+
name: string;
1633+
modifier: string;
1634+
type: string;
1635+
genericTypes: string[];
1636+
} = {
1637+
name: typeName,
1638+
modifier: avoidResolverOptionals ? '' : '?',
1639+
type: resolverType,
1640+
genericTypes: [mappedTypeKey, parentTypeSignature, contextType, argsType].filter(f => f),
1641+
};
16001642

1601-
const directiveMappings =
1602-
node.directives
1603-
?.map(directive => this._directiveResolverMappings[directive.name as any])
1604-
.filter(Boolean)
1605-
.reverse() ?? [];
1643+
if (this._federation.isResolveReferenceField(node)) {
1644+
if (!this._federation.getMeta()[parentType.name].hasResolveReference) {
1645+
return { value: '', meta };
1646+
}
1647+
signature.type = 'ReferenceResolver';
1648+
signature.genericTypes = [mappedTypeKey, parentTypeSignature, contextType];
1649+
meta.federation = { isResolveReference: true };
1650+
}
16061651

16071652
return {
1608-
mappedTypeKey: mappedType,
1609-
resolverType: directiveMappings[0] ?? 'Resolver',
1653+
value: indent(
1654+
`${signature.name}${signature.modifier}: ${signature.type}<${signature.genericTypes.join(
1655+
', '
1656+
)}>${this.getPunctuation(declarationKind)}`
1657+
),
1658+
meta,
16101659
};
1611-
})();
1612-
1613-
const signature: {
1614-
name: string;
1615-
modifier: string;
1616-
type: string;
1617-
genericTypes: string[];
1618-
} = {
1619-
name: node.name as any,
1620-
modifier: avoidResolverOptionals ? '' : '?',
1621-
type: resolverType,
1622-
genericTypes: [mappedTypeKey, parentTypeSignature, contextType, argsType].filter(f => f),
1623-
};
1624-
1625-
if (this._federation.isResolveReferenceField(node)) {
1626-
if (!this._federation.getMeta()[parentType.name].hasResolveReference) {
1627-
return { value: '', meta };
1628-
}
1629-
signature.type = 'ReferenceResolver';
1630-
signature.genericTypes = [mappedTypeKey, parentTypeSignature, contextType];
1631-
meta.federation = { isResolveReference: true };
1632-
}
1633-
1634-
return {
1635-
value: indent(
1636-
`${signature.name}${signature.modifier}: ${signature.type}<${signature.genericTypes.join(
1637-
', '
1638-
)}>${this.getPunctuation(declarationKind)}`
1639-
),
1640-
meta,
1641-
};
1660+
},
16421661
};
16431662
}
16441663

@@ -1711,12 +1730,17 @@ export class BaseResolversVisitor<
17111730
return `Partial<${argsType}>`;
17121731
}
17131732

1714-
ObjectTypeDefinition(node: ObjectTypeDefinitionNode): string {
1733+
ObjectTypeDefinition(node: ObjectTypeDefinitionNode): string | null {
1734+
const typeName = node.name as unknown as string;
1735+
const fieldsToGenerate = this._federation.findFieldNodesToGenerate({ node });
1736+
if (fieldsToGenerate.length === 0) {
1737+
return null;
1738+
}
1739+
17151740
const declarationKind = 'type';
17161741
const name = this.convertName(node, {
17171742
suffix: this.config.resolverTypeSuffix,
17181743
});
1719-
const typeName = node.name as any as string;
17201744
const parentType = this.getParentTypeToUse(typeName);
17211745

17221746
const rootType = ((): false | 'query' | 'mutation' | 'subscription' => {
@@ -1732,15 +1756,17 @@ export class BaseResolversVisitor<
17321756
return false;
17331757
})();
17341758

1735-
const fieldsContent = (node.fields as unknown as FieldDefinitionPrintFn[]).map(f => {
1736-
return f(
1737-
typeName,
1738-
(rootType === 'query' && this.config.avoidOptionals.query) ||
1739-
(rootType === 'mutation' && this.config.avoidOptionals.mutation) ||
1740-
(rootType === 'subscription' && this.config.avoidOptionals.subscription) ||
1741-
(rootType === false && this.config.avoidOptionals.resolvers)
1742-
).value;
1743-
});
1759+
const fieldsContent = (node.fields as unknown as FieldDefinitionResult[])
1760+
.map(({ printContent }) => {
1761+
return printContent(
1762+
node,
1763+
(rootType === 'query' && this.config.avoidOptionals.query) ||
1764+
(rootType === 'mutation' && this.config.avoidOptionals.mutation) ||
1765+
(rootType === 'subscription' && this.config.avoidOptionals.subscription) ||
1766+
(rootType === false && this.config.avoidOptionals.resolvers)
1767+
).value;
1768+
})
1769+
.filter(v => v);
17441770

17451771
if (!rootType && this._parsedSchemaMeta.typesWithIsTypeOf[typeName]) {
17461772
fieldsContent.push(
@@ -1752,6 +1778,10 @@ export class BaseResolversVisitor<
17521778
);
17531779
}
17541780

1781+
if (fieldsContent.length === 0) {
1782+
return null;
1783+
}
1784+
17551785
const genericTypes: string[] = [
17561786
`ContextType = ${this.config.contextType.type}`,
17571787
this.transformParentGenericType(parentType),
@@ -1962,8 +1992,8 @@ export class BaseResolversVisitor<
19621992

19631993
// An Interface in Federation may have the additional __resolveReference resolver, if resolvable.
19641994
// So, we filter out the normal fields declared on the Interface and add the __resolveReference resolver.
1965-
const fields = (node.fields as unknown as FieldDefinitionPrintFn[]).map(f =>
1966-
f(typeName, this.config.avoidOptionals.resolvers)
1995+
const fields = (node.fields as unknown as FieldDefinitionResult[]).map(({ printContent }) =>
1996+
printContent(node, this.config.avoidOptionals.resolvers)
19671997
);
19681998
for (const field of fields) {
19691999
if (field.meta.federation?.isResolveReference) {

‎packages/plugins/typescript/resolvers/tests/ts-resolvers.config.customDirectives.spec.ts

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,7 +62,6 @@ describe('customDirectives.sematicNonNull', () => {
6262
nonNullableListWithNonNullableItemLevel0?: Resolver<Array<ResolversTypes['String']>, ParentType, ContextType>;
6363
nonNullableListWithNonNullableItemLevel1?: Resolver<Array<ResolversTypes['String']>, ParentType, ContextType>;
6464
nonNullableListWithNonNullableItemBothLevels?: Resolver<Array<ResolversTypes['String']>, ParentType, ContextType>;
65-
__isTypeOf?: IsTypeOfResolverFn<ParentType, ContextType>;
6665
};
6766
`);
6867
});

0 commit comments

Comments
 (0)