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

ResolveProperty('__resolveType') not working with type-graphql #178

Closed
apiel opened this issue Mar 26, 2019 · 26 comments
Closed

ResolveProperty('__resolveType') not working with type-graphql #178

apiel opened this issue Mar 26, 2019 · 26 comments
Labels

Comments

@apiel
Copy link

apiel commented Mar 26, 2019

The last few days I migrated our project to use type-graphql with nest. I managed to migrate everything, but I am still stuck on one thing. Providing a custom __typename doesn't seem to work.

Here is my resolver:

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

    @Query(() => [ServiceProvider])
    all(): Promise<ServiceProvider[]> {
        return this.searchService.all();
    }

    @ResolveProperty('__resolveType')
    resolveType(obj: ServiceProvider): string {
         return obj.type;
    }

    @ResolveProperty()
    test(obj: ServiceProvider): string {
         return obj.type;
    }
}

But the resolveType doesn't seem to be called, where test method is called if I query for test field.

I also tried to use some interface like suggested 19majkel94 (from type-graphql) but still doesn't work: MichalLytek/type-graphql#181 (comment)

Is there any way to customize __typename with nest using type-graphql?

@kamilmysliwiec
Copy link
Member

Please, provide a minimal repository which reproduces your issue. :)

@apiel
Copy link
Author

apiel commented Mar 27, 2019

I made a repo with an example base on your 23-type-graphql sample https://github.com/apiel/nest-eg

Here is the request:

{
  recipes {
    id
    title
    __typename
    type
  }
}

I would like that __typename equal abc instead of Recipe.

@kamilmysliwiec
Copy link
Member

kamilmysliwiec commented Mar 27, 2019

Why do you want to change __typename? + I don't think that __resolveType can help you with this.

@kamilmysliwiec kamilmysliwiec transferred this issue from nestjs/nest Mar 27, 2019
@apiel
Copy link
Author

apiel commented Mar 27, 2019

We make request on our elasticsearch database with an index that share different types of document. We need to identify which kind of document we are dealing with in our graphql schema. For example you would have an elasticsearch database with car and truck.

19majkel94 explain it pretty well in his issue MichalLytek/type-graphql#181
Also see https://www.apollographql.com/docs/apollo-server/features/unions-interfaces#union-type

__resolveType was working before we migrate to type-graphql

@MichalLytek
Copy link

@apiel
__typename means type name - __typename of type Recipe = Recipe and you can't change that.

You can only provide a custom __resolveType function that will return a hint for graphql-runtime about the underlying type of the runtime data you are returning, like generic interface Person and specific type Student.

It may read __typename property value of data object (default behavior), check object prototype/instance of or use the type property like in your example (return obj.type).

@apiel
Copy link
Author

apiel commented Mar 28, 2019

Hmm, I think I don't manage to explain myself. But I created another small repo making the wished behavior without type-graphql: https://github.com/apiel/nest-eg2

{
  getCats {
    id
    name
    __typename
  }
}

I get the wished response:

{
  "data": {
    "getCats": [
      {
        "id": 1,
        "name": "Cat",
        "__typename": "CatA"
      }
    ]
  }
}

See the cats.graphql

interface Cat {
  id: Int
  name: String
  age: Int
}

type CatA implements Cat {
  id: Int
  name: String
  age: Int
}

And finally the resolver:

  @ResolveProperty('__resolveType')
  resolveType(): string {
      return 'CatA';
  }

How can I do the same with nest using type-graphql?


I also tried to copy Recipe in another instance of RecipeA without success https://github.com/apiel/nest-eg/tree/recipeA

@Resolver(of => Recipe)
export class RecipesResolver {
  constructor(private readonly recipesService: RecipesService) {}

  @Query(returns => [Recipe])
  async recipes(@Args() recipesArgs: RecipesArgs): Promise<RecipeA[]> {
    const recipes = await this.recipesService.findAll(recipesArgs);
    return recipes.map(recipe => {
      const recipeA = new RecipeA();
      Object.assign(recipeA, recipe);
      return recipeA;
    });
  }

  @ResolveProperty("__resolveType")
  resolveType(recipe: Recipe): string {
    return "RecipeA";
  }
}

@apiel
Copy link
Author

apiel commented Mar 28, 2019

You are not passing propertyName to the FieldResolver from type-graphql.

FieldResolver && FieldResolver(typeFunc, options)(target, key, descriptor);

export function createPropertyDecorator(
  propertyName?: string,
  typeFunc?: ReturnTypeFunc,
  options?: AdvancedOptions,
): MethodDecorator {
  return (
    target: Function | Object,
    key?: string | symbol,
    descriptor?: any,
  ) => {
    SetMetadata(RESOLVER_NAME_METADATA, propertyName)(target, key, descriptor);
    SetMetadata(RESOLVER_PROPERTY_METADATA, true)(target, key, descriptor);
    FieldResolver && FieldResolver(typeFunc, options)(target, key, descriptor);
  };
}

I don't know if this can help?

@kamilmysliwiec
Copy link
Member

@apiel so I took a look at your examples and they are different, hence, the behavior is different as well.

