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

Decorators #2249

Closed
rbuckton opened this issue Mar 7, 2015 · 139 comments
Closed

Decorators #2249

rbuckton opened this issue Mar 7, 2015 · 139 comments

Comments

@rbuckton
Copy link
Member

@rbuckton rbuckton commented Mar 7, 2015

ES7 proposal

The ES7 proposal for decorators can be found here: https://github.com/wycats/javascript-decorators
The ES7 proposal serves as the base of this proposal. Below are notes about how the type system

Decorator targets:

Class constructor

@F("color")
@G
class Foo {
}

desugars to:

var Foo = (function () {
    function Foo() {
    }
    Foo = __decorate([F("color"), G], Foo);
    return Foo;
})();

Methods

class Foo {
  @F(color)
  @G
  bar() { }
}

desugars to:

var Foo = (function () {
    function Foo() {
    }
    Foo.prototype.bar = function () {
    };
    Object.defineProperty(Foo.prototype, "bar", __decorate([F(color), G], Foo.prototype, "bar", Object.getOwnPropertyDescriptor(Foo.prototype, "bar")));
    return Foo;
})();

Static method

class Foo {
    @F("color")
    @G
    static sMethod() {}
}

desugars to:

var Foo = (function () {
    function Foo() {
    }
    Foo.sMethod = function () {
    };
    Object.defineProperty(Foo, "sMethod", __decorate([F("color"), G], Foo, "sMethod", Object.getOwnPropertyDescriptor(Foo, "sMethod")));
    return Foo;
})();

Properties

class Foo {
    @F("color")
    @G
    prop: number;
}

desugars to:

var Foo = (function () {
    function Foo() {
    }
    __decorate([F("color"), G], Foo.prototype, "prop");
    return Foo;
})();

Method/Accessor formal parameter

class Foo {
    method(@G a, @F("color") b) {}
}

desugars to:

var Foo = (function () {
    function Foo() {
    }
    Foo.prototype.method = function (a, b) {
    };
    __decorate([G], Foo.prototype, "method", 0);
    __decorate([F("color")], Foo.prototype, "method", 1);
    return Foo;
})();

Where the __decorate is defined as:

var __decorate = this.__decorate || function (decorators, target, key, value) {
    var kind = typeof (arguments.length == 2 ? value = target : value);
    for (var i = decorators.length - 1; i >= 0; --i) {
        var decorator = decorators[i];
        switch (kind) {
            case "function": value = decorator(value) || value; break;
            case "number": decorator(target, key, value); break;
            case "undefined": decorator(target, key); break;
            case "object": value = decorator(target, key, value) || value; break;
        }
    }
    return value;
};

Decorator signatures:

A valid decorator should be:

  1. Assignable to one of the Decorator types (ClassDecorator | PropertyDecorator | MethodDecorator | ParameterDecorator) as described below.
  2. Return a value (in the case of class decorators and method decorator) that is assignable to the decorated value.
declare type ClassDecorator = <TFunction extends Function>(target: TFunction) => TFunction | void;
declare type PropertyDecorator = (target: Object, propertyKey: string | symbol) => void;
declare type MethodDecorator = <T>(target: Object, propertyKey: string | symbol, descriptor: TypedPropertyDescriptor<T>) => TypedPropertyDescriptor<T> | void;
declare type ParameterDecorator = (target: Function, propertyKey: string | symbol, parameterIndex: number) => void;

Notes:

  • Decorating a function declaration is not allowed as it will block hoisting the function to the top of the scope, which is a significant change in semantics.
  • Decorating function expressions and arrow functions are not supported. The same effect can be achived by applying the decorator function as var x = dec(function () { });
  • Decorating function formal parameters is currently not part of the ES7 proposal.
  • Decorators are not allowed when targeting ES3
@rbuckton rbuckton added the Spec label Mar 7, 2015
@rbuckton rbuckton self-assigned this Mar 7, 2015
@fdecampredon
Copy link

@fdecampredon fdecampredon commented Mar 7, 2015

Excuse me from what I understand of the spec, we won't be able to do:

@F
function test() {
}

Am I right ?

@ivogabe
Copy link
Contributor

