Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for __typename #181

Open
MichalLytek opened this issue Oct 22, 2018 · 5 comments
Open

Support for __typename #181

MichalLytek opened this issue Oct 22, 2018 · 5 comments
Assignees
Labels
Enhancement 🆕 New feature or request
Projects
Milestone

Comments

@MichalLytek
Copy link
Owner

MichalLytek commented Oct 22, 2018

Right now, for interfaces and unions, TypeGraphQL determines the type of the returned value using foo instanceof Foo. This force to use class instances in every case where interfaces or unions are returned.

But this might be problematic in some cases, so it could just fallback to the __typename based detection for the easier cooperation with plain objects.

Side note

Also resolveType implementation in unions could be omitted as:

Optionally provide a custom type resolver function. If one is not provided, the default implementation will call isTypeOf on each implementing Object type.

Or maybe even better, for better compatibility, it should allow providing own resolveType function in interface and union decorators/creators.

@MichalLytek MichalLytek added the Enhancement 🆕 New feature or request label Oct 22, 2018
@MichalLytek MichalLytek added this to the 1.0.0 release milestone Oct 22, 2018
@MichalLytek MichalLytek self-assigned this Oct 22, 2018
@MichalLytek MichalLytek added this to Backlog in Board via automation Oct 22, 2018
@MichalLytek MichalLytek moved this from Backlog to To Do in Board Oct 24, 2018
@luketanner
Copy link

First of all, thanks @19majkel94 for adding this to your to-do list, it's a much-needed feature for our team. :)

Just chiming in to vocalise my support for both of the options you suggested: being able to set the __typename field to inform TypeGraphQL of the concrete type of an interface/union would be sufficient for our needs, but your second suggestion of allowing us to provide a resolveType function to the decorators/creators directly would be absolutely amazing!

@maplesteve
Copy link

maplesteve commented Mar 21, 2019

@19majkel94 Until this new feature arrives: how would I provide the type in the current version?

From your initial comment I would assume, that TypeGraphQL can do this somehow magically by itself, but I get the error Abstract type MobileNumberCPS must resolve to an Object type at runtime for field... in my implementation. I tried to provide __resolveType and __isTypeOf function that return the appropriate class name as string, but they don't get called.

This force to use class instances in every case where interfaces or unions are returned.

I don't understand this. Are you referring to the inner workings of TypeGraphQL or does my implementation need to provide a property or something in the class?

What do I need to do, that foo instanceof Foo returns the correct and expected value?
Would be very grateful for any hint!

@MichalLytek
Copy link
Owner Author

@maplesteve

// where interfaces or unions are returned.
@Query(returns => MyInterface)
myInterface() {
  // use class instances 
  const response = new MyObjectType();
  response.sampleField = "sampleFieldValue";
  return response;
}

@apiel
Copy link

apiel commented Mar 26, 2019

@19majkel94 any estimation of implementation?

I tried to use your alternative without success

import { Field, ID, ObjectType } from 'type-graphql';

@ObjectType()
export class ServiceProvider {
    @Field(() => ID)
    id!: string;

    @Field()
    name!: string;

    @Field()
    address!: string;
}

@ObjectType()
export class ServiceProviderA extends ServiceProvider {}

@ObjectType()
export class ServiceProviderB extends ServiceProvider {}

Here is my resolver (using nestjs):

@Resolver(ServiceProvider)
export class ServiceProviderResolvers {
    constructor(private readonly searchService: SearchService) {}

    @Query(() => [ServiceProvider])
    async all(): Promise<(ServiceProviderA | ServiceProviderB)[]> {
        const serviceProviders = await this.searchService.all();
        const result = serviceProviders.map((serviceProvider) => {
            if (serviceProvider.type === 'A') {
                return Object.assign(new ServiceProviderA(), serviceProvider);
            }
            return Object.assign(new ServiceProviderB(), serviceProvider);
        });
        return result;
    }

    // Before using type-graphql with nestjs this was working:
    // @ResolveProperty('__resolveType')
    // resolveType(obj: ServiceProvider): string {
    //     return obj.type;
    // }
}

I still get ServiceProvider as __typename instead of ServiceProviderA or ServiceProviderB.

Actually what would be super cool is to be able to do:

@ObjectType()
export class ServiceProvider {
    @Field(() => ID)
    id!: string;

    @Field()
    name!: string;

    @Field()
    address!: string;

    @TypeName() // this decorator would say that it has to use this value as __typename
    type!: string;
}

@MichalLytek
Copy link
Owner Author

MichalLytek commented Mar 26, 2019

@TypeName() // this decorator would say that it has to use this value as __typename

That's a great idea, you can reuse the property data instead of forcing to use spread operator and __typename property of a new object.

Here is my resolver (using nestjs):

Nest has it's own runtime for resolving types and other things - TypeGraphQL is used only for SDL generation and nothing else.

@MichalLytek MichalLytek moved this from To Do to In progress in Board Apr 4, 2019
@MichalLytek MichalLytek moved this from In progress to To Do in Board Sep 5, 2019
EndyKaufman pushed a commit to EndyKaufman/typegraphql-prisma-nestjs that referenced this issue Feb 9, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Enhancement 🆕 New feature or request
Projects
Board
  
To Do
Development

No branches or pull requests

4 participants