Allow decorators for functions as well

[svieira]

Decorators for classes, methods, and object properties are a really nice extension to the language. The only extra one I would like to see is the ability to add decorators to function declarations:

@RunOnce
function expensiveOperation() {
    return 1 + 1;
}

de-sugars to:

var expensiveOperation = RunOnce(function expensiveOperation() {
  return 1 + 1;
});

---

This suggests allowing decorators for any assignment, but I think that might be a bit much so I'm leaving it out of this PR:

// For the curious, that would allow constructs like this
@RunOnce
let expensiveOp = () => 1 + 1

// De-sugars to

let expensiveOp = RunOnce(() => 1 + 1);

[sebmck]

The issue with adding decorators to function declarations is that they're hoisted which means hoisting the decorators too which changes the position in which they're evaluated.

[svieira]

True, and that is a potential foot-gun, but I cannot think of any cases where source position is likely to be needed at run-time (I can see using decorators as targets for compile-time transformations using sweet-js, but in those cases the function hoisting behavior does not apply.) As I understand it, hoisting is likely to (though not specified to) happen in order, so even if you had a stateful decorator that added each decorated function to a queue, the functions would still run in order.

Am I just being dense and missing an obvious use case where source position could make or break functionality?

[sebmck]

Some examples:

var counter = 0;

var add = function () {
  // this function isn't even accessible if the decorator evaluation is hoisted
  // ie. done before `var counter = 0;`
  counter++;
};

@add
function foo() {

}

// what is the value of `counter`?
// if it's hoisted then it would be 2 when logically it should be 1

@add
function foo() {

}

var readOnly = require("some-decorator");

// is the evaluation hoisted?
// if so `readOnly` wont be defined if not then any reference to `foo` before the
// decorator will get the bare function
@readOnly
function foo() {

}

The behaviour is non-obvious and it's impossible to sprinkle to get obvious behaviour. It's similar to class declarations not being hoisted because their extends clause can be an arbitrary expression.

[svieira]

Chuckles Fair enough - thanks for taking the time to point out the issues!

[nmn]

The noted problems here suggest to me that decorators are possibly going to have very much the same problem as operator overloading is supposed to have.

The major concern I have with decorators is people writing too many decorators with not very good names. Or even for things that are pretty complicated. The concern I have is that decorators are only syntactic sugar to make code more declarative. But I hope they won't end up creating code that is often misleading and full of surprises.

I personally don't think any feature's merits should be judged by considering the worse use-cases. But I think it's still interesting to think about them to remove as many footguns as possible.

---

Relevant Point: Perhaps, functions decorators SHOULD work for functions. But since hoisting is the problem, Hoisting could be disabled for any function that is decorated. Since Function assignments work the same way, I don't think its too confusing.

[sebmck]

@nmn

But since hoisting is the problem, Hoisting could be disabled for any function that is decorated. Since Function assignments work the same way, I don't think its too confusing.

It is confusing. Function declarations that are hoisted sometimes isn't going to cut it.

[nmn]

@sebmck Fair point.

Thinking about it more, perhaps they are no so important for functions.

Functions that take take and return functions are pretty easy to write. They are more useful in classes as the syntax keeps things from getting hairy.

Thanks.

[Ciantic]

Why are they not important for functions? Memoize is just as needed in functions as well as in classes. Also for currying, who doesn't curry their functions ;)

If problem is you can accidentally use class decorators in functions, maybe another syntax:

@@memoize
function something() {
}

double at, or something.

Edit: Is hoisting really a problem? Being used Python a lot which implements function decorator, I don't think the order of decoration is used to anything except in bad code, which is really hard to safeguard anyway.

[sebmck]

@spleen387 Hoisting is still a problem.

[nmn]

first a question: do generator functions get hoisted? If not isn't that confusing as well.

If yes, there is already a proposal for making custom wrappers around generators, like async functions.

async function x(){}
// becomes
function x(){
  return async(function*(){
    ...
  })()
}

How does this proposal deal with hoisting?

[nmn]