@ivogabe ivogabe commented Mar 7, 2015

How does type serialization work with rest arguments?

@F()  
class Foo {  
    constructor(...args: string[]) {  
    }  
}  

function F(@paramterTypes types?: Function[]) {  
    return function (target) {  
        target.paramterTypes = types; // ???  
    }  
}
@MgSam
Copy link

@MgSam MgSam commented Mar 7, 2015

Using decorators seems straightforward enough, but I found the sections about declaring them to be confusing. C.4 says decorators need to be annotated with @decorator, but not a single one of the examples actually shows this happening.

Are decorator factories intended to be classes that implement the interfaces found in B?

@JsonFreeman
Copy link
Contributor

@JsonFreeman JsonFreeman commented Mar 7, 2015

What is the rule for refining the interpretation of CoverMemberExpressionSquareBracketsAndComputedPropertyName?

@JsonFreeman
Copy link
Contributor

@JsonFreeman JsonFreeman commented Mar 7, 2015

I noticed many of the typings have Function | Object at various points, but these will degenerate to Object at type check time. What is the reason to have Function there?

@JsonFreeman
Copy link
Contributor

@JsonFreeman JsonFreeman commented Mar 7, 2015

I am not crazy about the terms DecoratorFunction vs DecoratorFactory. I'd much rather follow the nomenclature of generators, which has Generator and GeneratorFunction. With this scheme, we would rename DecoratorFunction to Decorator, and DecoratorFactory to DecoratorFunction.

@JsonFreeman
Copy link
Contributor

@JsonFreeman JsonFreeman commented Mar 7, 2015

For the decorated exports, what is [lookahead ≠ @] for? Can HoistableDeclaration and ClassDeclaration actually start with a @?

@jayphelps
Copy link

@jayphelps jayphelps commented Mar 8, 2015

This is a dup of #1557

@JsonFreeman
Copy link
Contributor

@JsonFreeman JsonFreeman commented Mar 8, 2015

It's not really a dupe, as #1557 was for a different design. This issue is for the decorators design being implemented now.

@jayphelps
Copy link

@jayphelps jayphelps commented Mar 8, 2015

My mistake.

@fdecampredon
Copy link

@fdecampredon fdecampredon commented Mar 9, 2015

For decorator on function expression, could we not do something like :

@F("color") @G 
function myFunc() {
   doSomething();
}

transformed in :

var _t = function() {
   doSomething();
}
_t = F("color")(_t = G(_t) || _t) || _t;  

function myFunc() {
  return _t.apply(this, arguments)
}

It's a bit bother some to have to right every function like :

const myFunc = function () {}

You loose hoisting, and function.name

@mhegazy
Copy link

@mhegazy mhegazy commented Mar 25, 2015

Implementation added in PR #2399

@mhegazy
Copy link

@mhegazy mhegazy commented Mar 25, 2015

Updates: Proposal updated, and added link to the @wycats ES7 JavaScript decorators.

@fdecampredon
Copy link

@fdecampredon fdecampredon commented Mar 26, 2015

saddened that it became a class only thing ...
Also what about ambiant decorator did they get out of the scope ?

@JsonFreeman
Copy link
Contributor

@JsonFreeman JsonFreeman commented Mar 26, 2015

@fdecampredon with your proposal for functions, seems like you still lose the hoisting.

@fdecampredon
Copy link

@fdecampredon fdecampredon commented Mar 26, 2015

@JsonFreeman why ? if you insert _t at the top of the file ?

import something from 'something';

myFunc(something.something());

@F("color") @G 
function myFunc() {
  doSomething()
}
import something from 'something';

var _t = function() {
   doSomething();
}
_t = F("color")(_t = G(_t) || _t) || _t;  

myFunc(something.something());

function myFunc() {
  return _t.apply(this, arguments)
}

Also even if my proposal has a lot of issues, I would seriously like to be able to use decorators on function (even if I have to use variable assigned function) and lose hoisting.
Case like this gist seems a pretty good decorator use case for both function and class (especially coupled with ambient decorators if they still end up being in the scope)

@mhegazy
Copy link

@mhegazy mhegazy commented Mar 26, 2015

