TypeScript Resolvers - Apollo Federation Broken

[rherber]

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.

1. My GraphQL schema:

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
  }

2. My GraphQL operations:

N/A. Issue is on TypeScript compilation

3. My 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:

• OS: Windows 10
• @graphql-codegen/...: 1.17.7
• NodeJS: 12.16.3
• TypeScript 3.9.6
• Apollo gateway/federation: 0.19.0
• graphql: 15.3.0

Additional 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;
}

[rherber]

#4232 seems to be the PR that broke my situation

[zeidlos]

We have the same issue, coming from 1.17.4

[DouglasGabr]

Same issue here, had to revert to v1.17.3 until this issue is solved. IMO #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

[dotansimha]

@jakeblaxon any idea why it happens? it seems like the changes we recently merged in #4232 causes it.

[jakeblaxon]

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.

[jakeblaxon]

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!

[DouglasGabr]

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

[dotansimha]

Fixed in #4542 by @jakeblaxon - thanks!
The fix is available in @graphql-codegen/typescript-resolvers@1.17.8

[KoltesDigital]

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?