Another approach, let the function get hoisted. But the decorator should be applied where it is defined.

So this:

var readOnly = require("some-decorator");

@readOnly
function foo() {

}

will desugar (and hoist) to:

function foo() {
}
var readOnly = require("some-decorator");

foo = readOnly(foo)

[sebmck]

Generator function declarations are definently hoisted. Hoisting the function declaration and not the decorator is unintuitive and confusing. You can just wrap the function in a method call and it expresses the exact same thing without the confusing semantics.

On Friday, 17 April 2015, Naman Goel notifications@github.com wrote:

Another approach, let the function get hoisted. But the decorator should be applied where it is defined.

So this:

var readOnly = require("some-decorator");

@readOnly function foo() {

}

will desugar (and hoist) to:

function foo() { } var readOnly = require("some-decorator");

foo = readOnly(foo)

Reply to this email directly or view it on GitHub https://github.com/wycats/javascript-decorators/issues/4#issuecomment-94097610 .

Sebastian McKenzie

[nmn]

@sebmck I just updated my first comment. How does compositional functions deal with hoisting?

You can just wrap the function in a method call and it expresses the exact same thing without the confusing semantics. I get that, but the same could be said about classes as well.

I don't like the idea of decorators for only classes, and I'm just trying to find a solution to the hoisting problem. I feel like having decorators for only classes makes the language more disjointed and inconsistent than it already is.

[sebmck]

I get that, but the same could be said about classes as well.

Classes don't hoist so they don't suffer from the same problem that disallow function declaration decorators.

I don't like the idea of decorators for only classes, and I'm just trying to find a solution to the hoisting problem.

This isn't an intuitive solution to the hoisting problem and whatever solution is going to be an extreme hack.

[nmn]

This isn't an intuitive solution to the hoisting problem and whatever solution is going to be an extreme hack.

In that case, perhaps decorators aren't a great idea after all? Wouldn't having decorators for class methods but not normal functions be equally confusing, hacky and inconsistent?

[sebmck]

In that case, perhaps decorators aren't a great idea after all?

How are you getting that decorators aren’t a good idea just because you can’t put them on function declarations because of hoisting? An entire feature shouldn’t be nerfed because of one nefarious case.

Wouldn't having decorators for class methods but not normal functions be equally confusing, hacky and inconsistent?

No, because class methods aren’t function declarations.

On Sat, Apr 18, 2015 at 7:51 AM, Naman Goel notifications@github.com wrote:

This isn't an intuitive solution to the hoisting problem and whatever solution is going to be an extreme hack.

In that case, perhaps decorators aren't a great idea after all? Wouldn't having decorators for class methods but not normal functions be equally confusing, hacky and inconsistent?

Reply to this email directly or view it on GitHub: https://github.com/wycats/javascript-decorators/issues/4#issuecomment-94170865

[nmn]

No, because class methods aren’t function declarations.

Fair enough. I still have my reservations, but my intent was only to have a discussion about this.

[ivan-kleshnin]

How are you getting that decorators aren’t a good idea just because you can’t put them on function declarations because of hoisting? An entire feature shouldn’t be nerfed because of one nefarious case.

I imagine how people begin to write single method classes only to get decoration feature enabled where basic function could satisfy... While decorator functions can be applied without decorator syntax that's noisy and inconsistent.

@sebmck you're the best of us aware of ES(any). Any change to kill this hoisting behavior ever? That is hoisting that allows to use undefined vars hiding syntax errors until runtime. Right? Probably the worst JS language aspect :imp:

[jamiebuilds]

Any change to kill this hoisting behavior ever? That is hoisting that allows to use undefined vars hiding syntax errors until runtime. Right? Probably the worst JS language aspect

There's zero chance of function hoisting ever going away.

I imagine how people begin to write single method classes only to get decoration feature enabled where basic function could satisfy...

Let's explore what a function decorator would be (ignoring hoisting)

function memoize() {
  // do something...
}

@memoize
function myFunc() {
  // do something else...
}

