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

Decorator lazy evaluation for function declaration #3

Open
hax opened this issue Mar 4, 2018 · 47 comments
Open

Decorator lazy evaluation for function declaration #3

hax opened this issue Mar 4, 2018 · 47 comments

Comments

@hax
Copy link
Contributor

hax commented Mar 4, 2018

Let's continue the discussion here.

@hax
Copy link
Contributor Author

hax commented Mar 4, 2018

copy from @hax 's comment: tc39/proposal-decorators#40 (comment)


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.

@hax
Copy link
Contributor Author

hax commented Mar 4, 2018

copy from @rbuckton 's comment: tc39/proposal-decorators#40 (comment)


@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):

When the function declaration is encountered during evaluation,
The first time the function declaration is accessed as a value.
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

@hax
Copy link
Contributor Author

hax commented Mar 4, 2018

Copy from @trotyl 's comment tc39/proposal-decorators#40 (comment)


@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.

@hax
Copy link
Contributor Author

hax commented Mar 4, 2018

@trotyl The whole idea is eval decorator lazily when first access f whenever locally or in other modules, that means what need to export is $$f_accessor() call pattern (not result).

With https://github.com/tc39/proposal-module-get you refer to, it's possible to transform the @deco export f() {...} to

...
export get f() { return $$f_accessor() }
...

Without export get extension, it's hard to transform it to ES2015 module, but it's possible to transform it to CommonJS.

Note: The other option is use Proxy to wrap f, but I'm not sure whether it will introduce other problem.

Anyway, there are the difficulties of polyfills, not for engines.

@rbuckton
Copy link

rbuckton commented Mar 4, 2018

Option 3 is not really viable, because it falls apart even without modules:

function deco() { f(); }
@deco function f() {}

If we did go with Option 3, we'd still need Option 1 for the recursive reference case, or we'd get a stack overflow. In the case of recursive dependencies between modules, we'd still need Option 1.

At the end of the day, it seems only one of three scenarios make sense:

  1. While decorators could be allowed on FunctionExpression, they can never be permitted on FunctionDeclaration. To decorate a FunctionDeclaration you would need to rewrite it as a FunctionExpression.
  2. Decorating a FunctionDeclaration gives it TDZ like let/const.
  3. (2), but with some kind of syntactic mark that makes it clear it has let/const-like semantics (my Option 2 above).

If we choose (1) the fact we cannot decorate function declarations will likely come up again and again as it is inconsistent with the ability to decorate classes and function expressions. If we choose (2) we would have to convince the rest of TC39 that it is worth expanding TDZ to this case, but we would have the consistency across these declarations hopefully without needing to revisit the decision. If we choose (3) we have to defend the syntactic marker.

Are there any other scenarios that make sense that I'm missing?

@trotyl
Copy link

trotyl commented Mar 4, 2018

@hax The module getter won't be hoisted as well, consider:

let foo = 42;
export get f() { return foo; }

It will be an early error when accessing foo before the declaration.

Even transpiled to:

