Nested input types are not deserialized into class instances

[adam-stanek]

Describe the bug When having nested input types only the top-most class gets transformed into it's instance. Any nested objects remains plain javascript objects.

To Reproduce

@InputType()
class NestedTestInputType {
  @Field()
  foo: number
}

@InputType()
class TestInputType {
  @Field()
  nested: NestedTestInputType
}

@Service()
@Resolver(of => /* ... */)
export class TestResolver {
   @Query(returns => [String])
  async test(@Arg('filter') filter: TestInputType) {
    assert(filter instanceof TestInputType, 'Filter must be instance of TestInputType') // <-- OK
    assert(
      filter.nested instanceof NestedTestInputType,
      'Filter#nested must be instance of NestedInputType',
    ) // <-- FAILS
    return []
  }
}

Expected behavior Nested input types should be deserialized into instances of their classes recursively.

Enviorment (please complete the following information):

• type-graphql@0.12.3

[MichalLytek]

Unfortunately, I can confirm that. I've added proper test cases for that on the branch.

This bug applies also to arrays and nested inputs inside args type classes. It might also show up in nested @Root objects but it's irrelevant due to the way how graphql is designed and how resolvers are called. And due to this, nested validation also won't work.

At the beginning I've used class-transformer to handle that plain to class transformations but I had some issues with object which fields values were promises. So I had to switch to the simple return Object.assign(new Target(), data); solution that works only on first level.

But looks like even plainToClass is not a solution as we still have to decorate the fields with @Type decorator, otherwise they are stil objects, not instances of the class.

@ArgsType()
class SampleNestedArgs {
  @Field()
  factor: number;

  @Field()
  @Type(() => SampleInput) // without this, `input.constructor` is `Object`
  input: SampleInput;
}

As it applies to input type classes, the solution might be to load the input type class metadata during transformation and check for nested fields and then call it recursive. But using MetadataStorage inside this helpers is not a good idea, I need to find a way how to get this field types info there.

[adam-stanek]

Thank you for such a quick reaction. I am beginning to use your library so I am not yet familiar with all it's internals. What exactly do you see as a problem when using MetadataStorage inside of said helper? From my point of view it seem like an only solution.

[MichalLytek]

Basically, after a dozen of new features current architecture is getting very messy. I've designed it with no problems like this in mind, so it's now the bottleneck.

Generally, now decorators just collect simple metadata and put them in MetadataStorage, then it's "builded" (attaching fields metadata to classes, etc.) and then it's used to generate the schema. There's a lot of null assertion and other code smells. So I have to redesign it to introduce the builded metadata as a separate step with own class that have better designed shape of data.

So after this refactor, I would have access to the info about the input type field in handler's params metadata, that are passed from the schema generator down to the convert type helper. No global access or finding in arrays for data 😉 https://github.com/19majkel94/type-graphql/blob/7c534eac95d24b53cd40929b1101e2a35b893e37/src/resolvers/create.ts#L15-L17 https://github.com/19majkel94/type-graphql/blob/7c534eac95d24b53cd40929b1101e2a35b893e37/src/resolvers/create.ts#L32-L33

So this may be related to #123 as it also rely on the change of the metadata info format.

This would also simplify the schema generation step that now have to looks for super classes metadata and other things.

[DavidBabel]

I fall into this issue too. For me, it is not a problem on the schema generation, and it seems to work but i got this log on each request :

No metadata found. There is more than once class-validator version installed probably. You need to flatten your dependencies

Is it related, and will it be fixed by your current work on this issue ?

[MichalLytek]

See #150 - if you don't use validation, just disable it 😉

[raydirty]

@19majkel94 What is the status here? Do you need any help?

[MichalLytek]

It's generally blocked by #183 - I need to rewrite the args parsing phase to have more metadata info needed to deeply transform the nested object.

[matcic]

Hey @19majkel94, what are the problems you ran into with class-transformer? I understand that the appropriate type could be inferred by storing some metadata when all decorators are built, but like you said by decorating all fields with @Type the problem can at least be solved. Is there any chance you could revert to using class-transformer?

[MichalLytek]

class-transformer crashes when the property is a promise, which is a case of transforming e.g. an object type with TypeORM lazy loading fields: https://github.com/19majkel94/type-graphql/blob/d15d1317065da5256f2eb8b2fd9f2e51bb933497/examples/typeorm-lazy-relations/entities/user.ts#L25-L28

If you can't wait for a proper fix, feel free to fork and replace this line of convertToType function with class-transformer and use @Type decorators to provide type info for it: https://github.com/19majkel94/type-graphql/blob/d15d1317065da5256f2eb8b2fd9f2e51bb933497/src/helpers/types.ts#L97-L99