Ignoring the aspect of hoisting, this would be equivalent to:

function memoize() {
  // do something...
}

var myFunc = memoize(function() {
  // do something else...
});

There's no way someone would rather do this:

function memoize(desc) {
  // do something...
  desc.value = value;
  return desc;
}

class MyFuncClass {
  @memoize
  myFunc() {
    // do something else...
  }
}

let myFuncInstance = new MyFuncClass();
myFuncInstance.myFunc();

So yeah we're left with sticking with this:

var myFunc = memoize(function() {
  // do something else...
});

or adding function decorators:

@memoize
function myFunc() {
  // do something else...
}

and because of hoisting function decorators become impossible.

So what's the point of this?

[ivan-kleshnin]

@thejameskyle I want to believe something like SaneScript deprecation machinery at compiler level could handle such cases.

[sebmck]

@ivan-kleshnin Do you mean behind a directive?

[ivan-kleshnin]

@sebmck, yes. I just want to express my opinion that language without at least tiny amount of deprecation process or at least "workaround modes" can't exist forever. Not breaking the web motto sounds good, but complexity grows and grows and we still lack a lot of things... I'm seriously considering to just drop everything and move to ClojureScript or other language which can evolve because it's not bound by backward compatibility craziness. Babel is truly awesome but a lot of things can't be fixed at the transpiler level without serious deviations from ES specs. JS still sucks at enterprise scale. I understand why people continue to use Ruby and other languages on backend.

[yuchi]

To solve this a lot of companies enforced the use of function expressions over function declaration. In fact once you change a function declaration to an expression you get different semantics.

To give this use case more power ES6 now asks the function expression to be named after the variable name.

Also, for succinctness you can go full-arrows here:

const myFunc = memoize(( ) => {
  //
});

[nmn]

It's been a very useful discussion.

1. I agree with @sebmck that the feature should not be nuked for one feature. Honestly it is simpler to wrap functions in functions that classes right now, so decorators are more useful for classes anyway.
2. However, this discussion is becoming a discussion about succinctness. I think the point of decorators is to make things look more declarative. I would like to argue that passing classes through functions is relatively short to write as well.
3. I was interested in this: https://github.com/jhusain/compositional-functions Generator functions are also hoisted, and it works by making a function that returns a function. So it applies the transformation at call time. I realize that it may not be the best idea for normal functions.

[sebmck]

Compositional functions are different as the grammar only allows ImportedBinding:

CompositionFunctionDeclaration : ImportedBinding [no LineTerminator here] function BindingIdentifier ( FormalParameters ) { FunctionBody } This means that you can only do import foo from “foo”; foo function bar() {}.

Imports are also hoisted so they’ll be bound before the function declarations are.

On Wed, Apr 22, 2015 at 11:11 PM, Naman Goel notifications@github.com wrote:

It's been a very useful discussion.

1. I agree with @sebmck that the feature should not be nuked for one feature. Honestly it is simpler to wrap functions in functions that classes right now, so decorators are more useful for classes anyway.
2. However, this discussion is becoming a discussion about succinctness. I think the point of decorators is to make things look more declarative. I would like to argue that passing classes through functions is relatively short to write as well.
3. At this point, I really want to discuss this: https://github.com/jhusain/compositional-functions

Generator functions are also hoisted. So how does it work?

Reply to this email directly or view it on GitHub: https://github.com/wycats/javascript-decorators/issues/4#issuecomment-95353509

[hannahhoward]

The recent Angular weekly meeting notes suggests this is up for discussion again. Is it?

[cesarandreu]

From what I call tell in #7, it seems like syntax/grammar(?) is still in flux... However, based on babel's current behavior, it kinda sucks that the following works for class declarations and not functions:

import bar from './bar'

// Works
@bar
export class Foo {}

// Doesn't work
@bar
export function foo () {}

Why not allow functions to be decorated by other hoisted values? (i.e. other functions and imports)

[rtm]