@fdecampredon this does not work for the general case, as the decorators are expressions themselves. e.g.

myFunc();  // assumes function declaration is hoisted

var dec = (t) => t; // defininig a decorator

@dec
function myFunc() {}

if you hoist the function declaration and application of the decorator then you break the decorator. if you only hoist the function declaration, but not the decorator application you can witness the function in an undecorated state. no appealing solutions here.

this is the same issue as with class declaration extend clause, which in ES6 is an expression. the result was not hoisting the class declaration, just the symbol (akin to var declaration, but not function declarations)

@fdecampredon
Copy link

@fdecampredon fdecampredon commented Mar 26, 2015

Oups didn't think about it thank you @mhegazy.
However why the function part have completely abandoned the original @jonathandturner proposal had the rule :

decorated function declarations cannot be hoisted to the containing scope

Loosing hoisting is sure a drawback, but I find it damageable to transform it into an class only feature when it would have use case for other construct.

@JsonFreeman
Copy link
Contributor

@JsonFreeman JsonFreeman commented Mar 26, 2015

Let's see what the desired set of constraints seem to imply:

  • decorated function should be hoisted
  • function should be decorated as soon as it is available - therefore decorator application must be hoisted
  • decorator must be defined before it is applied - therefore decorator definition itself (or all entities referenced by the decorator expression) must be hoisted
  • decorator expression gets evaluated at the place it is lexically applied - therefore the application cannot be hoisted

The only resolution I can see for this is the following: For function decorators, we only allow something of the form @identifier. We do not allow a left hand side expression. In addition, the identifier must be a direct reference to a function declaration (including a decorated function). All function decorations that take place in the scope must be emitted at the top of the scope, in the order that they were applied.

@mhegazy
Copy link

@mhegazy mhegazy commented Mar 27, 2015

The problem breaking hoisting rules is that it is surprising. If you are writing javascript for a while you expect function declarations to be available before they are lexically declared; now by adding a seemingly simple syntactic marker (the decorator) this fundamental nature of function declaration is altered.

Having said that, the ES7 proposal is still in its initial stages, so I expect it to evolve and expand; so it is conceivable that the final proposal would include functions in some form.

@JsonFreeman
Copy link
Contributor

@JsonFreeman JsonFreeman commented Mar 27, 2015

I do think they should be hoisted. I am saying that the only decorations we allow on functions are decorations that are definitely themselves hoisted. Namely identifiers that reference function declarations.

But there is another problem here. It is actually impossible to simultaneously hoist all decorated functions and ensure that they are not observed in their undecorated state. The problem can be seen with a decorator cycle.

@dec1
function dec2(target: Function) {
   // Do stuff
}

@dec2
function dec1(target: Function) {
   // Do stuff
}

Even if both functions are hoisted, which one gets decorated first? If dec2 gets decorated first, then dec1 will not itself be decorated by the time it is applied to dec2.

So we would have to choose between the following:

  • The functions are not hoisted
  • The undecorated functions are hoisted, but the decorator applications don't get hoisted with them.

While I don't like either of these, I don't even think anything else is possible.

@mhegazy
Copy link

@mhegazy mhegazy commented Mar 27, 2015

This is the JS proposal. so the engine would not know if the expression refers to a function declaration or not, with static analysis we could tell though. consider this:

@dec1
function dec2(target: Function) {
   // Do stuff
}

dec2 = undefined;

@dec2
function dec1(target: Function) {
   // Do stuff
}
@Koloto
Copy link

@Koloto Koloto commented Sep 4, 2015

@cybrown Yes, it uses the property descriptor for methods and accessors. I tested on plain properties (fields w/o accessors) only. But it seems that decorators can be allowed in ES3 for properties (w/o accessors) and classes. It would be helpful.

@Koloto
Copy link

@Koloto Koloto commented Sep 4, 2015

And a fake property descriptor can be used for methods when targeting ES3. Something like { writable: true, enumerable: true, configurable: true }. So I can't see any reason to not support ES3.

@JsonFreeman
Copy link
Contributor

@JsonFreeman JsonFreeman commented Sep 5, 2015

