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 decorators altering return type of method #7349

Closed
opensrcken opened this issue Mar 2, 2016 · 3 comments
Closed

support decorators altering return type of method #7349

opensrcken opened this issue Mar 2, 2016 · 3 comments
Labels
Duplicate An existing issue was already created

Comments

@opensrcken
Copy link

TypeScript Version:

nightly (1.9.0-dev.20160217)

Code

function wrap<T, U>(wrapper: (arg: T) => U) {
    return function(target: Object, propertyKey: string, descriptor: TypedPropertyDescriptor<(...args: any[]) => T>) {
        var originalMethod = descriptor.value; // save a reference to the original method

        // NOTE: Do not use arrow syntax here. Use a function expression in
        // order to use the correct value of `this` in this method (see notes below)
        const newDescriptor = {
            value: function(...args: any[]) {
                console.log("The method args are: " + JSON.stringify(args)); // pre
                var result = wrapper(<T> originalMethod.apply(this, args));               // run and store the result
                console.log("The return value is: " + result);               // post
                return result;                                               // return the result of the original method
            }
        };

        return newDescriptor;
    }
}

function len(val: string): number {
    return val.length;
}

class MyClass {
    @wrap<string, number>(len)
    myMethod(arg: string) {
        return "Message -- " + arg;
    }
}

const t: number = new MyClass().myMethod("testing");

Expected behavior:
I expect this to compile, and modify the return type of MyClass#myMethod.

Actual behavior:

tsc decorators-broken.ts --experimentalDecorators --target es5
decorators-broken.ts(25,5): error TS1241: Unable to resolve signature of method decorator when called as an expression.
  Type '{ value: (...args: any[]) => number; }' is not assignable to type 'void'.
decorators-broken.ts(31,7): error TS2322: Type 'string' is not assignable to type 'number'.

I'm trying to create a decorator that wraps methods, potentially altering the wrapped method's return type, making function composition possible through decoration. I initially thought this was a bug, but upon inspecting MethodDecorator's type signature, this is probably intentional:

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

Can we add something like the following?

type MutatingMethodDecorator = <T, U>(target: Object, propertyKey: string | symbol, descriptor: TypedPropertyDescriptor<T>): TypedPropertyDescriptor<U> | void;
@opensrcken
Copy link
Author

Another illustration of this limitation...

import { decorate } from 'core-decorators';

// see: https://github.com/jayphelps/core-decorators.js/blob/master/test/unit/decorate.spec.js
// and: https://github.com/DefinitelyTyped/DefinitelyTyped/blob/master/core-decorators/core-decorators.d.ts
function toString(fn) {
    return function (val: number): string {
        return String(val);
    }
}

class Task {
  @decorate(toString)
  id(data: number) {
    return data;
  }
}

var task = new Task();
var ret: number = task.id(1);
console.log(typeof ret);
$ node decorator-test.js
string

Note that the compiler insists that the type of ret is number, because of the issue described in the OP.

@opensrcken
Copy link
Author

This one is probably more difficult to add, and I know mixins are already on the TS roadmap, but if the type information of a class could be altered by decorators, that would be quite nice:

import { mixin } from 'core-decorators';

const SingerMixin = {
    sing(sound: string) {
        alert(sound);
    }
};

const FlyMixin = {
    // All types of property descriptors are supported
    get speed(): number { return 42; },
    fly() {},
    land() {}
};

@mixin(SingerMixin, FlyMixin)
class Bird {
    singMatingCall() {
        //TODO: For @mixin, I don't know how we can make it work for TypeScript...
        // this.sing('tweet tweet');
    }
}

var bird = new Bird();
bird.singMatingCall();
bird.fly();
tsc core-dec-test-2.ts --experimentalDecorators --target es5
core-dec-test-2.ts(28,6): error TS2339: Property 'fly' does not exist on type 'Bird'.

@mhegazy
Copy link
Contributor

mhegazy commented Mar 28, 2016

looks like a duplicate of #4881

@mhegazy mhegazy closed this as completed Mar 28, 2016
@mhegazy mhegazy added the Duplicate An existing issue was already created label Mar 28, 2016
@microsoft microsoft locked and limited conversation to collaborators Jun 19, 2018
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Duplicate An existing issue was already created
Projects
None yet
Development

No branches or pull requests

2 participants