Also you are not passing propertyName to the FieldResolver from type-graphql.

We don't have to - Nest is using type-graphql only to generate GraphQL definitions.

@apiel
Copy link
Author

apiel commented Mar 28, 2019

How to achieve https://github.com/apiel/nest-eg2 with type-graphql?

@kamilmysliwiec
Copy link
Member

First of all, you would have to use interfaces (not types) as in https://github.com/apiel/nest-eg2 (Cat interface)

@apiel
Copy link
Author

apiel commented Mar 28, 2019

But how can I use interface with nest and type-graphql? Do you have any example?

@kamilmysliwiec
Copy link
Member

See https://typegraphql.ml/docs/interfaces.html

@apiel
Copy link
Author

apiel commented Mar 28, 2019

Hey super, this seem to be what I need ;-) Then I guess it might work with:

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

I will try tomorrow and then close the issue. Thx

@apiel
Copy link
Author

apiel commented Mar 29, 2019

Still having issue:

@InterfaceType()
export abstract class Recipe {
  @Field(type => ID)
  id: string;

  @Field()
  title: string;

  @Field({ nullable: true })
  description?: string;

  @Field()
  creationDate: Date;

  @Field(type => [String])
  ingredients: string[];
}

@ObjectType({ implements: Recipe })
export class RecipeA implements Recipe {
  id: string;
  title: string;
  description: string;
  creationDate: Date;
  ingredients: string[];
}
@Resolver(of => Recipe)
export class RecipesResolver {
  constructor(private readonly recipesService: RecipesService) {}

  @Query(returns => [Recipe])
  async recipes(@Args() recipesArgs: RecipesArgs): Promise<Recipe[]> {
    return this.recipesService.findAll(recipesArgs);
  }

  @ResolveProperty('__resolveType')
  resolveType(recipe: Recipe): string {
    return 'RecipeA';
  }
}

It's not compiling because @Resolver(of => Recipe) has typing issue: Type 'typeof Recipe' is not assignable to type 'Type<any>'. Cannot assign an abstract constructor type to a non-abstract constructor type.

If I remove abstract on the class Recipe it compile but fail on runtime with the error UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'fields' of undefined.

If I change resolver to @Resolver('Recipe'), I get the runtime error UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'getObjectType' of undefined

https://github.com/apiel/nest-eg/tree/recipeA

@MichalLytek
Copy link

Also you are not passing propertyName to the FieldResolver from type-graphql.

We don't have to - Nest is using type-graphql only to generate GraphQL definitions.

Actually, we have to. According to TypeGraphQL docs:

Note that if a field name of a field resolver doesn't exist in resolver object type, it will create in schema a field with this name. This feature is useful when the field is purely calculable (eg. averageRating from ratings array) and you don't want to pollute the class signature.

So @apiel code will produce a Recipe type with id, title, ... and resolveType: String!.
Changing the name to __resolveType I guess will result with schema generation error as graphql-js doesn't allow to create fields with name starting with an underscore.

There is a pending TypeGraphQL issue about that, as by default it uses instance of checks to resolve the type that might be not convenient when the 3rd party library like Typegoose doesn't use class instances.
MichalLytek/type-graphql#181

@apiel
Copy link
Author

apiel commented Mar 29, 2019

@19majkel94 thanks for the clarification

@iangregsondev
Copy link

@kamilmysliwiec I am also having the same issue, can you confirm its an issue with the graphql nestjs implementation ?

Being going around crazy for the last few hours :-) I just can't get a simple example to work.

@michelcve
Copy link

@kamilmysliwiec I'm also having the same problems as @apiel. No matter what I do, I can't get the @ResolveProperty to work. As soon as I use it, the server 'hangs'. When inspecting with debugger, I see the same error as @apiel : UnhandledPromiseRejectionWarning: TypeError: Cannot read property 'fields' of undefined.

@kamilmysliwiec
Copy link
Member

Please, create a separate issue + provide a repo that reproduces it @michelcve

@michelcve
Copy link

@kamilmysliwiec While creating a test project, I finally figured it out.

In my tsconfig.json I had target: es5. This works fine, until you start using @ResolveProperty. The resolution for this is switching target to target: es6.

@apiel
Copy link
Author

apiel commented May 12, 2019

Why do we close this issue? Is it solved?

@kamilmysliwiec
Copy link
Member

Let's track this issue in a one place MichalLytek/type-graphql#181

@apiel
Copy link
Author

apiel commented May 12, 2019

But some weeks ago, when I tried to use interface https://typegraphql.ml/docs/interfaces.html, as you suggested, it didn't work. Nest didn't seem to support this feature from type-graphql. Did you fix this? If yes could you provide link of the commit/PR number?

@kamilmysliwiec
Copy link
Member

See:
#231
#203

@apiel
Copy link
Author

apiel commented May 12, 2019

Ah super :D I will give a try tomorrow.

@lock
Copy link

lock bot commented Apr 25, 2020

This thread has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs.

@lock lock bot locked as resolved and limited conversation to collaborators Apr 25, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Projects
None yet
Development

No branches or pull requests

5 participants