@sccolbert I see. That makes sense. Incidentally, TypeScript is working on improved this typing for functions and methods. I wonder if that would be of any help here. Although I suppose typing is not the issue for you, it's runtime semantics.

@sccolbert
Copy link

@sccolbert sccolbert commented Sep 5, 2015

@JsonFreeman Improved this typing sounds intriguing for some of my other use cases. Do you have any more info on that?

@JsonFreeman
Copy link
Contributor

@JsonFreeman JsonFreeman commented Sep 5, 2015

I think the most developed discussion on this typing is at #3694.

@sccolbert
Copy link

@sccolbert sccolbert commented Sep 5, 2015

Cheers!

@TakoLittle
Copy link

@TakoLittle TakoLittle commented Sep 19, 2015

error TS1207: Decorators cannot be applied to multiple get/set accessors of the same name.

@get
public get myValue():any{...}

@set
public set myValue(value:any){...}

code above is not allowed even it make more sense compare to

@get
@set
public get myValue():any{...}

public set myValue(value:any){...}
@mhegazy
Copy link

@mhegazy mhegazy commented Sep 19, 2015

Getter and setter are defined in one call to Obect.defineProperty. This rather a js quirk, the declaration of set and get though separate, really are the same property declaration. The error check in the compiler is to alert users when thinking of them separately; the decorators are applied only once to the property descriptor.

@TakoLittle
Copy link

@TakoLittle TakoLittle commented Sep 21, 2015

just wondering since the compiler can sense the get and set with same name and combine into single Object.defineProperty, why not also combine the decorator?
or perhaps an option flag to tell compiler to combine them, and leave a warning message instead of throwing error.

thank you

@rbuckton
Copy link
Member Author

@rbuckton rbuckton commented Sep 21, 2015

@TakoLittle: The reason we don't do this today partially stems from how decorators are composed. Decorators follow the same principals as Mathematical function composition, where (fg)(x) is composed as f(g(x)). In the same sense, it can be thought that:

@F
@G
class X {}

Is approximately:

F(G(X))

The compositionality of decorators breaks down when you decorate both the getter and the setter:

class C {
  @F
  set X(value) {}

  @G
  get X() {}
}

How do F and G compose here? Is it based purely on document order (i.e. F(G(X)))? Are each set of decorators for the getter and the setter discrete, and then executed in document order (i.e. G(F(X)))? Do get and set imply any specific ordering (i.e. is the get always before the set or vice versa)? Until we're 100% certain the most consistent approach that doesn't surprise users, or have a well documented approach that is part of the decorators proposal with at least stage 2 or better acceptance within ECMA-262, we feel it is best to be more restrictive and error here as it allows us to relax that restriction at a later date without introducing a breaking change that could easily go unnoticed and possibly result in unexpected behaviors at runtime.

@TakoLittle
Copy link

@TakoLittle TakoLittle commented Sep 21, 2015

@rbuckton thank you so much for detailed explanation
TS team great work!! ^^d

@omeid
Copy link

@omeid omeid commented Feb 20, 2016

Where is the documentation for this? and care to link the implementation commit?

Thanks.

@EisenbergEffect
Copy link

@EisenbergEffect EisenbergEffect commented Feb 20, 2016

@mhegazy What is the status on the implementation of the latest version of the spec. I understand there are some changes there.

@mhegazy
Copy link

@mhegazy mhegazy commented Feb 22, 2016

This issue tracked the original version of the proposal. since this is completed we are closing this issue. for any updates to the spec, we will log new issues and outline all the breaking changes. I do not think the proposal is at a place now to be ready for us to jump on it. We are working closely with @wycats on the new proposal.

@EisenbergEffect
Copy link

@EisenbergEffect EisenbergEffect commented Feb 23, 2016

@mhegazy Thank you for the update. I'd love to stay informed. When you create the new issue for the spec update, please link it here so I can be notified and follow. The Aurelia community makes heavy use of decorators and we'll want to synchronize with both TypeScript and Babel on the update. Again, thanks for the great work the TS team is doing!

@whitecolor
Copy link

@whitecolor whitecolor commented Aug 27, 2016

Function decoration is need of course.
Are there also plans for decorating of other objects in the code?

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
You can’t perform that action at this time.