Skip to content

Commit 5dad86e

Browse files
authored
CODEGEN-859 [graphql-modules-preset] Fix __isTypeOf getting picked from from all objects (#10447)
* Handle __isTypeOf correctly in graphql-modules-preset * Add changeset * Fix dev-tests * Update unit tests
1 parent fc7a2f9 commit 5dad86e

File tree

8 files changed

+116
-7
lines changed

8 files changed

+116
-7
lines changed

.changeset/lemon-insects-attend.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'@graphql-codegen/graphql-modules-preset': patch
3+
---
4+
5+
Fix \_\_isTypeOf wrongly picked on objects that are not implementing types or union members

dev-test/modules/blog/generated.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ export namespace BlogModule {
1010
export type User = Types.User;
1111
export type Query = Pick<Types.Query, DefinedFields['Query']>;
1212

13-
export type ArticleResolvers = Pick<Types.ArticleResolvers, DefinedFields['Article'] | '__isTypeOf'>;
13+
export type ArticleResolvers = Pick<Types.ArticleResolvers, DefinedFields['Article']>;
1414
export type QueryResolvers = Pick<Types.QueryResolvers, DefinedFields['Query']>;
1515

1616
export interface Resolvers {

dev-test/modules/dotanions/generated.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -23,7 +23,7 @@ export namespace DotanionsModule {
2323

2424
export type PaypalResolvers = Pick<Types.PaypalResolvers, DefinedFields['Paypal'] | '__isTypeOf'>;
2525
export type CreditCardResolvers = Pick<Types.CreditCardResolvers, DefinedFields['CreditCard'] | '__isTypeOf'>;
26-
export type DonationResolvers = Pick<Types.DonationResolvers, DefinedFields['Donation'] | '__isTypeOf'>;
26+
export type DonationResolvers = Pick<Types.DonationResolvers, DefinedFields['Donation']>;
2727
export type MutationResolvers = Pick<Types.MutationResolvers, DefinedFields['Mutation']>;
2828
export type UserResolvers = Pick<Types.UserResolvers, DefinedFields['User']>;
2929

dev-test/modules/users/generated.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,7 @@ export namespace UsersModule {
99
export type User = Pick<Types.User, DefinedFields['User']>;
1010
export type Query = Pick<Types.Query, DefinedFields['Query']>;
1111

12-
export type UserResolvers = Pick<Types.UserResolvers, DefinedFields['User'] | '__isTypeOf'>;
12+
export type UserResolvers = Pick<Types.UserResolvers, DefinedFields['User']>;
1313
export type QueryResolvers = Pick<Types.QueryResolvers, DefinedFields['Query']>;
1414

1515
export interface Resolvers {

packages/presets/graphql-modules/src/builder.ts

Lines changed: 18 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,7 @@ type RegistryKeys = 'objects' | 'inputs' | 'interfaces' | 'scalars' | 'unions' |
3434
type Registry = Record<RegistryKeys, string[]>;
3535
const registryKeys: RegistryKeys[] = ['objects', 'inputs', 'interfaces', 'scalars', 'unions', 'enums'];
3636
const resolverKeys: Array<Extract<RegistryKeys, 'objects' | 'enums' | 'scalars'>> = ['scalars', 'objects', 'enums'];
37+
const withIsTypeOfKeys: Array<'objects'> = ['objects'];
3738

3839
export function buildModule(
3940
name: string,
@@ -65,6 +66,7 @@ export function buildModule(
6566
const picks: Record<RegistryKeys, Record<string, string[]>> = createObject(registryKeys, () => ({}));
6667
const defined: Registry = createObject(registryKeys, () => []);
6768
const extended: Registry = createObject(registryKeys, () => []);
69+
const withIsTypeOf: { objects: string[] } = createObject(withIsTypeOfKeys, () => []);
6870

6971
// List of types used in objects, fields, arguments etc
7072
const usedTypes = collectUsedTypes(doc);
@@ -216,7 +218,9 @@ export function buildModule(
216218
'DefinedFields',
217219
// In case of enabled `requireRootResolvers` flag, the preset has to produce a non-optional properties.
218220
requireRootResolvers && rootTypes.includes(name),
219-
!rootTypes.includes(name) && defined.objects.includes(name) ? ` | '__isTypeOf'` : ''
221+
!rootTypes.includes(name) && defined.objects.includes(name) && withIsTypeOf.objects.includes(name)
222+
? ` | '__isTypeOf'`
223+
: ''
220224
)
221225
)
222226
.join('\n'),
@@ -405,6 +409,11 @@ export function buildModule(
405409
case Kind.OBJECT_TYPE_DEFINITION: {
406410
defined.objects.push(name);
407411
collectFields(node, picks.objects);
412+
413+
if (node.interfaces?.length > 0) {
414+
withIsTypeOf.objects.push(name);
415+
}
416+
408417
break;
409418
}
410419

@@ -433,6 +442,10 @@ export function buildModule(
433442

434443
case Kind.UNION_TYPE_DEFINITION: {
435444
defined.unions.push(name);
445+
446+
for (const namedType of node.types || []) {
447+
pushUnique(withIsTypeOf.objects, namedType.name.value);
448+
}
436449
break;
437450
}
438451
}
@@ -453,6 +466,10 @@ export function buildModule(
453466

454467
pushUnique(extended.objects, name);
455468

469+
if (node.interfaces?.length > 0) {
470+
pushUnique(withIsTypeOf.objects, name);
471+
}
472+
456473
break;
457474
}
458475

packages/presets/graphql-modules/tests/__snapshots__/integration.spec.ts.snap

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ export type Mutation = Pick<Types.Mutation, DefinedFields['Mutation']>;
2525
2626
export type PaypalResolvers = Pick<Types.PaypalResolvers, DefinedFields['Paypal'] | '__isTypeOf'>;
2727
export type CreditCardResolvers = Pick<Types.CreditCardResolvers, DefinedFields['CreditCard'] | '__isTypeOf'>;
28-
export type DonationResolvers = Pick<Types.DonationResolvers, DefinedFields['Donation'] | '__isTypeOf'>;
28+
export type DonationResolvers = Pick<Types.DonationResolvers, DefinedFields['Donation']>;
2929
export type MutationResolvers = Pick<Types.MutationResolvers, DefinedFields['Mutation']>;
3030
export type UserResolvers = Pick<Types.UserResolvers, DefinedFields['User']>;
3131

packages/presets/graphql-modules/tests/builder.spec.ts

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -479,3 +479,90 @@ test('should generate a signature for ResolveMiddleware (with widlcards)', () =>
479479
};
480480
`);
481481
});
482+
483+
test('only picks __isTypeOf from implementing types (of Interfaces) and union members', () => {
484+
const output = buildModule(
485+
'test',
486+
parse(/* GraphQL */ `
487+
type Query {
488+
me: User
489+
pet: Pet
490+
offer: Offer
491+
}
492+
493+
type User {
494+
id: ID!
495+
username: String!
496+
}
497+
498+
interface Pet {
499+
id: ID!
500+
name: String!
501+
}
502+
type Cat implements Pet {
503+
id: ID!
504+
name: String!
505+
canScratch: Boolean!
506+
}
507+
type Dog implements Pet {
508+
id: ID!
509+
name: String!
510+
canBark: Boolean!
511+
}
512+
type Elephant {
513+
id: ID!
514+
}
515+
extend type Elephant implements Pet {
516+
name: String!
517+
hasTrunk: Boolean!
518+
}
519+
520+
union Offer = Discount | Coupon
521+
type Discount {
522+
id: ID!
523+
name: String!
524+
}
525+
type Coupon {
526+
id: ID!
527+
name: String!
528+
}
529+
`),
530+
{
531+
importPath: '../types',
532+
importNamespace: 'core',
533+
encapsulate: 'none',
534+
requireRootResolvers: false,
535+
shouldDeclare: false,
536+
rootTypes: ROOT_TYPES,
537+
baseVisitor,
538+
useGraphQLModules: true,
539+
}
540+
);
541+
542+
// User does not pick `__isTypeOf` because it is not a union member, or implementing types
543+
expect(output).toBeSimilarStringTo(`
544+
export type UserResolvers = Pick<core.UserResolvers, DefinedFields['User']>;
545+
`);
546+
547+
// Cat picks `__isTypeOf` because it is an implementing type of Pet
548+
expect(output).toBeSimilarStringTo(`
549+
export type CatResolvers = Pick<core.CatResolvers, DefinedFields['Cat'] | '__isTypeOf'>;
550+
`);
551+
// Dog picks `__isTypeOf` because it is an implementing type of Pet
552+
expect(output).toBeSimilarStringTo(`
553+
export type DogResolvers = Pick<core.DogResolvers, DefinedFields['Dog'] | '__isTypeOf'>;
554+
`);
555+
// Elephant picks `__isTypeOf` because it is an implementing type of Pet, via `extend type `
556+
expect(output).toBeSimilarStringTo(`
557+
export type ElephantResolvers = Pick<core.ElephantResolvers, DefinedFields['Elephant'] | '__isTypeOf'>;
558+
`);
559+
560+
// Discount picks `__isTypeOf` because it is a union member
561+
expect(output).toBeSimilarStringTo(`
562+
export type DiscountResolvers = Pick<core.DiscountResolvers, DefinedFields['Discount'] | '__isTypeOf'>;
563+
`);
564+
// Coupon picks `__isTypeOf` because it is a union member
565+
expect(output).toBeSimilarStringTo(`
566+
export type CouponResolvers = Pick<core.CouponResolvers, DefinedFields['Coupon'] | '__isTypeOf'>;
567+
`);
568+
});

packages/presets/graphql-modules/tests/integration.spec.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ describe('Integration', () => {
3434
test('should not duplicate type even if type and extend type are in the same module', async () => {
3535
const { result } = await executeCodegen(options);
3636

37-
const userResolversStr = `export type UserResolvers = Pick<Types.UserResolvers, DefinedFields['User'] | '__isTypeOf'>;`;
37+
const userResolversStr = `export type UserResolvers = Pick<Types.UserResolvers, DefinedFields['User']>;`;
3838
const nbOfTimeUserResolverFound = result[4].content.split(userResolversStr).length - 1;
3939

4040
expect(nbOfTimeUserResolverFound).toBe(1);
@@ -176,7 +176,7 @@ describe('Integration', () => {
176176

177177
// Only Query related properties should be required
178178
expect(usersModuleOutput.content).toBeSimilarStringTo(`
179-
export type UserResolvers = Pick<Types.UserResolvers, DefinedFields['User'] | '__isTypeOf'>;
179+
export type UserResolvers = Pick<Types.UserResolvers, DefinedFields['User']>;
180180
export type QueryResolvers = Required<Pick<Types.QueryResolvers, DefinedFields['Query']>>;
181181
`);
182182
expect(usersModuleOutput.content).toBeSimilarStringTo(`

0 commit comments

Comments
 (0)