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

MethodDecorator gets TS2322/TS2315 #17936

Open
Norgerman opened this issue Aug 21, 2017 · 9 comments
Open

MethodDecorator gets TS2322/TS2315 #17936

Norgerman opened this issue Aug 21, 2017 · 9 comments
Labels
Bug A bug in TypeScript Domain: Decorators The issue relates to the decorator syntax
Milestone

Comments

@Norgerman
Copy link

TypeScript Version: 2.4.2
Code

export function route(method: string, path: string): MethodDecorator {
    return (target: any, name: string, descriptor: TypedPropertyDescriptor<Function>) => {
        routeManager.regeisterRoute({
            constructor: target.constructor,
            function: descriptor.value,
            method,
            path
        });
    }
}
export function route(method: string, path: string): MethodDecorator<Function> {
    return (target: any, name: string, descriptor: TypedPropertyDescriptor<Function>) => {
        routeManager.regeisterRoute({
            constructor: target.constructor,
            function: descriptor.value,
            method,
            path
        });
    }
}

Expected behavior:
No error
Actual behavior:

error TS2322: Type '(target: any, name: string, descriptor: TypedPropertyDescriptor<Function>) => void' is not assignable to type 'MethodDecorate
or'.
  Types of parameters 'descriptor' and 'descriptor' are incompatible.
    Type 'TypedPropertyDescriptor<T>' is not assignable to type 'TypedPropertyDescriptor<Function>'.
      Type 'T' is not assignable to type 'Function'.
error TS2315: Type 'MethodDecorator' is not generic.
@ikatyang
Copy link
Contributor

For the first one, it is caused by stricter generic checks (#16368).

It seems you have to define a CustomMethodDecorator first, since there is no generic on MethodDecorator, and its declaration requires a non-constraint generic.

Or just enable the --noStrictGenericChecks option.


For the second one, there is indeed no generic on MethodDecorator, the error message seems a little weird.

(lib.d.ts)

declare type MethodDecorator = <T>(target: Object, propertyKey: string | symbol, descriptor: TypedPropertyDescriptor<T>) => TypedPropertyDescriptor<T> | void;

@Norgerman
Copy link
Author

CustomMethodDecorator or --noStrictGenericChecks will works, but I think it should infer T as Function in case 1, or make MethodDecorator generic in case 2.

@ikatyang
Copy link
Contributor

MethodDecorator is defined as <T>(...something: SomeType[]): SomeOutput<T>;

So that it'll accept all kinds of T without any constraint, but in your case 1 it only accept Function, it is indeed not compatible from stricter perspective.

NOTE: With --noStrictGenericChecks enabled, all generics will be considered any before comparing.


make MethodDecorator generic in case 2.

I'm not familiar with that MethodDecorator type, it'd be better to raise another issue for that things.

@aluanhaddad
Copy link
Contributor

aluanhaddad commented Aug 21, 2017

There is no requirements that decorators return one of the predefined decorator types in lib.d.ts.

I would strongly advise against enabling --noStrictGenericChecks and instead using the following (which actually has a richer type but passes the checker thanks to inference.

declare const routeManager: {
  regeisterRoute: (config: {
    constructor?: Function,
    function?: Function,
    method: string,
    path: string
  }) => void;
};

export function route(method: string, path: string) {
  return <T extends object,
        K extends keyof T,
        R extends Promise<any>>
      (target: T, name: keyof T, descriptor: TypedPropertyDescriptor<(...args: any[]) => R>) => {
        routeManager.regeisterRoute({
        constructor: target.constructor,
        function: descriptor.value,
        method,
        path
      });
  };
}

class A {
  @route('GET', 'api/values') m() {
    return Promise.resolve(['a', 'b']);
  }
}

Not only will this pass the type checker since it is a valid decorator type, but the decorator will validate that the decorated method actually returns a Promise for a resource (just an example).

Trying to exactly match the predefined decorator types would preclude this very useful typechecking behavior.

@Norgerman
Copy link
Author

Well, it works. Just like what @ikatyang says to define a CustomMethodDecorator.

@aluanhaddad
Copy link
Contributor

@Norgerman Ah, I missed that comment. Sorry for being redundant.

@mhegazy mhegazy added the Bug A bug in TypeScript label Aug 21, 2017
@mhegazy mhegazy added this to the TypeScript 2.6 milestone Aug 21, 2017
@mhegazy mhegazy added the Domain: Decorators The issue relates to the decorator syntax label Aug 31, 2017
@mhegazy mhegazy modified the milestones: TypeScript 2.7, TypeScript 2.8 Jan 9, 2018
@petispaespea
Copy link

@Norgerman Could you please share your way of implementing CustomMethodDecorator?

@Norgerman
Copy link
Author

@petispaespea see this comment

@parzhitsky
Copy link

parzhitsky commented May 24, 2021

In case somebody needs an exact code, his is how I solved this issue in my project:

interface MethodDecoratorCustom<Instance extends object = object> {
    (
        target: Instance | Constructor<Instance>,
        key: PropertyKey,
        descriptor: TypedPropertyDescriptor<Method<Instance | Constructor<Instance>>>,
    ): void | TypedPropertyDescriptor<Method<Instance | Constructor<Instance>>>;
}

(See definitions of Method and Constructor, and a usage example.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug A bug in TypeScript Domain: Decorators The issue relates to the decorator syntax
Projects
None yet
Development

No branches or pull requests

7 participants