[matcic]

@19majkel94 this is precisely what I am doing, I guess this is my best option until a proper fix then 😄

[spali]

The workaround does not work if using an abstract class ResourceResolver as in the example https://github.com/19majkel94/type-graphql/blob/d15d1317065da5256f2eb8b2fd9f2e51bb933497/examples/resolvers-inheritance/resource/resource.resolver.ts#L36

The Abstract Resolver class knowns by parameter the Class Type due the given parameter ResourceCls: ClassType so I can use these in getOne and getAll. But for embedded resources I don't know the Classtype of the embedded objects to make it recursive. Note: In getOne and getAll I call a service that queries the database based on GraphQLResolveInfo which returns the native Objects. I have no chance there to get the correct class for embedded objects.

Did I miss something?

[spali]

Would it be possible to make the Class type available in the GraphQLResolveInfo? Then we could get all required information there to build the objects. I.e. in the selectionSet.fieldNodes or returnType property?

[MichalLytek]

@spali Related to #123 - I would rather not extending or modifying GraphQLResolveInfo object.

[spali]

I got your point. I already had to "hack" the GraphQLResolveInfo object due the lack of adding custom metadata in a supported way. I use custom decorators that put's DB related information to it, which I use in the service injected into the Context that I use in the resolver that is inherited by all my Resolvers to get the related objects from the db. To avoid additional db queries I build the queries by "custom joinmonster" which recursively uses the GraphQLResolveInfo. To be honest, I'm struggling here a bit, since I need to return the objects in the specific class I don't know anymore how I should solve this problem without build more and more hacks to get it work again.

Do you have any suggestion which way I should go?

[OniVe]

@19majkel94

But looks like even plainToClass is not a solution as we still have to decorate the fields with @Type decorator, otherwise they are stil objects, not instances of the class.

Why not combine decorators, for example:

import { Type } from "class-transformer";
import { ValidateNested } from "class-validator";
import { ClassType, Field } from "type-graphql";
import {
  AdvancedOptions,
  MethodAndPropDecorator
} from "type-graphql/dist/decorators/types";

export type ReturnTypeFunc = (returns?: undefined) => Function | ClassType;

export function NestedField(
  returnTypeFunction?: ReturnTypeFunc,
  options?: AdvancedOptions
): MethodAndPropDecorator;
export function NestedField(
  returnTypeFunction: ReturnTypeFunc,
  options?: AdvancedOptions
): MethodDecorator | PropertyDecorator {
  const fieldFn = Field(returnTypeFunction, options);
  const typeFn = Type(returnTypeFunction);
  const validateNestedFn = ValidateNested();

  return (target, propertyKey, descriptor) => {
    fieldFn(target, propertyKey, descriptor);
    typeFn(target, propertyKey as string);
    validateNestedFn(target, propertyKey as string);
  };
}

@InputType()
export class NestedTestInputType {
  @Field()
  foo: number
}

@InputType()
export class TestInputType {
  @NestedField((type) => NestedTestInputType)
  nested: NestedTestInputType
}

[rafalpetryka]

Hi! As @19majkel94 said I would like to use workaround with using Type decorator. But what if I have array with nested objects?

@InputType()
export class SomeInput {
  @Field(() => [CourseInput])
  @Type(() => CourseInput)
  public courses?: CourseInput[];
}

I've added Type decorator, but without expecting results (constructor = Object). PS. I tried to remove explicit type from Field, but without success - I got an error 'You need to provide explicit type for SomeInput#courses !'

[bpofficial]

Has there been any progress on this issue? One of my classes relies on a nested array of an object. I have tried the majority of the fixes here although I'm not sure what there is to do. Is there anything that we as the community can do to assist in the completion of this ticket?

[MichalLytek]

@bpofficial For now you can create a fork, modify the convertToType to use class-transformer and the @Type() workaround. But be aware, it fails with Promise, so no TypeORM lazy-loading possible.

[godness84]

I didn't want to fork and maintain my own version. So I cooked a TypedArgs decorator that leverages createParamDecorator in order to replace the method parameter with the deep typed one (using class-transformer). It can be used instead of Args decorator.

Here it is the code, hope it helps, hope @19majkel94 's eyes will not bleed. 😄

import { plainToClass } from "class-transformer";
import { Args, createParamDecorator, SymbolKeysNotSupportedError } from "type-graphql";
import { findType } from "type-graphql/dist/helpers/findType";
import { ValidateOptions, ReturnTypeFunc } from "type-graphql/dist/decorators/types";
import { getTypeDecoratorParams } from "type-graphql/dist/helpers/decorators";
import { validateArg } from "type-graphql/dist/resolvers/validate-arg";

