tc39 / proposal-decorators

Decorators for ES6 classes
https://arai-a.github.io/ecma262-compare/?pr=2417
2.72k stars 108 forks source link

Allow decorating functions #40

Closed Alphare closed 6 years ago

Alphare commented 6 years ago

Hi,

At the bottom of the README, one can read "Arguments and function declarations cannot be decorated.". Is there a reason why functions should not be decorated? Coming from a Python development background, my (only) use-case for decorators is to wrap functions, allowing me to extract redundant logic, short-circuit and do other great meta-programming things.

I would expect to be able to write code like:

@myDecorator
function myFunc(args) { 
    // code 
}

and

@myDecorator
const myFunc = (args) => { 
    // code 
}

Edit Maybe a better way of writing lambdas with decorators:


const myFunc = @myDecorator (args) => { 
    // code 
}

Argument decoration seems like a great feature as well:

function myFunc(@myDecorator callback, otherArgs) { 
    // code 
}

Thank you!

ljharb commented 6 years ago

One complication with function declarations is that they are hoisted - and they might be hoisted prior to where myDecorator is available or declared.

Alphare commented 6 years ago

I guess so, but I would expect programmers to be aware of this limitation and arrange their code accordingly.

ljharb commented 6 years ago

The only way to arrange your code such that it would work is if you had function myDecorator() {} declared before your decorated function declaration, or if you imported myDecorator from somewhere first - that would ace out a lot of coding styles.

pakal commented 6 years ago

The requirement to have decorators declared before their use on functions seems not very constraining - non-hoisted functions like Python's have the same requirement. And most decorators will probably come from third-party libraries.

I feel function decorators are even more wanted than class decorators, as people are quite used to them in other languages.

Alphare commented 6 years ago

Most points I would make regarding the syntax, use cases and motivation are contained within the PEP 318 document, should that be useful.

EDIT This bit is funny, we're in the exact opposite situation here:

Modifying classes in this fashion is also possible, though the benefits are not as immediately apparent. Almost certainly, anything which could be done with class decorators could be done using metaclasses, but using metaclasses is sufficiently obscure that there is some attraction to having an easier way to make simple modifications to classes.

littledan commented 6 years ago

As @ljharb has said here, function hoisting is the big thing which makes supporting decorated function declarations in JavaScript different, and the difference with Python is that Python doesn't have function hoisting in general. I worry that if we make the way functions are bound different when they are decorated, the effect will be confusion about how things don't quite match up. For that reason, this proposal just adds class, method and field decorators and not function decorators.

iddan commented 6 years ago

But what about const foo = () => functions? They are not hoisted right?

littledan commented 6 years ago

@iddan No, they are not, but is that a common enough idiom to warrant a language feature around? And why not just do const foo = decorator(() => ...); as you can do today already?

iddan commented 6 years ago

Because that's the whole point of decorators - syntax sugar and clear meta-programming. You can use the same case for classes as well - why not just do const foo = decorator(class {}).

An example with recompose:

With Decorators

@pure
@withState('data', 'setData', null)
const MyComponent = ({ data, setData }) => (
    <div onClick={() => setData(Math.random())}>{data}</div>
);

Without Decorators