I'm curious why this issue would have been closed. If one agrees that there is a strong case for decorators, certainly it should apply in equal measure to functions. The current spec is designed to provide decorators for functions, but then says you can have them only if they're members of some class or object.

I'm wondering if there's a bit of anti-function sentiment at work here--something like, "real men use objects and classes and member functions, not wimpy plain old functions", or "in real frameworks like Exxxx everything is an object and since all I really want is syntactic sugar for writing computed properties why do I need decorators on anything other than literal object methods?".

AFAICT the only issue here is hoisting. But to jump from that to saying we can't decorate functions at all seems to be throwing out the baby with the bathwater. If we can't figure out how to have a single, coherent decoration mechanism, we should throw in the towel. The hoisting issue seems to be a red herring to me. For instance, the following code has well-understood, if counter-intuitive behavior:

function foo() {
    console.log(x);    // undefined
    var x = 99;
    console.log(x);    // 1
}

No one would claim that this is obvious, but that's the way the language works. By the same token, one could handle the function/hoisting/decorator case as follows:

function bazify(fn) { fn.baz = 99; return fn; }

function foo() {
    console.log(bar.baz);  // undefined

    @bazify
    function bar() { }

    console.log(bar.baz);    // 99
}

This is no more or less confusing or anti-intuitive than existing behavior from hoisting vars. The effect would be exactly the same as the desugared version below:

function foo() {
  console.log(bar.baz);  // undefined

  function bar() { }
  bar = bazify(bar);

  console.log(bar.baz);    // 99
}

To put it another way, for consistency with current hoisting behavior the decorator would not be hoisted. The decorator would have similar behavior to the initial value in a var statement.

Oops, just noticed this is precisely what @nmn proposed, which seems eminently reasonable.

[RReverser]

Just in case this helps somebody - I've gathered a couple of workarounds for decorating functions in a composable way into one article here: https://rreverser.com/ecmascript-decorators-and-functions/

Let me know if I missed something.

[Macil]

The decorator would have similar behavior to the initial value in a var statement.

ES6 is adding let and const because they weren't happy with var's semantics. Those variables act differently if referred to before defined:

function foo() {
    console.log(x);    // throws an exception
    let x = 99;
    console.log(x);
}

Making functions be hoistable but decorators are only applied on the line they're used is like making a function half hoistable in a worst of both worlds way. If you consider decorators that critically change the behavior of a function like a RunOnce decorator, then splitting up the definition of the function from the application of the decorator in a way (accidentally) visible to other code can surely only lead to confusion. Applying a decorator like that should be done in a way such that the undecorated function isn't ever visible to any other code but the decorator itself.

[rtm]

@AgentME A valid point. Let's step back and take a look at the big picture. I hope everyone agrees that it would real nice to have decorated functions. Or, you could go a step further and say, it would be odd to have decorated methods and classes but not functions. The problem is how to handle hoisting.

AFAICS, hoisting is not an issue if the decorator has no side effects. Of course there is nothing wrong with writing decorators with side effects, and there are valid use cases for doing so, but they are rare. Furthermore, even if a decorator does have side-effects, in only some subset of such cases will the side effects together with hoisting result in unexpected behavior.

So to say that we will simply give up on decorated functions because of the few cases where decorators have side effects, and those side effects cause unexpected behavior due to hoisting, is really throwing out the baby with the bathwater. We are telling people that you cannot decorate functions at all, ever, or to decorate functions they must wrap them in a class or object, because of some potentially unexpected behavior in what is decidedly an edge case.

If it is considered unacceptable to hoist separately from decorating, and I do see your point, then we should hoist and decorate at the same time, and document and communicate the potential issues involved when doing so, rather than simply giving up on decorated functions forever.

[svieira]

I am of the mind (now) that the best way to go about this would be to treat decorated function declarations as function expressions, but I don't have the time to look into how that would work with the existing grammar.

[Macil]

@rtm Making the application of the decorator be hoistable could only work if the definition of the decorator itself is hoistable. Consider a decorator created from an expression:

