Describe the bug
Upgraded from 1.16.3 to 1.17.7 and all resolvers resolving a federated type broke.
To Reproduce
Steps to reproduce the behavior:
Attempt to compile TypeScript files using generated types and resolvers.
Schema from service 1
extend type Query {
getRoomType(id: String!): RoomType
getRoomTypes(destination: String!): [RoomType]
}
type RoomType @key(fields: "id") @key(fields: "operaCode operaPropertyCode") {
propertyId: String!
operaCode: String!
property: Property!
id: String!
name: String!
active: Boolean
operaPropertyCode: String @deprecated(reason: "Not intended for client use.")
}
extend type Property @key(fields: "id") {
id: String! @external
roomTypes: [RoomType!]
}
Schema from service 2
extend type Query {
getProperty(id: String!): Property
getProperties(destination: String = "all"): [Property!]!
}
type Property @key(fields: "id") @key(fields: "operaPropertyCode operaRoomTypeCode") {
id: String!
propertyId: String!
path: String!
detailPageBrandURL: String
detailPageCorpURL: String
operaPropertyCode: String
name: String!
images: Images
shortName: String
region: String!
regionName: String
timezone: String
websiteUrl: String
emailAddress: String
phoneNumber: PropertyPhoneNumber!
propertyAddress: PropertyAddress!
digitalKeyVendor: String @deprecated(reason: "digitalKeyVendor is now at the room level instead of property.")
operaRoomTypeCode: String @deprecated(reason: "Not intended for client use.")
}
type PropertyPhoneNumber {
generalLabel: String
generalNumber: String
reservationsLabel: String
reservationsNumber: String
additionalPhone1Label: String
additionalPhone1Number: String
cabanasNumber: String
}
type PropertyAddress {
streetAddress: String
addressLocality: String
addressRegion: String
postalCode: String
addressCountry: String
}
N/A. Issue is on TypeScript compilation
codegen.yml config file:generates:
# Property Service
packages/gql-types/src/schema/property/schemaTypes.generated.ts:
schema:
- ./services/property/src/schema.ts
config:
federation: true
typesPrefix: PropertySchema
useIndexSignature: true
noSchemaStitching: true
defaultMapper: Partial<{T}>
mappers:
Property: ../../content/index#PropertyWithRoomTypeKey
plugins:
- typescript
- typescript-resolvers
# Room type services
packages/gql-types/src/schema/room-type/schemaTypes.generated.ts:
schema:
- ./services/room-type/src/schema.ts
config:
federation: true
typesPrefix: RoomTypeSchema
useIndexSignature: true
noSchemaStitching: true
defaultMapper: Partial<{T}>
mappers:
Property: ../stubs#PropertyStub
plugins:
- typescript
- typescript-resolvers
hooks:
afterAllFileWrite:
- prettier --write
Expected behavior
TypeScript files should compile as they did with version 1.16.3
Environment:
@graphql-codegen/...: 1.17.7Additional context
Example resolver causing problems for service with schema 1:
import { RoomTypeSchemaResolvers } from ''; //generated types file
export const resolvers: RoomTypeSchemaResolvers<any> = {
Query: {
RoomType: {
property(roomType) {
return { __typename: 'Property', id: roomType.propertyId };
}
}
};
Error:
Property 'propertyId' does not exist on type '({ __typename: "RoomType"; } & GraphQLRecursivePick<Partial<Pick<RoomTypeSchemaRoomType, "__typename" | "propertyId" | "operaCode" | "id" | "name" | "images" | "active" | "attributes" | "operaPropertyCode"> & { ...; }>, { ...; }>) | ({ ...; } & GraphQLRecursivePick<...>)'.
Property 'propertyId' does not exist on type '{ __typename: "RoomType"; } & GraphQLRecursivePick<Partial<Pick<RoomTypeSchemaRoomType, "__typename" | "propertyId" | "operaCode" | "id" | "name" | "images" | "active" | "attributes" | "operaPropertyCode"> & { ...; }>, { ...; }>'.
interfaces for mappers:
export type PropertyWithRoomTypeKey = Property & {
operaRoomTypeCode?: string;
};
export interface Property {
path: string;
detailPageCorpURL: string;
detailPageBrandURL: string;
propertyId: string;
id: string;
name: string;
location: Location;
shortName: string;
region: string;
regionName: string;
propertyAddress: PropertyAddress;
timezone: string;
websiteUrl: string;
emailAddress: string;
emailContactInfo: string;
phoneNumber: PropertyPhoneNumber;
corporateSortOrder: string;
termsAndConditions: string;
checkinTime: string;
checkoutTime: string;
petsAllowed: boolean;
smokingAllowed: boolean;
operaPropertyCode: string;
reopenDate: string;
digitalKeyVendor: string;
contentType: string;
}
export interface PropertyAddress {
streetAddress: string;
addressLocality: string;
addressRegion: string;
postalCode: string;
addressCountry: string;
}
export interface PropertyPhoneNumber {
generalLabel: string;
generalNumber: string;
reservationsLabel: string;
reservationsNumber: string;
additionalPhone1Label: string;
additionalPhone1Number: string;
additionalPhone2Label: string;
additionalPhone2Number: string;
cabanasNumber: string;
}
export interface PropertyStub {
__typename: string;
id?: string;
operaPropertyCode?: string;
operaRoomTypeCode?: string;
}
https://github.com/dotansimha/graphql-code-generator/pull/4232 seems to be the PR that broke my situation
We have the same issue, coming from 1.17.4
Same issue here, had to revert to v1.17.3 until this issue is solved. IMO https://github.com/dotansimha/graphql-code-generator/pull/4232 needs to be reverted and fixed to only apply the @key logic in extended types from other services, I'm not sure if that's possible tho
@jakeblaxon any idea why it happens? it seems like the changes we recently merged in https://github.com/dotansimha/graphql-code-generator/pull/4232 causes it.
It looks like @DouglasGabr pinpointed the issue. The requires/key logic should only apply to extended parent types (i.e. types from external schemas), but it's being applied to non-extended types as well. This is because the if statement in the transformParentType method in packages/utils/plugins-helpers/src/federation.ts only checks if the parent type contains a key directive. @dotansimha do you know of an easy way to check if the parent type is extended? I ran a quick test to get the kind of the ast node passed in to this method, but it looks like it's always ObjectTypeDefinition and never ObjectTypeExtension. Once we have this info, we just need to include it in the if statement and that should solve the issue.
I've been looking into how to solve this. It seems that GraphQL doesn't preserve the extend keyword during introspection, so by the time we have access to the schema in the federation module, it's impossible to tell which types were originally extended (even schemas loaded from files since they pass through the printSchema and parse graphql-js methods several times beforehand).
Because of this, the simplest solution I can think of would be to check if any of the type's fields contain an @external directive. I believe that this is logically equivalent, since base types can't contain external fields and extended types must have at least one external field for the key. I'll open up a PR with this fix soon, but I wanted to share this idea in case anyone sees a potential problem with it. Let me know if you have any concerns. Thanks!
I think it solves the problem, relying on the @external directive seems to be a good way to distinguish between base and extended types, I can't think of any scenario this approach wouldn't work
Fixed in https://github.com/dotansimha/graphql-code-generator/pull/4542 by @jakeblaxon - thanks!
The fix is available in @graphql-codegen/[email protected]
I suppose that the RoomType interface in service 1 is essentially:
interface RoomType {
id: string;
propertyId: string;
...
}
and that service 2 has a __resolveReference for Property like:
export const resolvers = {
Property: {
__resolveReference: (keys, _args, { propertyRepository }) => propertyRepository.tryGet(keys.id),
...
},
...
};
@rherber can you confirm this?