MichalLytek / type-graphql

Create GraphQL schema and resolvers with TypeScript, using classes and decorators!
https://typegraphql.com
MIT License
7.99k stars 674 forks source link

Typed decorators - enhanced type-safety #221

Open MichalLytek opened 5 years ago

MichalLytek commented 5 years ago

Right now it's dangerously easy to have inconsistence between schema type and TS type in class definition:

class Test {
  @Field(() => [Author])  // wrong item type
  actors: Actor[];

  @Field({ nullable: true })
  optionalField: string; // optional in schema, required in TS
}

It can be easily fixed (I was not aware of this possibility in TS) by using generics instead of predefined types like PropertyDecorator.

Quick demonstration of the proof of concept:

function Field<T>(typeFn: () => T) {
  return <TPropertyName extends string>(
    prototype: { [P in TPropertyName]: GetSimpleType<T> },
    propertyName: TPropertyName,
  ) => {
    console.log("TypeGraphQL FTW!");
  };
}

type GetSimpleType<T> = T extends BooleanConstructor ? boolean : never;

class Test {
  @Field(() => Boolean)
  okField: boolean;

  @Field(() => Boolean)
  badField: string;
}

image

Argument of type 'Test' is not assignable to parameter of type '{ badField: boolean; }'.
  Types of property 'badField' are incompatible.
    Type 'string' is not assignable to type 'boolean'

TODO: figure out how to get rid of false negative, like using custom scalars, etc.

Disclaimer: Inspired by @stephentuso ts-graphql 😃

Veetaha commented 5 years ago

@19majkel94 Very good feature, but is there a way to provide strong typing support for method parameters decorators? I can't make tsc to issue an error for this code:

export interface ClassType extends Function {
    new (...args: any[]): unknown;
}

export type StaticMethodParameterDecorator = (
    <
        TMethodName  extends string | symbol,
        TClassType   extends ClassType & Record<TMethodName, (this: TClassType, ...args: unknown[]) => unknown>
    >
    (
        targetClass:    TClassType, 
        methodName:     TMethodName, 
        parameterIndex: number
    ) => void
);

function staticParameterDecorator(): StaticMethodParameterDecorator {
    return () => {};
}

class Cl {
    method(            // instance method, but no error
        @staticParameterDecorator()
        bool: boolean
    ) {
        return bool;
    }
}

Did you find a solution?

MichalLytek commented 5 years ago

Static method? Does TypeGraphQL supports this? 😜

You can check out the ts-graphql source code for help: https://github.com/stephentuso/ts-graphql/blob/dev/src/decorators/Field.ts

Veetaha commented 5 years ago

@19majkel94 Well, ok, we can skip this part, as there are no decorators targeted to static accessors/properties/methods in typegraphql. I inspected some of ts-graphql code previously, but failed to find a solution for strongly typing parameter decorators, that you will face when implementing yours if haven't yet.

MichalLytek commented 5 years ago

I think that typed decorators will be superseded by TypeScript transform plugin to enhance reflection system - more info soon 💪

adhamsalama commented 7 months ago

Hello @MichalLytek. I hope all is well. I have just found out about this behavior while using NestJS, and it led me to this GitHub issue, so, I am wondering if there are/will be any updates/solutions to this.