const MyComponent = pure(withState('data', 'setData', null)(({ data, setData }) => (
    <div onClick={() => setData(Math.random())}>{data}</div>
));

or more precisely to retain function name

const MyComponent = pure(withState('data', 'setData', null)(function MyComponent ({ data, setData }) {
   return <div onClick={() => setData(Math.random())}>{data}</div>
}));
  1. The first example is more readable. The visual pattern of const [NAME] = (ARGS) => RESULT stays and you get each higher order component to be on another line.

  2. When composing functions as long as the |> operator doesn't exists composing functions and applying them on inline functions becomes cumbersome and unreadable.

  3. In the second example which is the common case for inlining functions and higher order ones MyComponent.name is "MyComponent" and not "" - this is very important for debugging and making sense of code.

Also Meta programming can be easily distinguished visually for example in:

@cached
const getData = () => localStorage.getItem('data')

getData() definition is easily distinguishable from the @cached which only suggests adding external functionality

trotyl commented 6 years ago

@littledan Wrong close, should be tc39/proposal-decorators-previous#40 rather than tc39/proposal-decorators#40.

Alphare commented 6 years ago

@littledan Could you re-open this issue?

littledan commented 6 years ago

I'd still like to leave this feature for a follow-on proposal, due to both the issues described above and the lesser utility of it compared to class decorators (although use cases are demonstrated above). I'd encourage people who are interested in the feature to make a separate proposal repository and prototype various implementations in transpilers, to get more of data on the tradeoffs and motivation here.

littledan commented 6 years ago

Previous discussion: https://github.com/tc39/proposal-decorators-previous/issues/35

iddan commented 6 years ago

Two questions:

  1. Can this be proposed while this proposal is active?
  2. How can I help defining this proposal (as a non TC39 member)?
littledan commented 6 years ago
  1. Yes, very much so. Parameter decorators are at Stage 1, while class decorators are at Stage 2. It actually helps the class decorators proposal along to have things more thought through about how these follow-on proposals will look, so we can be well-informed about their possible interactions/consistency.
  2. You can create a proposal repository and start with an explainer. Unfortunately our documentation for how to do this isn't so great at the moment, but you can see an overview of the process at CONTRIBUTING.md. I'd recommend focusing the explainer on:
    • Why this is a problem worth solving, and why it's not sufficient to simply use a function call for these use cases.
    • The high-level shape of the semantics (will it be based on just passing the function to a function, or does it make sense to use a descriptor like class decorators do?)
    • Advocating for one or discussing the tradeoffs of various approaches for the function hoisting problem (this could be in an issue in the repo).
trotyl commented 6 years ago

Parameter decorators are at Stage 1

@littledan From tc39/proposals it's still stage 0, isn't it?

hax commented 6 years ago

The best solution for the hoisting problem I can imagine is transform

@deco function f() {
  funcBody
}

to

function f(...args) {
  $do_decorate_f()
  return $decorated_f.call(this, ...args)
}
var $decorated_f
function $do_decorate_f() {
  if (!$decorated_f) $decorated_f = deco(function f() {
    funcBody
  })
}
$do_decorate_f()

Aka, not hoist the execution of decorator until first call.

littledan commented 6 years ago

@trotyl Oh, you're right, thanks.

@iddan @Alphare @hax Would you be interested in starting another repository to continue this investigation? Please let me know if the instructions are unclear or if I could help with anything.

iddan commented 6 years ago

Yes, I'll start a repo.

iddan commented 6 years ago

Started at https://github.com/iddan/proposal-function-expression-decorators

rbuckton commented 6 years ago

@hax, your example only works if the decorator deals with the function's execution, but not if the decorator augments the function object itself.

My opinion is that decorating a function declaration should transform it into something like this:

var f = @deco function() {
  funcBody
}

However that means adding a decorator to a function might mean needing to move the entire function around in your code if you were depending on function declaration hoisting.

Another possibility is to introduce block-scoped function declaration syntax like this:

let function f() {}
const function g() {}

And then only permitting decorators on a block-scoped function declaration. It still requires moving the function, but it is visually consistent with block-scoped variable declarations.

A third option would be to make decorator evaluation lazy and apply it at one of two times (whichever comes first):

The closest approximation of this approach in existing code would be this:

let g = f; // access f before declaration
f(); // invoke f before declaration

@deco function f() {
  f(); // invoke f inside of declaration
}

Becomes:

let g = $$f_accessor(); // access f before declaration
(void 0, $$f_accessor())(); // invoke f inside of declaration

function f() {
  f(); // invoke f inside of declaration
}
var $$f_decorated;
function $$f_accessor() {
  return $$f_decorated || ($$f_decorated = f = deco(f));
}
$$f_accessor(); // eval decorator if not already accessed
iddan commented 6 years ago

I like the third option. With a little tree cleaning this will work in any situation and usually will only require using the naïve implementation.

trotyl commented 6 years ago

@rbuckton The third option won't work with circular dependency in ES module.

Given the scenario:

// main.mjs
import './f';

// f.mjs
import { g } from './g';

export function f ( x ) {
  return x + 1;
}

console.log(g(1));

// g.mjs
import { f } from './f';

export function g(x) {
  return x - 1;
}

console.log(f(1));

Which should output 2 and 0;

When decorating f with some decorator:

// f.mjs
import { g } from './g';

function deco(fn) {
  return function (x) {
    return fn(x) + 100;
  }
}

export @deco function f ( x ) {
  return x + 1;
}

console.log(g(1));

Option a): still exports f itself:

