proposal-function-expression-decorators icon indicating copy to clipboard operation
proposal-function-expression-decorators copied to clipboard

Decorator lazy evaluation for function declaration

Open hax opened this issue 7 years ago • 47 comments

Let's continue the discussion here.

hax avatar Mar 04 '18 03:03 hax

copy from @hax 's comment: https://github.com/tc39/proposal-decorators/issues/40#issuecomment-369867557


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 avatar Mar 04 '18 03:03 hax

copy from @rbuckton 's comment: https://github.com/tc39/proposal-decorators/issues/40#issuecomment-370010647


@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 avatar Mar 04 '18 03:03 hax

Copy from @trotyl 's comment https://github.com/tc39/proposal-decorators/issues/40#issuecomment-370124254


@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 avatar Mar 04 '18 03:03 hax

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

hax avatar Mar 04 '18 03:03 hax

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?

rbuckton avatar Mar 04 '18 03:03 rbuckton

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

trotyl avatar Mar 04 '18 04:03 trotyl

@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 avatar Mar 04 '18 06:03 hax

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

hax avatar Mar 04 '18 07:03 hax

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

rbuckton avatar Mar 04 '18 14:03 rbuckton

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

@bar
const foo = () =>

iddan avatar Mar 04 '18 14:03 iddan

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 avatar Mar 04 '18 14:03 iddan

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

hax avatar Mar 04 '18 16:03 hax

@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 avatar Mar 04 '18 17:03 hax

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.

hax avatar Mar 04 '18 17:03 hax

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

Alphare avatar Mar 04 '18 17:03 Alphare

Would decorators only be processed when the function is called?

My second comment https://github.com/iddan/proposal-function-expression-decorators/issues/3#issuecomment-370198619 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.)

hax avatar Mar 04 '18 17:03 hax

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.

Alphare avatar Mar 04 '18 17:03 Alphare

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

hax avatar Mar 04 '18 18:03 hax

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 avatar Mar 04 '18 18:03 trotyl

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)

trotyl avatar Mar 04 '18 19:03 trotyl

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

rbuckton avatar Mar 04 '18 19:03 rbuckton

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 avatar Mar 04 '18 19:03 hax

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

hax avatar Mar 04 '18 19:03 hax

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.

trotyl avatar Mar 04 '18 20:03 trotyl

@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

Alphare avatar Mar 04 '18 20:03 Alphare

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 🤪)

hax avatar Mar 04 '18 20:03 hax

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.

trotyl avatar Mar 04 '18 20:03 trotyl

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?

iddan avatar Mar 05 '18 08:03 iddan

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

trotyl avatar Mar 05 '18 09:03 trotyl

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

iddan avatar Mar 05 '18 09:03 iddan