TypeError in validateTypeImplementsInterface

[MichalLytek]

In TypeGraphQL I have a test case for throwing an error when object type field type doesn't match with the interface one, like:

interface SampleInterface {
  sampleField: String!
}
type SampleObject implements SampleInterface {
  sampleField: Int!
}

After upgrading to v15.0.0, it now throws this error:

TypeError: Cannot read property 'type' of undefined
          at validateTypeImplementsInterface (D:\#Projekty\type-graphql\node_modules\graphql\type\validate.js:296:298)
          at validateInterfaces (D:\#Projekty\type-graphql\node_modules\graphql\type\validate.js:276:5)
          at validateTypes (D:\#Projekty\type-graphql\node_modules\graphql\type\validate.js:198:7)
          at validateSchema (D:\#Projekty\type-graphql\node_modules\graphql\type\validate.js:54:3)
          at graphqlImpl (D:\#Projekty\type-graphql\node_modules\graphql\graphql.js:79:62)
          at D:\#Projekty\type-graphql\node_modules\graphql\graphql.js:28:59
          at new Promise (<anonymous>)
          at Object.graphql (D:\#Projekty\type-graphql\node_modules\graphql\graphql.js:26:10)
          at Function.generateFromMetadata (D:\#Projekty\type-graphql\src\schema\schema-generator.ts:111:32)
          at Object.buildSchema (D:\#Projekty\type-graphql\src\utils\buildSchema.ts:31:40)

What I am doing to validate that is to execute the introspection query against the built schema.

In v14.x it was throwing a normal GraphQLError with a detailed info about this invalid interface implementation.

[MichalLytek]

@IvanGoncharov anything I could help you with solving this issue? it's blocking me with releasing new version 😕

[josephktcheung]

Hi @MichalLytek @IvanGoncharov I've traced the error and found that it's because of this change in graphql-js 51e3f14#diff-661d5fd66a200ef8826c539915861a7bR357 where
ifaceField.astNode?.type is changed to ifaceField.astNode.type.

In type-graphql astNode is not given to GraphqlQLInterfaceType https://github.com/MichalLytek/type-graphql/blob/master/src/schema/schema-generator.ts#L374-L418, therefore ifaceField.astNode is undefined hence ifaceField.astNode.type throws Cannot read property 'type' of undefined

So the question is do we need astNode to be defined for context.reportError. @IvanGoncharov perhaps you can help explain your change from ifaceField.astNode?.type to ifaceField.astNode.type so that we know which library has to be updated.

Solution 1: graphql-js reverts change 51e3f14#diff-661d5fd66a200ef8826c539915861a7bR357

Solution 2: type-graphql generates astNode when it generates graphql types

I notice that type-graphql's astNode generation methods return undefined when there's no directiveMetadata and there's a FIXME: use proper AST representation. So my assumption is that type-graphql has planned to generate astNode, just that it's not completed yet.

[MichalLytek]

@josephktcheung
TypeGraphQL uses raw graphql-js constructs, like new GraphQLInterfaceType() and provides the config fields to generate the type.

astNode is only a crazy workaround for enabling directives in code-first approach, so they should be optional or generated by graphql-js under the hood if no astNode is provided.

At least that's how it was working in v14 - when no astNode provided, no ast available in runtime on those types.

[josephktcheung]

@MichalLytek I agree. From definition astNode is a Maybe type, also output of context.reportError is GraphQLError and GraphQLError's nodes can be undefined. Therefore I think that solution 1 is more correct and I've created a PR to fix it #2513

While we wait for graphql-js team to review it, in order to release type-graphql without the need of waiting for a new minor version of graphql-js to be published to npm (which may take a while), we may adopt solution 2 temporarily and return astNode for generated GraphQLTypes as a workaround of this bug until the new version is published. Just my 2 cents. I'm a user of type-graphql and I'm too waiting for the new release :)