-
Notifications
You must be signed in to change notification settings - Fork 127
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
Allow decorators for functions as well #4
Comments
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. |
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? |
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. |
Chuckles Fair enough - thanks for taking the time to point out the issues! |
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. |
It is confusing. Function declarations that are hoisted sometimes isn't going to cut it. |
@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. |
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. |
@spleen387 Hoisting is still a problem. |
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.
How does this proposal deal with hoisting? |
Another approach, let the function get hoisted. But the decorator should be applied where it is defined. So this:
will desugar (and hoist) to:
|
Generator function declarations are definently hoisted. Hoisting the On Friday, 17 April 2015, Naman Goel [email protected] wrote:
Sebastian McKenzie |
@sebmck I just updated my first comment. How does compositional functions deal with hoisting?
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. |
Classes don't hoist so they don't suffer from the same problem that disallow function declaration decorators.
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? |
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.
No, because class methods aren’t function declarations. On Sat, Apr 18, 2015 at 7:51 AM, Naman Goel [email protected]
|
Fair enough. I still have my reservations, but my intent was only to have a discussion about this. |
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 @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 👿 |
There's zero chance of function hoisting ever going away.
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? |
@thejameskyle I want to believe something like SaneScript deprecation machinery at compiler level could handle such cases. |
@ivan-kleshnin Do you mean behind a directive? |
@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. |
To solve this a lot of companies enforced the use of function expressions over function declaration. 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(( ) => {
//
}); |
It's been a very useful discussion.
|
Compositional functions are different as the grammar only allows ImportedBinding: CompositionFunctionDeclaration : 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 [email protected]
|
The recent Angular weekly meeting notes suggests this is up for discussion again. Is it? |
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) |
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:
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:
This is no more or less confusing or anti-intuitive than existing behavior from hoisting
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 Oops, just noticed this is precisely what @nmn proposed, which seems eminently reasonable. |
The thing with Arrow functions do not accept // KnexSQL //
// ...
.join(function () {
this.on(...) // `this` is defined
})
// ...
.join(() => {
this.on(...) // `this` is undefined
}) |
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":
which would get expanded to:
What I mean is that instead of returning function in "init-time" the function would get rewritten in a way 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) |
@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. |
Wouldn't it be possible to throw syntax error if function is mentioned anywhere before decorator is defined? |
Decorators are all about sugar; there's no new functionality AFAIK.
|
Yep, I am here because I'd love to write:
|
Why can we not just throw an error if the decorator isn't hoisted? This proposal is built on top of ES6 therefore we can assume hoisted imports are available and in a single file a decorator of a hoisted function can be a hoisted function. We don't need to add anything else to the language, we have the tools to make hoisted decorators work. Why not? |
What if you decorate a hoisted function lazily: @exclaim
function sayHello(name) {
console.log("Hello " + name);
}
function exclaim(func) {
return function(name) {
return func(name + "!");
}
} Desugars to: function sayHello() {
console.log("decorate late");
sayHello = exclaim(function sayHello(name) {
console.log("Hello " + name);
});
return sayHello.apply(this, arguments);
}
function exclaim(func) {
return function(name) {
return func(name + "!");
}
}
sayHello("John");
sayHello("Bill"); Output:
You would only do this if the function you are decorating has been hoisted. This should work regardless if the decorator is hoisted or not. |
This thread becomes repetition of same ideas that were already discussed, over and over. Maybe it would make more sense to lock the issue for now? |
@RReverser perhaps I'm missing something. Has the idea I expressed above been expressed earlier? |
Nevermind. While it works, the reference is different before and after the call. |
What about decorating a return value? Would it be treated the same as export statements, however that gets resolved? function foo() {
@bar
return function () { ... }
} or function foo() {
return @bar function () { ... }
} And of course I'm also here to throw my +1 for function decorators :) |
|
@ackvf const fun = () => {}
fun = foo(fun) instead of const fun = foo(() => {}) but let foo = fn => (console.log(fn.name), fn)
let fun = () => {}
fun = foo(fun) I implement let decorator in babel, here is a example which needs the feature to get let function's name. |
You might want to use pipeline operator for that instead:
|
Why do we have a problem with removing hoisting altogether? Classes don't hoist, const/let declarations don't hoist as well. I suggest that at the moment user decorates a function, it becomes unhoisted, and it's his responsibility to properly position that function within the code, just like with classes. For example this: @decorate
function foo() { return 1; }
console.log(foo()); // 1 ...gets transformed into this: const foo = decorate(function foo() { return 1; });
console.log(foo()); // 1 And if user assumes that it's hoisted, throw a ReferenceError: console.log(foo()); // 1 - OK
function foo() { return 1; }
console.log(foo()); // 1 console.log(foo()); // ReferenceError
@decorate
function foo() { return 1; }
console.log(foo()); I believe JS should softly guide people away from hoisting, and remove bad language features overall. |
It'd be a shame to exclude decorators from plain functions. It was said that hoisting isn't going away, one can assume backwards compatibility. But what if something akin to "use strict" – e.g. "no hoisting" – could be added to the top of a script/module? It would be considered a compilation hint. You would only use it in situations like this where hoisting gets in the way. |
My understanding is that the big reason a script-wide feature like strict mode actually made it through in the first place was not because it threw away old misfeatures, but because the act of throwing away those misfeatures enabled brand new valuable use cases (SES / Google Caja javascript sandboxing). And the separate modes idea finally went away in the new happy-path case: modules force strict mode on. I think the standards groups would be extremely hesitant to reverse that victory to re-add a new script/module-wide "use stricter" mode. Instead of script/module-wide settings, I think it makes a lot of sense to look at how the async-await feature added a new "await" keyword. "await" was not a reserved word in Javascript prior to async-await being added, so if it were added as a new reserved word, then all code that used "await" as a variable name would break. The standard solved this by making "await" a keyword only inside of async functions. The standard avoided breaking old code by making you opt in to the new behavior by making a function async. Making it so decorated functions don't hoist seems to me like a strategy consistent with how async-await was added without breaking old code. You locally opt in to the new behavior by using a new feature. |
I don't like being that +1 guy, but this is sorely missed. Assignment decorators (tc39/proposal-decorators#51) and disabling hoisting on decorated function declarations seem like very reasonable solutions. |
Come from the fair land of Python, [plain / const x = () =>] function decorators are perhaps more important than class member function decorators. |
Decorators for functions allow to define a custom how a function should behave. We already have similar and a "hardcoded" syntax of async functions: async function foo() {} (yep, there is also
A more specific example (the thing I was trying to implement before I discovered that there is no decorators for functions) is a multi-thread calculation via inline Worker: @worker function myWorker() {}
await myWorker(); And currently I need to wrap the const myWorker = worker(function myWorker() {});
await myWorker(); or worse: const myWorker = function myWorker() {} |> worker;
await myWorker(); which is very inobvious if a function gets more than a few lines. This is a really wanted feature. I hope somebody more experienced than me could make a spec for that. More examples (also borrowed from above messages): // call the function on window load and store it for further use
@onload function onWindowLoad() {} // call the original function only after something is happened (like window load)
@onsomethinghappen function() {
// do something
}
// currently it can be done using async function and an additional await call
async function() {
await waitForSomethingHappen;
// do something
} // a debounced or throttled function
@debounce(100) function debounced() {}
@throttle(100) function throttled() {} // log some information on function call
@log function logged() {} // a function which doesn't throw an error even if it's happened
@try function mayThrowAnError() {} // a momoized functional React component
const ReactComponent = @memo () => (<div>Hello world</div>) // delayed function
@timeout(1000) function delayed() {} Some syntax examples: @decorator function() {}
@decorator(...args) function() {}
const foo = @decorator function() {}
const foo = @decorator () => {}
return @decorator () => {}
export default @decorator async () => {} |
Guys, I've made a little article about the function decorators: https://github.com/finom/function-decorators-proposal and also started a discussion at ESDiscuss mailing list. I don't know where it leads but I hope to get at least a little chance of approval of stage 0 from TC39 committee. TL;DR
@decorator1 @decorator2 function foo() { return "Hello" }
// Will become:
let foo = decorate([decorator1, decorator2], function foo() { return "Hello" }); // no hoisting! |
Actually this reality today, as this is what one have to do in Angular for Pipes: import { Pipe, PipeTransform } from '@angular/core';
@Pipe({name: 'exponentialStrength'})
export class ExponentialStrengthPipe implements PipeTransform {
transform(value: number, exponent?: number): number {
return Math.pow(value, isNaN(exponent) ? 1 : exponent);
}
} |
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:
de-sugars to:
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:
The text was updated successfully, but these errors were encountered: