Skip to content
This repository has been archived by the owner on Jan 25, 2022. It is now read-only.

Decorate a function #35

Closed
zhaoyao91 opened this issue Jun 14, 2017 · 29 comments
Closed

Decorate a function #35

zhaoyao91 opened this issue Jun 14, 2017 · 29 comments

Comments

@zhaoyao91
Copy link

Here is my story:

Firstly, I wrote React components using HOCs pattern. Say, I have some hocs and compose them into a component like this:

compose(
  hoc1,
  hoc2,
  ...
)(function MyComponent (props) {...})

Then, I learned the decorator syntax and changed my code into such format:

@hoc1
@hoc2
...
class MyComponent extends React.Component {
  render () {...}
}

But this is not perfect. The final great format should be like this:

@hoc1
@hoc2
...
function MyComponent (props) {...}

But currently decorator can not be applied to function, so sad :(

@borela
Copy link

borela commented Jun 14, 2017

What's the implementation of your decorators?

@zhaoyao91
Copy link
Author

a function which takes a component and return a (maybe another new) component

@jkrems
Copy link

jkrems commented Jun 15, 2017

The usual problem with decorating functions is hoisting. E.g. would the code below throw? Would it use the state of dec at the time of the "original" place of the function declaration? If the latter, what would the first call do? Would it call the undecorated function? What about the subsequent calls?

It's somewhat simpler for function expressions - but then the syntax offers little over the existing solution of just calling the "decorator" function.

MyTarget();

let dec = decoratorA;

MyTarget();

dec  = decoratorB;

MyTarget();

@dec
function MyTarget() {}

MyTarget();

@julien-f
Copy link

julien-f commented Aug 9, 2017

@jkrems IMHO, decorating function expressions is far cleaner than the wrapper syntax:

const MyComponent = compose(
  hoc1,
  hoc2,
  hoc3
)(function () {
  // ...
})

@hoc1
@hoc2
@hoc3
const MyComponent = function () {
}

@zenparsing
Copy link
Member

@littledan What do you think about this issue? It has come up over and over again; the inability to decorate a function is completely unintuitive and seems like a showstopper to me.

@littledan
Copy link
Member

We were at an impasse before due to the ugly question of how decorators and function hoisting would interact. If you have a decorator and a function defined in the same block/top-level, it's not clear how to combine function hoisting with applying the decorators.

I'm not sure what the solution to this problem is. I see how function decorators are useful, but what I don't see is how it's a showstopper--the use cases for function decorators and class decorators are pretty distinct, and the features could proceed separately, right?

@julien-f
Copy link

The hoisting problem disappears if the decorators are applied to the assignment (with or without variable declaration), but it could indeed be another proposal.

@littledan
Copy link
Member

@julien-f You mean we'd allow decorators on assignments of function literals, but not on function declarations? Interesting idea. I am not sure if everyone would buy into that.

@bathos
Copy link

bathos commented Aug 17, 2017

I’m curious re:

I see how function decorators are useful

since it doesn’t seem to be explained yet in this thread. Class and method decorators provide hooks into facets of class declaration that otherwise require somewhat verbose, imperative mutations of the constructor and prototype after declaration. In languages that have function decorators it is usually (I think?) because there are similar things to hook into (or because functions aren’t first class). It’s not obvious to me though, looking at this, what decorating a function actually means except "a new syntax for calling" that happens to accept only one kind of first argument. Is there something meaningful that this would provide?

@allenwb
Copy link
Member

allenwb commented Aug 17, 2017

@littledan

We were at an impasse before due to the ugly question of how decorators and function hoisting would interact. If you have a decorator and a function defined in the same block/top-level, it's not clear how to combine function hoisting with applying the decorators.

This seems to have a fairly simply solutions. Bind decorated function declarations exactly the same way that class declarations are bound. They are in a TDZ until evaluation reach the declaration at which point the decorator(s) are applied (however you want to define their semantics) to produce the function value that initialized the bound function name.

@zenparsing
Copy link
Member

As the OP illustrates, some libraries like React allow the user to gracefully refactor from a function to a class which implements a stateful version of that function (or vice versa).

Simplified:

interface RenderFunction {
  (props) : Element;
}

interface Component {
  props: object;
  render(): Element;
}

type Renderable = RenderFunction | Component;

If a decorator is useful for Component, then it would seem like that same decorator would be useful for RenderFunction. And in fact, that's how the connect "decorator" from react-redux works. It "decorates" both RenderFunction and Component in the same way.

export const A = connect(...)(
  function render(props) {
    // ...
  }
);

export const B = connect(...)(
  class extends Component {
    render() {
      // ...
    }
  }
);

With the current proposal, we could use the pretty decorator syntax for B, but not for A:

export const A = connect(...)(
  function render(props) {
    // ...
  }
);

@connect(...)
export class B {
  render() {
    // ...
  }
}

This discrepancy is both unintuitive and undesirable.

If for some reason we are unable to have function decorators, we'll be stuck in this unfortunate situation forever. Therefore I think that we need an answer to the function decorator problem in order to proceed with confidence.

@jkrems
Copy link

jkrems commented Aug 17, 2017

@zenparsing That example assumes stage 0 decorators where the decorator is just (fn) -> fn. This is no longer how decorators work. It's not just syntactic sugar for wrapping it in a call anymore.

@zenparsing
Copy link
Member

@jkrems I understand, and that's why I used quotes when describing connect as a "decorator". Doesn't change the argument, though. 😉

@littledan
Copy link
Member

@allenwb We discussed this option before. The obvious downside is the refactoring hazard--it may be surprising to lose function hoisting, among other observable changes, when you just add a decorator.

Other alternatives do seem worse, though. Is this refactoring hazard worth it to everyone?

@iddan
Copy link

iddan commented Aug 21, 2017

It is for me. Currently the community is kind of on-hold with decorators as the babel team omits it from their presets and create-react-app excludes this feature as well. I think as this is only a proposal in stage-2 it has the mandate to change.

@gilbert
Copy link

gilbert commented Aug 22, 2017

I think the refactoring hazard fear is overblown. If you accidentally decorate a function that needs to be hoisted, wouldn't you see the error immediately? For example:

import { wrap } from 'some-lib'

var g = f() + 20

// @wrap <-- This will fail fast
function f () { return 10 }

Uncommenting the @wrap line will throw an error the moment you run your code again; there is no prolonged search for what caused it. It might come as a surprise for a beginner, but it seems like that's the worst it can do.

In many cases (or even most) it won't even matter, such as this slightly different example:

import { wrap } from 'some-lib'

export var g = (x) => f() + x

// @wrap <-- Adding this is no problem
function f () { return 10 }

@littledan
Copy link
Member

@gilbert I guess it shouldn't be so hard to debug because of the quick ReferenceErrors, but this isn't the same as lending itself to an easy-to-understand mental model long-term. Function hoisting is already a bit complicated (e.g., switching to 'let'-style in a block, Annex B 3.3 in sloppy mode); this adds more complexity if you're scanning code and trying to figure out what it's doing.

@iddan
Copy link

iddan commented Aug 23, 2017

But it might be worthwhile. How do they handle it in Python? Are they facing the same problems?

@zhaoyao91
Copy link
Author

I have suggested a so-called composition operator which may take this issue away from proposal-decorators. How would you think about it?

@littledan
Copy link
Member

@iddan Python doesn't do function hoisting. You can define functions top-down rather than bottom-up only as long as you don't actually use them before everything is defined. In Python, instead, the semantics are to define the functions one at a time as the code is executing.

These are the semantics proposed by @allenwb above. They would "work", the only problem is that it's a big break from the way JavaScript works generally. For example, in JS, you can generally do something like this:

f();

function f() { }

In Python, or in JS f is decorated and we adopt @allenwb's semantics, that is not permitted (and leads to a runtime error).

@iddan
Copy link

iddan commented Aug 29, 2017

  1. I relate to the suggestion to only allow decorating var + arrow function expressions. They aren't hoisted as so problem solved
f(); // f is not a function

const f = () => {}

1.1. If we are focusing the discussion to @g const f = () => {}: is it important to support decorating non-functional variables?

@g
const f = () => {} // got you!

@g
const a = {} // what?
  1. I think it makes perfect sense that @g function f() {} will be treated differently than f() but I agree it might be confusing for newcomers
f() // <- yes

function f() {}

g() // <- no

@h
function g() {}

@iddan
Copy link

iddan commented Sep 25, 2017

Any comments?

@littledan
Copy link
Member

I'm not totally satisfied by your solution as many programmers like to use the function declaration syntax, and it seems incongruous to move away from it. I also don't understand why, if you're already using =, it wouldn't be OK to just put parens around the whole thing and use a function call.

I'm thinking that we should handle function decorators as a follow-on to class decorators, not in the first pass.

I have another idea as to a solution to the hoisting problem which doesn't depend on changing the declaration form. The idea is: decorators which are defined in the same block scope can't themselves be decorated--only the set of undecorated function declarations are available in scope when evaluating the decorator expression in the function declaration, and others are defined in an inner nested scope. In practice, this means that a very unlikely case will yield a ReferenceError, which can be avoided by moving the decorator to a separate module (either CJS or ESM would work).

What do you think?

@iddan
Copy link

iddan commented Sep 26, 2017

Can you give a code example of the edge case?

@zenparsing
Copy link
Member

@littledan

Another solution here would be to not allow decorators on classes.

If we only allowed decorators on class elements (and later, property definitions in object literals) we get a couple of benefits:

  • We avoid the hoisting issue with functions.
  • The scope of decorators becomes more well-defined. The current proposal seems to be expandable to just about everything in the language. There is too great a temptation to use decorators to solve every DSL problem. This will lead to unfathomable messes of code. Restricting decorators to inside classes or object literals focuses them on their primary use case: object shape metaprogramming.
  • There is far greater syntactic flexibility within class bodies and object literals. There is no need to claim this last reasonable sigil @. Claiming unnecessary syntactic space creates risks for the evolution of the language (and indeed some of your other proposals).

@littledan
Copy link
Member

I don't really understand why saying no to one feature request will make another feature request go away...

@zenparsing
Copy link
Member

Because it clearly defines the scope of the feature.

One of the main risks that I see in introducing decorators into the language (and, to be clear, I think the risks of the current proposal outweigh the benefits) is that, as a metaprogramming feature blessed with premium syntactic space, they are subject to scope creep. With the current design, scope creep is almost guaranteed. Various parties want to decorate functions, parameters, variable declarations, etc. all for ostensibly valid use cases.

In my opinion, this will not help the language long-term, especially a language that is so flexible already. We already have transpilers; there's no need to open up this pandora's box.

@littledan
Copy link
Member

Function decorators could definitely be useful. This proposal leaves them out, but they could be good to pursue as a follow on. Do you see anything here that would block that path?

@littledan
Copy link
Member

Let's continue the discussion at tc39/proposal-decorators#40 .

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

No branches or pull requests

10 participants