Object.defineProperty(module, 'f', {
  get() { ... }
}

This is still a statement and won't be executed at that time.

@hax
Copy link
Contributor Author

hax commented Mar 4, 2018

@rbuckton Oh I never thought about recursive... What happen for class decorator?

// deco.js
import {A} from './a'
export function deco() {
  new A() // <- call undecorated A ?
}
// a.js
import {deco} from './deco'
@deco
export class A {}

If the behavior is return original undecorated A, I think we can just do a little adjustment to deal with reenter like:

var $$f_decorating, $$f_decorated;
function $$f_accessor() {
  if (!$$f_decorated) {
    if ($$f_decorating) return f
    $$f_decorating = true
    f = deco(f);
    $$f_decorated = true
  }
  return f;
}

@hax
Copy link
Contributor Author

hax commented Mar 4, 2018

@trotyl What I mean is using export get to export f like this:

export get f() { return f_accessor() }

I assume export get f hosited same as export function f.
And f_accessor as function decl is hoisted, so it works.

@rbuckton
Copy link

rbuckton commented Mar 4, 2018

@hax, classes have a TDZ and the declaration isn't yet initialized when the decorator is called, so you would get a runtime error. This is not the case with functions.

@iddan
Copy link
Owner

iddan commented Mar 4, 2018

Have you considered to limit this proposal to instead of introducing new syntax?

@bar
const foo = () =>

@iddan
Copy link
Owner

iddan commented Mar 4, 2018

I created a transpiling PR (#2) so hoisting ideas can be tested in the wild.
Would you like to merge the basic transpiling for now? Or to limit it to var func = expressions?

@iddan iddan mentioned this issue Mar 4, 2018
3 tasks
@hax
Copy link
Contributor Author

hax commented Mar 4, 2018

@rbuckton Ok, so we could also throw similar runtime error when meet reenter?

@hax
Copy link
Contributor Author

hax commented Mar 4, 2018

@iddan I think the keypoint is, if we allow decorate arrow functions, function expressions, or even some new syntax, but not function declaration, it just make programmer get much more confusion when they finally try decorate plain old functions.

We all know hoisting is troublesome, even without decorator, you may meet some bug caused by a hoisting function accessing a variable in TDZ or uninitialized state. But in most cases, it works. I don't see much difference with addition of decorator. When ES2015 introduce modules, it also keep the hoisting semantic. Note, without hoisting function declarations, there is no reason to support circular dependency modules because you will just get TDZ or undefined. The designers of ES modules put effort to support hoisting semantic of function semantic. So should we do.

I believe the direction of lazy evaluation is promising (or maybe Mirror,though I haven't figured it out now, hope @rbuckton could have time to explain how it work). We just need to get a reasonable behavior for most cases, and leave edge cases as is (just like throw runtime Error for TDZ).

@hax
Copy link
Contributor Author

hax commented Mar 4, 2018

The transpiling of decorating function declarations may be a problem, because we rely on export get f() {} and assume it hosit same as export function f() {}. Or we need cross-module compiling, change all import f from and the usage of f to import $$get_f from and $$get_f(), which seem very impossible.

@Alphare
Copy link

Alphare commented Mar 4, 2018

I'm getting somewhat confused with the chronological order of things. Would decorators only be processed when the function is called?
(Also, great stuff so far. This is above my JS skillset, but this gives me lots of ammo for improving.)

@hax
Copy link
Contributor Author

hax commented Mar 4, 2018

Would decorators only be processed when the function is called?

My second comment #3 (comment) use this strategy. But @rbuckton is right, it should be the first time of function is accessed, not called. (That's why we need new export get f for transpiling. Normal export function f can not provide such semantic.)

@Alphare
Copy link

Alphare commented Mar 4, 2018

That would remove the ability of creating "register" decorators - that is a decorator that commits its wrapped function to a global register on module import for example - and I assume other great meta-programming patterns.
Maybe it's impossible to implement in JS, I have no idea.

@hax
Copy link
Contributor Author

hax commented Mar 4, 2018

@Alphare Sorry I don't get it, could you give a example code?

@trotyl
Copy link

trotyl commented Mar 4, 2018

without hoisting function declarations, there is no reason to support circular dependency modules because you will just get TDZ or undefined

@hax There could be non-eager usage, like:

import { f } from './f';

export const g = () => {
  f();
}

As long as they're not used in top-level (module initialization), it makes no difference whether hoisted or not.

@trotyl
Copy link

trotyl commented Mar 4, 2018

I assume export get f hosited same as export function f.

@hax A reason that module getter cannot be hoisted is the computed "property" name, like:

const t1 = 'f';
const t2 = 'oo';
export get [t1 + t2]() {
  return 42;
}

Should be same as:

const t1 = 'f';
const t2 = 'oo';
Object.defineProperty(module, t1 + t2, { get(): { return 42;} })

Even not listed in the current https://github.com/tc39/proposal-module-get proposal, should still be valid for consistency with normal property accessor (in follow-on proposals).

Technically only ImportName must be static since hoisted, ExportName would be OK to be computed as long as determined during initialization. (But could be bad for tooling)

@rbuckton
Copy link

rbuckton commented Mar 4, 2018

@Alphare, my suggestion for lazy decorator evaluation was to have the decorator applied the first time the decorated function is accessed, not the first time it is called. Unfortunately, this is impossible to transpile with full fidelity if you are targeting ES modules, but is generally feasible when targeting CommonJS or AMD (and I'm not certain, offhand, whether it is feasible in SystemJS). In those cases transpilers could only do a "best effort" and export it via something like an export { $$f_decorated as f }; statement (which means it may be undefined). Sometimes "best effort" is all you can hope for in a transpiler, and those using the transpiler would need to be aware of the shortcoming.

@hax
Copy link
Contributor Author

hax commented Mar 4, 2018

As long as they're not used in top-level (module initialization), it makes no difference whether hoisted or not.

@trotyl U r correct. But in that time (pre ES6), there is no arrow functions, and it's very unlikely to see var f = function () {} instead of function f() {}. So it just leave top-level codes if get rid of function declarations. An important requirement of ES modules is support modularizing existing codes. Ok, I see my "without hoisting function declarations" precondition fail anyway, so the inference is meaningless. 🤕

@hax
Copy link
Contributor Author

hax commented Mar 4, 2018

@trotyl Good example. But I don't see use cases for supporting computed property for export get.

Normal getter/setter need it, mainly because we need to support access symbol property or some invalid identifier key like "spaced key". But we can only import/export valid identifier.

Anyway, it's the issue of module-get proposal. Whether it would have hoisting semantic, it's not available for transpiling now.

@trotyl
Copy link

trotyl commented Mar 4, 2018

An important requirement of ES modules is support modularizing existing codes.

In a real-life JavaScript application (even before es6), most invocations happen in callbacks, like event handlers, schedulers, etc. So supporting hoisted funtion may not be as appealing as it sounds.

Even in initialization logic one would normally write:

import { foo } from 'foo';

$(function () {
  var res = foo();
  $('#hello').text(res);
});

Due to the natural conditions of JavaScript.

@Alphare
Copy link

Alphare commented Mar 4, 2018

@rbuckton So, pardon my ignorance, but does this mean that it would never be possible in VanillaJS down the road without major import API changes?
Thanks for answering

@hax
Copy link
Contributor Author

hax commented Mar 4, 2018

In a real-life JavaScript application (even before es6), most invocations happen in callbacks, like event handlers, schedulers, etc.

Ideally the main function should be called start from DOMContentLoaded, unfortunately many do not obey (eg. most 3rd-party ad load itself via document.write("<script src=wtf>")). And there is Node.js which normally start from a plain script.

PS. It's a bit off topic, we can argue it in other place (like my wechat group 🤪)

@trotyl
Copy link

trotyl commented Mar 4, 2018

That would remove the ability of creating "register" decorators - that is a decorator that commits its wrapped function to a global register on module import for example

@Alphare Your case is not related to this topic and always feasible. It would always be called during initialization, this topic is about the ordering between each other during initialization.

@iddan
Copy link
Owner

iddan commented Mar 5, 2018

I suggest we separate the work process to three goals:

  1. Specification for function expressions i.e @f () => and maybe const f as well.
  2. Solution for non exported decorated function declarations
  3. Solution for exported decorated function declarations.

Because I think we can parallel the thinking processes and get to richer conclusions.
@littledan @hax @trotyl @Alphare what do you think?

@trotyl
Copy link

trotyl commented Mar 5, 2018

@iddan Regarding the first one, there's already an decorator proposal for function expressions (at stage-0-proposals but no repo given), would it be better to contact the current champion first?

@iddan
Copy link
Owner

iddan commented Mar 5, 2018

Indeed. This repo suppose to reflect the proposal as it's currently only has a document

@hax
Copy link
Contributor Author

hax commented Mar 5, 2018

To be honest, I think the decorator for function expressions/arrow functions/assignment expressions are all replacement solutions due to the difficulty of function declarations. If decorator for function declarations is possible (or if we choose a new syntax declaration like const f() {}), we may not need to support decorating expressions.

Note: As current decorator (for class) proposal, if I read it correct, only support decorating class declarations, not class expressions.

@iddan
Copy link
Owner

iddan commented Mar 5, 2018

The React community can leverage decorating function expressions a great deal because as I mentioned in previous discussions top popular libraries like react-redux, recompose and lodash/fp and some libraries actually already use the decorator semantics

@hax
Copy link
Contributor Author

hax commented Mar 5, 2018

@iddan It seems all examples in https://github.com/iddan/proposal-function-expression-decorators/blob/master/EXAMPLES.md could also use function declaration instead?

@ljharb
Copy link

ljharb commented Mar 5, 2018

Avoiding declarations is often desirable, because it prevents anyone from relying on hoisting (and aids readability by ensuring to the reader that no hoisting is occurring). Decorating declarations but not expressions wouldn’t be acceptable.

@hax
Copy link
Contributor Author

hax commented Mar 5, 2018

@ljharb I may read spec wrong, it seems current class decorator only apply to class declaration not class expression?

@AbrahamTewa
Copy link

AbrahamTewa commented Apr 16, 2018

copy from @AbrahamTewa 's comment: tc39/proposal-decorators#40 (comment)


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');
}

/* somehow equivalent of :
var test = function test() {
  return decorator(function() {
    console.log('test');
  });
}
*/

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

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() {}
}

@aprilmintacpineda
Copy link

Any news on this?

@hax
Copy link
Contributor Author

hax commented Jan 20, 2020

@aprilmintacpineda No news. Actually decorator proposal (decorators for methods and classes) still have many problems. It seems this proposal only make sense if decorator proposal could go forward.

@aprilmintacpineda
Copy link

@hax Thanks for the response. Are those problems known to the proponents and is anyone/group trying to solve those problems?

@hax
Copy link
Contributor Author

hax commented Jan 21, 2020

@aprilmintacpineda This depends on the champions of decorators proposal. To be honest, I'm not very optimistic about decorator proposal, it has been three versions. The latest version (static decorator) changed a lot. Though I understand there are reasons for all the changes but I find many programmers are confused why decorator proposal become like that (the new version don't follow semantic/syntax of any similar features in other languages, C# attribute, Java annotation, Python decorator, and TS legacy decorator).

@aprilmintacpineda
Copy link

@hax Hmmm... Then I guess decorators are most likely as good as abandoned? I haven't seen any updates on the discussions so far and it doesn't look like anyone is working on it.

@ljharb
Copy link

ljharb commented Jan 21, 2020

It’s absolutely being worked on still; a lack of updates on something just means there’s no changes, not that there’s no continued discussion.

@brillout
Copy link

It’s absolutely being worked on still; a lack of updates on something just means there’s no changes, not that there’s no continued discussion.

👍

Is there an update about this written somewhere?

@ljharb
Copy link

ljharb commented Jan 24, 2021

This isn’t the repo for decorators, see https://github.com/tc39/proposal-decorators

@brillout
Copy link

Thanks @ljharb, I found recent infos about function decorators at tc39/proposal-decorators#353.

@revmischa
Copy link

Suppose function decorators were only allowed on function expressions in strict mode? They cannot be hoisted and are common and familiar enough.

'use strict'

const cachedFunc = @cached (foo: number) => foo + 42

Here we could safely decorate the function no?

@whosyourlaowang
Copy link

whosyourlaowang commented Nov 16, 2021

why not create a keyword $$func [name], It is equivalent to let [name] = function () {}

@debounce(200)
@trycatch
@loading
@cahced
async $$func run() {
 // ...
}

// equal 

let run = debounce(trycatch(loading(cahced(async function () {
 // ...
))), 200)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests