Inherited fields lost & types orphaned when implementing an interface

[russell-dot-js]

Describe the Bug
Hi @MichalLytek, great to see you contributing to the project again. Can't wait to get the next version!

• When implementing a simple interface, the implementations are not included in the schema if a resolver returns the base interface.
• When a type implements an interface that implements another interface, the field definitions of the root interface are not included in the schema of that type.

To Reproduce
https://github.com/kingsmendv/type-graphql-issue-multi-level-interface
See the generated schema.gql

Expected Behavior

• SuperSuperExample should be in the schema
• the "id" field from Example should be included in the definition of SuperDuperExample

Logs
N/A

Environment (please complete the following information):

• OS: [e.g. Windows]: macOs
• Node (e.g. 10.5.0): 14.17.3
• Package version [e.g. 0.12.2] (please check if the bug still exist in newest release): 1.1.1
• TypeScript version (e.g. 2.8.2): 4.9.5

Additional Context
This is NOT the same as #726 or #907 but may be related to #373

One way to get the expected behavior is by duplicating the interface-inherits-interface definition on all your types. E.g. if you change @ObjectType({ implements: SuperExample }) to @ObjectType({ implements: [Example, SuperExample] }) the schema will emit properly.

[MichalLytek]

When implementing a simple interface, the implementations are not included in the schema if a resolver returns the base interface.

I think that was fixed some time ago. Can you try on 2.0.0-beta.1?

[russell-dot-js]

When implementing a simple interface, the implementations are not included in the schema if a resolver returns the base interface.

I think that was fixed some time ago. Can you try on 2.0.0-beta.1?

Interesting, I'll check it out! Where are you tracking these beta releases? There hasn't been a tag since 1.2.0-rc.1
https://github.com/MichalLytek/type-graphql/releases

[russell-dot-js]

@MichalLytek I tried again with the beta branch and got the same result:
kingsmendv/type-graphql-issue-multi-level-interface@7b0b9ab

[MichalLytek]

@russell-dot-js Sync version of build schema skips the schema check (which is async) thus may lead to generating invalid schema.

If I change your example repository code into:

async function main() {
  await buildSchema({
    resolvers: [QueryResolver],
    emitSchemaFile: true,
  });
}

main().catch(console.error);

Then I get this error:

GeneratingSchemaError: Some errors occurred while generating GraphQL schema:
  Type SuperDuperExample must implement Example because it is implemented by SuperExample.
,  Interface field SuperExample.id expected but SuperDuperExample does not provide it.
Please check the `details` property of the error to get more detailed info.

So if I fix your classes and decorators:

@ObjectType({ implements: [SuperExample, Example] })
export class SuperDuperExample extends SuperExample {

Then the generated schema file sounds looks correct:

One way to get the expected behavior is by duplicating the interface-inherits-interface definition on all your types. E.g. if you change @ObjectType({ implements: SuperExample }) to @ObjectType({ implements: [Example, SuperExample] }) the schema will emit properly.

The rules about interfaces are GraphQL spec rules. I just follow with the requirement of manually specifying all the interfaces in the inheritance chain.

[MichalLytek]

When implementing a simple interface, the implementations are not included in the schema if a resolver returns the base interface.

That's again a wrong schema design in your example repository.

Remember that TypeGraphQL strip off all the not used types and interfaces.
It's not capable of detecting new XYZ() return statements.

Basically, there's an auto include types implementing interface mechanism.
However, in your case your resolvers returns Example interface.
The Example interface is not implemented by any other type, because you have forgot about the requirement of specifing all the interfaces.

So when you type this code:

@InterfaceType()
export abstract class Example {
  @Field(() => String)
  id!: string;
}

@InterfaceType({ implements: Example })
export abstract class SuperExample extends Example {
  @Field(() => String)
  somethingElse!: string;
}

@ObjectType({ implements: SuperExample })
export class SuperSuperExample extends SuperExample {}

TypeGraphQL does not see SuperSuperExample type because it's not implementing Example which is the only object type referenced in decorators in resolver. That's why the schema looks like this:

interface Example {
  id: String!
}

type Query {
  getExample: Example!
}

So if you update your classes:

@ObjectType({ implements: [SuperExample, Example] })
export class SuperSuperExample extends SuperExample {}

It generates the schema as you wanted to 😄

interface Example {
  id: String!
}

type Query {
  getExample: Example!
}

interface SuperExample implements Example {
  id: String!
  somethingElse: String!
}

type SuperSuperExample implements Example & SuperExample {
  id: String!
  somethingElse: String!
}

[russell-dot-js]

Thank you for the thorough response @MichalLytek - I especially appreciate you pointing out that that synchronous schema generation skips validation, I wasn't aware of that!

I understand that the graphql spec specifies that all implemented interfaces must be specified, but this seems like something type-graphql should abstract out for us. Since we aren't writing graphql types directly, rather, we are essentially using OOP to define our types. In OOP, when C implements B, and B implements A, C implicitly implements A. Having to duplicate the knowledge of the entire inheritance chain not only leads to somewhat-spaghetti code, but also reduces or even eliminates the value of having interface B to begin with.

Since we are writing classes to define our types, my intuition was that type-graphql would understand that:

@InterfaceType()
abstract class A {
  @Field(() => String)
  id!: string;
}

@InterfaceType({ implements: A })
abstract class B extends A {
  @Field(() => String)
  otherField!: string;
}

@ObjectType({ implements: B })
class C extends B {}

should compile to:

interface A {
  id: String!
}

interface B implements A {
  id: String!
  otherField: String!
}

type C implements B & A {
  id: String!
  otherField: String!
}

We should have all of this knowledge at runtime while generating the schema - we know that C implements B and B implements A - so what is the advantage of forcing this nuance of graphql onto the developer? I understand that this is a graphql library, but if we wanted to write graphql schemas instead of typescript, we wouldn't be using type-graphql

FYI this is particular painful when you have a large # of types related to a workflow - as the object moves through the workflow and its status changes, additional fields are added. This is where it bit me in particular, the maintenance of that code is a headache but only because the traditional rules of OOP are not followed

[MichalLytek]

You're absolutely right. I've created test snippets based on your example to make sure all works ok 💪

Closing as fixed by 84e7033 🔒