bar();
var decoratorFactory = new DecoratorFactory({label: 'foo'});
var RunOnce = decoratorFactory.makeRunOnce();
@RunOnce
function bar() {
  console.log('bar');
}

(If anyone starts to picture radical hoisting strategies that could solve the above example, just consider the possibility that DecoratorFactory's constructor could call bar().)

You could make it so decorated functions are only hoistable if their decorators are all hoistable, but that makes it so you can no longer look at just a function definition and tell if it's hoistable or not without considering the full context. You could make the choice that https://github.com/jhusain/compositional-functions did, and require decorators be hoistable imported bindings (or function declarations), but that should affect class decorators too for consistency and seems arbitrary here. Making decorated functions be unhoistable function expressions seems like the simplest solution.

[julien-f]

IMHO, requiring decorators to be hoistable or imported bindings should be safe first step in supporting decorators for functions.

[loganfsmyth]

I personally don't see a way that function declaration hoisting and decorators can be made compatible while hoisting is preserved. No matter what, some change in behavior from ES6-standard behavior would have to be made. Making decorated function declarations un-hoistable seems like the best approach to me if this is a behavior that is pursued, though I'm not sure I'm set on that anyway.

Related to @AgentME's point above, there is at least one place in the ES6 spec currently where it will be difficult not to have conflicts. The import edge case in the current spec is the one that comes to mind

Consider

import 'bar';
export function helper(){}

If we look at this from an CommonJS standpoint

exports.helper = helper;
require('bar');

function helper(){}

Note that helper is actually available before bar is even loaded. That means if there were circular references here, bar could actually import this module and use helper during its own execution.

If we expand that to a decorator:

import 'bar';

@foo
export function helper(){}

function foo(){}

what is the exported value of helper? By definition, imports are processed before the module body executes. No javascript code is evaluated, which means while foo has a value, there is no execution context in which the decorator could execute. Does it first export an un-decorated helper, then update the exported value after the import has completed? It could be special-cased for this one example to export undefined first or something, but that leaves the cases previously mentioned. The simplest over-all solution would be to essentially treat decorated function declarations as variable declarations of function expressions.

[RReverser]

@loganfsmyth Your example will throw - you can't have local declaration with the same name as imported binding as it's behavior is similar to const.

[loganfsmyth]

@RReverser Ahh, good catch, the foo from in the last block is just a typo, shouldn't be there at all.

[kobezzza]

Why we can't just add a function declaration without hoisting?

@bar
def foo() {
  return 1;
}

In ES6 was added let instead of var, because var was a shit. And now we can do the same for the function.

[ivan-kleshnin]

@kobezzza despite this would be a 2348234 way to declare function I'm afraid I agree :smile: It also solves the following boilerplate problem:

let foo = function () {}; 
// + no hoisting
// - anonymous function 

function foo () {}; 
// - hoisting
// + named function

let foo = function foo () {}; 
// + no hoisting 
// + named function
// - boilerplate, DRY violation

And finally:

def foo () {} // equivalent to `let foo = function foo () {};`
// + no hoisting 
// + named function

I'm not sure about def – I would choose func but it's not me to decide.

[esamattis]

Doesn't arrow function do exactly that?

let foo = () => {}; 
assert(foo.name, "foo");

In fully implemented ES6 it will also have a name:

http://www.2ality.com/2015/09/function-names-es6.html

Babel example.

[RReverser]

I'm not sure about def – I would choose func but it's not me to decide.

Could just reuse let like in object literals if going that way at all:

let x = 1, y = 2, foo() { ... };

But yeah, it's essentially the same as arrow function in ES6.

[lukescott]

Shouldn't this:

@foo
export function helper(){}

be this:

export @foo function helper(){}

...or does it matter?

With the hoistability problems, couldn't this:

function foo() {
  console.log(bar.baz); // 99
  @bazify
  function bar() { }
  console.log(bar.baz); // 99
}

be desuggared as:

function foo() {
  var bar = bazify(function bar() { })
  console.log(bar.baz);  // 99
  console.log(bar.baz);  // 99
}

The transpiler would have to change the placement of the declaration to maintain the hoistability. At that point the decorated function is "hoisted". I'm not sure if it's worth the complexity though, or what the edge cases of that would be.

Otherwise if you make the function syntax decorator-able, you either have to choose between not hosting the function at all or hosting the non-decorated function and decorating it in-line (as mentioned by others above).

There is another option: Take the same approach as let:

@bazify function bar() {} // invalid syntax
@bazify func bar() {} // valid, not hoisted
// or (bound this)
@bazify bar() => {} // valid, not hoisted
// or (non-bound this)
@bazify bar() -> {} // valid, not hoisted
// other possible variation of bound this
@bazify func bar() => {}

If this approach is taken function would be left alone and you would have to use a different syntax to declare functions if you wanted to hoist them.

As a side-note, what about this use-case?

var obj = {
  @bazify bar() {}
}
// or
var obj = {
  bar: @bazify function bar() {} // hosting doesn't apply here
}

With above, consistency is also a concern.

[loganfsmyth]

@lukescott The value after @ is just an expression, which is evaluated in the current scope. You could just as easily have

function foo() {
  console.log(bar.baz); // 99
  var bazify = function(target, name, descriptor){};
  @bazify
  function bar() { }
  console.log(bar.baz); // 99
}

which with your proposal would then throw because bazify is undefined if the evaluation of the decorator were hoisted. That is the core of the hoisting issue.

[lukescott]

@loganfsmyth Ah, gotcha. It could also be this as well (given the current spec):

function foo() {
  console.log(bar.baz); // 99
  function bazify(target, name, descriptor){};
  @bazify
  function bar() { }
  console.log(bar.baz); // 99
}

But still, there is no guarantee a decorator would also be hoisted.

What about the the let approach? Just leave function alone and have another way to declare a function that isn't hoisted that can be decorated.

[calebmer]

+1 to the bash/object syntax-ish approach which acts like a let.

@bazify
bar() {
  // Whatever
}

Also, what about expanding variable definitions to support the object-like method syntax?

let foo() {}
const bar() {}
var buz() {}

[calebmer]

Is this a blocker for the spec's completion?

[ivan-kleshnin]

The thing with def is that it never was a reserved word (like let and const were) so adding it would break a lot of code. So I agree that @RReverser and @calebmer proposition of let or const reusal may be the best alternative. Not sure if spec's would allow that, though.

Arrow functions do not accept this injection which may be a dealbreaker. Some library APIs depend heavily on this.

// KnexSQL //
// ...
.join(function () {
  this.on(...) // `this` is defined
})

// ...
.join(() => {
  this.on(...) // `this` is undefined
}) 

[cztomsik]

I'm curious if decorators are the real way to go - what I want is more compact (kinda backwards) syntax in the hope of more readable code.

Functional programming is very powerful and it can easily get cryptic and hard to explain. I also think this is the reason why are utility-belts (lodash) more successful than real FP libraries - it's easy to teach them even to beginners. My point is that - what if all we need is "rewriting":

@@cache({...})
function hash(str){
  let hash = '';

  ...

  return hash;
}

which would get expanded to:

function hash(str){
  return cache({...}, (str) => {
    let hash = '';

    ...

    return hash
  });
}

What I mean is that instead of returning function in "init-time" the function would get rewritten in a way cache() would get called. This is actually simple function composition but I think it might be easier to understand by newbies while still very valuable.

Unlike decorators, it retains function name and plays nicely with hoisting and named exports. We would loose ability to store function meta-informations but I personally don't believe in magic Angular2 is trying to do.

(I am not sure how would this relate to classes but I can see value in this for functions)

[Macil]

@cztomsik That still has the issue that if the decorator isn't hoistable, and the function gets called before the decorator is declared, then you get an exception.

Also many decorator functions (runOnce / memoization cache functions) should be called only once per function decoration, so they can set up some variables in a closure.

[cztomsik]

Wouldn't it be possible to throw syntax error if function is mentioned anywhere before decorator is defined?