export function TypedArgs(): ParameterDecorator;
export function TypedArgs(options: ValidateOptions): ParameterDecorator;
export function TypedArgs(
  paramTypeFunction: ReturnTypeFunc,
  options?: ValidateOptions,
): ParameterDecorator;
export function TypedArgs(
  paramTypeFnOrOptions?: ReturnTypeFunc | ValidateOptions,
  maybeOptions?: ValidateOptions,
): ParameterDecorator {
  const { options, returnTypeFunc } = getTypeDecoratorParams(paramTypeFnOrOptions, maybeOptions);

  return (prototype, propertyKey, parameterIndex) => {
    if (typeof propertyKey === "symbol") {
      throw new SymbolKeysNotSupportedError();
    }

    const { getType } = findType({
      metadataKey: 'design:paramtypes',
      prototype,
      propertyKey,
      parameterIndex,
      returnTypeFunc,
    })

    const paramDecoratorFunc = createParamDecorator(async ({ args }) => {
      const typedArgs = plainToClass(getType() as any, args)
      return await validateArg(typedArgs, true, options.validate)
    })

    const argsOptions = { ...options, validate: false }
    const argsFunc = returnTypeFunc ? Args(returnTypeFunc, argsOptions) : Args(argsOptions)

    paramDecoratorFunc(prototype, propertyKey, parameterIndex)
    argsFunc(prototype, propertyKey, parameterIndex)
  };
}

[kalyuk]

maybe this will help someone, i just created decorator

import { plainToClass, ValidateNested } from 'class-validator';
import { Field } from 'type-graphql';

export function Nested() {
    return (target: any, propertyName: string) => {
        const Ctx = Reflect.getMetadata('design:type', target.constructor.prototype, propertyName);

        Field(() => Ctx)(target, propertyName);
        ValidateNested()(target, propertyName);

        Object.defineProperty(target.constructor.prototype, propertyName, {
            get() {
                return this[`__${propertyName}`];
            },
            set(value: any) {
                this[`__${propertyName}`] = plainToClass(Ctx, value);
            }
        });
    };
}

@InputType()
export class UserActivateDTO {
    @Length(31, 32)
    @Field()
    public hash: string;

    @Length(6, 256)
    @Field()
    public password: string;

    @IsDateString()
    @Field()
    public bDay: string;

    @Field()
    @IsEmail()
    public email: string;

    @Nested()
    public restore: UserRestoreDTO
}

[j]

@MichalLytek this is kind of disappointing... so many people are probably using class validations with nested inputs expecting them to work. After a year of use in production, I just found out that a lot of internal apps don't ever validate nested inputs.... My typings are completely broken as well because nested inputs aren't actually what I would expect them to be.

[chrisdostert]

@j i know you might not intend it but your comment comes off as a little chippy. Keep in mind this is a free open source project @michaellytek has given us and i for one am very thankful for all he’s done. If we’re nice to him maybe he’ll keep it up. Try to be positive and contribute a fix if possible.

[j]

@chrisdostert Not meaning to come off chippy. In regards to contributing a fix, I've submitted a few PR's to this library as we use it to to employee a handful of people and do quite a bit of revenue using this library. In fact, before your comment, I submitted a fix for this. (https://github.com/MichalLytek/type-graphql/pull/452) We'd be the first to sponsor this project when profits start growing. I could have phrased things differently, but in the end of the day, it sucks giving trust to a project and realizing it doesn't work as you think it would.

We use nested inputs for "patch" updates, and just realized that validations don't run. Users on our app can do any edit mutation with invalid data. We unit test validations separately have expected them to run correctly in type-graphql b/c that's what's documented (although nested input examples don't exist). 😛

Anyway, I'm willing to contribute to this feature. https://github.com/MichalLytek/type-graphql/pull/452

Sorry @MichalLytek if I came off chippy to you. You come off chippy to me 99% of your comments to me ;) haha. But I appreciate this library 1000%. <3

[MichalLytek]

@j I might be chippy because I'm irritated that I have so little time and so much things to do, so I am painfully honest and direct 😕

And I also appreciate your help with PRs and nice feature requests 😉

We'd be the first to sponsor this project when profits start growing.

I hope so ❤️ I dream about being able to work half-time on TypeGraphQL without financial losses 😄

[MichalLytek]

Fixed by #462 - you can check by installing 0.18.0-beta.5 - npm i type-graphql@beta 🎉