// ...
export function f ( x ) {
  return x + 1;
}

var $$f_decorated;
function $$f_accessor() {
  return $$f_decorated || ($$f_decorated = f = deco(f));
}
$$f_accessor();

console.log(g(1));

Then undecorated original function gets called.

Option b): exports the decorated one:

// ...
function f ( x ) {
  return x + 1;
}

var $$f_decorated;
export { $$f_decorated as f }
function $$f_accessor() {
  return $$f_decorated || ($$f_decorated = f = deco(f));
}
$$f_accessor();

console.log(g(1));

Then it would throw due to calling undefined: TypeError: f is not a function.

Option c): exports the accessor call result:

// ...
function f ( x ) {
  return x + 1;
}

var $$f_decorated;
function $$f_accessor() {
  return $$f_decorated || ($$f_decorated = f = deco(f));
}
var $$f_accessor_export = $$f_accessor();
export { $$f_accessor_export as f }

console.log(g(1));

Also got undefined and throws as above.


So there's still no way to keep the behavior consistent between decorated and undecorated functions. The result becomes:

The decorated function would be hoisted in most cases, but not hoisted in some special cases

That's a pretty bad idea for consistency.

Also, making decorator affects execution of dependent files would break current module semantics, and making transformer depends on external files could be very complex (if possible) and cannot help with libraries.

One solution could be waiting for https://github.com/tc39/proposal-module-get lands first.

Not possible even with that landed.

littledan commented 6 years ago

How would it be if we closed this thread and continued discussion on @iddan 's new repo?

Alphare commented 6 years ago

@littledan Fine with me, thanks. :)

hax commented 6 years ago

@trotyl @rbuckton I copy the recent related comments to https://github.com/iddan/proposal-function-expression-decorators/issues/3 , @littledan you can close or lock this thread, thank you.

AbrahamTewa commented 6 years ago

Hi everyone, I'm not sure to understand the hoisting problem, since the decoration is performed at runtime. The decorator is not evaluated until it's used :

try {
  test();
}
catch (e) {
  console.log(e.message); // "TypeError: decorator is not a function"
}

@decorator
function test() {
  console.log('test');
}

var decorator = function() {
  console.log('decorator');
}

test();
// decorator
// test

This seems to me to be the expected behavior : the decorator method is called only when used (ie when calling the test function). Hoisted before or after the decorated function or not at all is not a problem, since it will only be used at runtime.

We have exactly the same probem with classes:

var namespace = {};

@namespace.decorator
class Test {
}

new Test(); "TypeError: namespace.decorator is not a function"

var namespace = {
   decorator : function() {}
}
littledan commented 6 years ago

@AbrahamTewa As @hax said above, let's follow up in https://github.com/iddan/proposal-function-expression-decorators/issues/3 . Closing this thread per @hax's suggestion to avoid confusion.

brillout commented 1 year ago

Is there any update on function decorators? EXTENSIONS.md mentions function decorators but there doesn't seem be any proposal?

TypeScript is implementing decorators, so this may be the right time to propose something for function decorators.

pzuraq commented 1 year ago

Function decorators would be a separate proposal and would need to advance through the stage process from the beginning on their own, so they do not have much of impact on TS implementing the current decorators proposal.