tc39 / proposal-decorators-previous

Decorators for ECMAScript
183 stars 16 forks source link

Proposal: Instead of decorating function statements, decorating assignment expressions #32

Closed clarabstract closed 6 years ago

clarabstract commented 7 years ago

I found it very surprising that the decorator proposal does not support the most common Python use-case for decorators: functional wrapping. Having followed the previous discussions on this I understand the problem with hoisting this presents, and I agree that turning function statements into unhoistable expressions for decorators only is too inconsistent.

Of course ECMAScript is not Python and the expectation that decorators ought fill the exact same role is not necessarily grounded in anything. That said, it is still the least surprising expectation just because the syntax does invite comparison. I would also argue that functional composition, is, in general the more useful and easier to follow pattern compared to in-place modification of object descriptors. It is worth supporting if possible.

Thus my proposal is to support decorating arbitrary assignment expressions, as follows:

@decorator
let identifier = expression;

// equivalent to
let identifier = decorator(expression);

// same with const
@decorator
const identifier = expression;

Local name assignment is of course never hoisted, so nobody should ever expect that to kick in.

Ideally SetFunctionName continues to recognize patterns such as let myFunc = () => {...} even with a decorator present so that any functions defined this way continue to have names we can inspect and reflect on.

What's more, this also lets us do wrapped classes that are not modified in-place. This is actually desired for the very common React pattern of "higher order components" which just want to return a new class (or sometimes function) rather than modify a class in-place:

@injectIntl
const MyComponent  = class extends React.Component {
}
// equivalent to
const MyComponent = injectIntl(class MyComponent extends React.Component {
});

Assuming it is not too much to ask that users understand the distinction between function statements vs function expressions, and class statements vs class expressions (and I don't believe it is), then the difference in decorator function illustrated above should be very clear cut: expression decorators apply to expressions only and work as expression wraps. Same way class decorators apply to class statements and work differently from member decorators which apply to class members only.

My only concern is that this does allow using decorators for a few use cases that are probably not a good fit - for example, decorating an actual expression:

// let's not
@sum
let a = [1,2,3,4];

But this is just poor coding style, and I would argue has legitimate use cases as well. It is not uncommon to export some class or component "raw" for use in many different contexts, and then export it separately with a complicated wrap in a different module - for example:

import PureComponent from 'pure-component';

@connect(({someKey}) => ({someKey}) )
const ConnectedComponent = PureComponent;
export default ConnectedComponent;

On that note, it might also be handy (but certainly not required) to allow export variable declaration statements to be decorated as well:

@decorator
export let identifier = expression;

// equivalent to

export let identifier = decorator(expression);

Seems intuitive enough to me.

We can also handle default exports, since we are still just dealing with assigning arbitrary expressions of sorts (we are assigning to the module's default export name):

@decorator
export default expression;

// equivalent to
export default decorator(expression);

Please let me know what you think.

I personally feel that the proposal as-is is just too narrow in scope. It's very specifically targeted at modifying the inner workings of classes and class members, something I have very mixed feelings about using at all.

aluanhaddad commented 7 years ago

While I agree with the use cases outlined and while I share some of your reservations about the current proposal, I think it would be simpler to decorate the target of the assignment, the function expression, as opposed to the binding.

So instead of

@decorator
let identifier = expression;

Why not have

let identifier = @decorator expression;

While decorating the binding "feels" more declarative, it would not only introduce the potential abuses mentioned (@sum), but would also be unnecessarily less general, with respect to function expressions, than it otherwise would be.

The result also misses out on a key use case of class decorators, namely constructor parameter decorators, which would be as useful, if not more useful, on functions then they already are for class constructors.

Also this proposal raises the question of decorating var bindings which are hoisted, albeit in a very different sense than function statements, and do not have the temporal deadzone semantics of let and const bindings.

MMeent commented 7 years ago

A discussion about decorating variable declarations (then named 'decorators for variables') was held on the python-ideas mailing list as well. (here).

It concluded with something along the lines of

Decorators are a pattern created to shorten and clarify function and class creation notation:

  1. to keep the decoration near the function declaration; and
  2. to avoid having to repeat the function name three times. (source)

Both of which are not applicable to variable decorators the way they are written in this proposal.


I also note that the current proposals using descriptor objects for classes and class methods/properties are quite nice IMO, as you'll have access by default to the 'finer grained' properties of a class/method/property. Determining what type you are decorating by checking arg1.type is a nice design, and adapting it for non-class variables would be, well, interesting.

If you want yet another way to call your functions, my vote is against (see above). If you can specify a reason why this decorator is needed other than "I don't want to see my function being called", then please specify, I'm interested as well.

PS. Please note that you give an argument of "What's more, this also lets us do wrapped classes that are not modified in-place", this should be possible using normal decorators, but I'm not quite sure. What do you mean by 'modified in-place' other than 'assigned another value than the class literal'?

clarabstract commented 7 years ago

@aluanhaddad The RHS syntax would be fine too, provided function naming can still see "past" decorators. That said, applying multiple decorators on multiple lines might get awkward ?

let identifier = @decorator1
@decorator2
expression

I don't think that's too ambiguous for the parser to resolve? I would really like to avoid having to bracket the entire RHS if possible.

Could you give me an example of a use case with param? It would only affect parameter default values right? What exactly would it save other than a pair of brackets? i.e. function (param = decorator(expression)) { ... } vs function (param = @decorator expression) { ...}.

edit: Forgot to mention also that the @sum problem doesn't go away - it just becomes let a =@sum [1,2,3,4]. I do not actually think this is that big a problem though. It is bad style, but not as such dangerous. It does what it looks like. There are many other things JavaScript lets you do that are bad style, many of which are dangerous.

clarabstract commented 7 years ago

@MMeent As far as I can tell both of those points are exactly applicable here, and would be solved by this proposal.

The example problem code in python:

def bar():
    pass
bar = foo(bar)

bar is mentioned three times, foo is called too far from the function definition. This is exactly the same problem I have today with function wrapping in ES6:

let bar = function () {
}
bar = foo(bar);

bar is mentioned three times. foo is called too far from the function definition. As far as I can tell it's the exact same situation.

The solution would not be applicable to python because anonymous lambdas do not really fit the same role as they do in ES, and naming lambdas by their assignment identifier has no reason to happen in Python.

On the flip side, the Python solution of decorating function definitions would be just fine in ES, were it not for the aforementioned hoisting problem. Hence this proposal.

Regarding things like checking arg1.type - in case it was not clear: I am suggesting this proposal in addition to the existing class and member decorator specifications, which solve a somewhat different problem (which is still worth solving).

Regrading modifying in-place - perhaps I do not understand the current specification correctly? As far as I can tell, class decorators must modify the class descriptor they are passed in-place - they cannot return a new value. I assume this means the wrapped class' descriptor is being modified, a new class is not being created. I read this as something somewhat closer to Python's __metaclass__ which alters the class creation behavior (it is also in my opinion one of the Python's flakier aspects as it makes subclassing an unreliable operation... thankfully this is not the case here). You certainly cannot turn a class into a function (which is needed for higher order React components).

clarabstract commented 7 years ago

Also, I was unaware var declarations get hoisted.... wouldn't that cause out of order execution of decorators as well? If that is the case, could we just not allow var to be decorated? (in general, the rule can be "if it gets hoisted, you can't decorate it")

MMeent commented 7 years ago

@rubyruy for functions and classes, you are correct:

function functionName(){} functionName = decorator(functtionName);

But with an assignment, that is different. This proposal proposes that @decorator let decorated = statement should be syntactic sugar for let decorated = decorator(statement) (IIRC). This does only trade parenthesis for @-syntax of calling single-argument functions, which I'm not a fan of.


Something I want to ask you: how can my decorator failproof decide what it decorates? right now I check arguments[0].type, but if @dec let var1 = stmnt only transforms to let var1 = dec(stmnt), you wont have access to type if stmnt is not a crafted object with a 'type' field/accessor.


On RHS-decorators: how do I know what decorator type to use/what decorator pattern to use in these cases:

let className = @decorated class {}; /* variable decorator or class decorator? */

let functionName = @decorated function() {} /* function decorator or variable decorator? */

I hope I made my concerns clear here.


variable hoisting is only setting that variable to undefined/creating a place for the variable in the local scope. The assignment happens on the correct location in the code.

Function declaration hoisting happens as well, but all function declarations are still executed in order of occurrence.


I do think variable decorating is an interesting idea, but it'd need a lot of fleshing out.

Maybe something like the following would be better as the following:

function decorator(descriptor) {
  const {type, ...} = descriptor;
  if (type == "variable") {
    {writable, initialValue, accessors, ...} = descriptor;
    //... do something with it
    return decorated;
  }
}

@decorator
let decorated = statement;

which would give decorators only to initial variable declarations (with let and/or const), but allows setting local behaviour for those names, like accessors for typed variables, or something similar.


In regards to class wrapping: it can be done, but you'll need it only in specialized cases, which allows for specialised decorators, which target specific methods and/or functions, though an easy example is:


class Wrapper {
  get inner() { return null };
}

function wrapping (descriptor) {
    let a = (d) => descriptor;
    let innerClass = @a class {}; /* create a class wit the descriptor of the decorated class*/
    let descr = {};
    descr.constructor = Wrapper.constructor;
    descr.parent = Object.getPrototypeOf(Wrapper);
    descr.members = Object.getOwnPropertyDescriptors(Wrapper);
    descr.members.inner = {get: ()=> innerClass};
    return descr;
}

@wrapping
class Wrapped {}

This should result in a sibling class of Wrapper, with the same methods and fields. Not perfect, but good enough.

clarabstract commented 7 years ago

Decorating the assignment is still saving you duplicating the function name three times because of the special case implicit function naming of let foo = () => {...}. It does not work with a wrapped function (e.g. let foo = decorator(() => {...}), what happens if multiple functions show up in the expression? ) - but it could work through the proposed decorator syntax because the identifier still refers to only one function and one function alone.

I will concede that you can get the repetition down to only two by in-lining the decorator:

let foo = decorator(function foo() {
})

It is still repetition though. Having to bracket/unbracket the entire expression when adding decorators is also not ideal. Stacking decorators also requires a runtime compose() type utility function or even more awkward bracketing.

The use case outside assigning function/class expressions is indeed not very strong, but it is also not really the point either. At the end of the day all I really want is a clean and practical way to do normal function wrapping. I strongly suspect it to be the more common use case. Python didn't even bother with class decorators for several revisions.


I don't think I follow. Decorators would have to be grammatically defined as attached to a handful of statements (let assignment, const assignment, named export, default export, etc.). They decorate the evaluated value of the RHS expression of the assignment statement.

If you are asking about having the equivalent of descriptor information in class/member decorators but for the decorated expression's evaluated value, that simply does not exist here, nor could it. I don't think that should be seen as a problem though - it certainly is not relevant to the problem I want to see solved.


I'm not sure I understand your variable descriptor example. If I'm reading it correctly it would let the same decorator do something different depending on what kind of assignment it is assigned to. I think I would much rather be able to assume that changing a const to a let or export let will never affect the operation of a decorator. If you really need that sort of functionality you can simply parameterize your decorator (i.e. it's a function that returns the actual decorator function - very common in Python).

I'm also not sure what accessor would refer to. Surely you don't mean being able to turn a variable declaration into something else entirely? If you just mean the regular object descriptor, why not just Object.getOwnPropertyDescriptor ?


The class wrapper example is good to know about though though we still won't be able to decorate React functional components. Yes, having classes and functions being interchangeable this way is a React-specific quirk, though it is not entirely outside the JavaScript tradition of classes being nothing but functions with a prototype. Classes are still not a distinct concept in ES6+ right? class is just syntactic sugar for creating prototype-functions? (And then as a separate concept we also have object descriptors, but you can use one without the other.)

clarabstract commented 7 years ago

Oh and regarding RHS decorators being used on classes, class decorators would only apply to class statements. I do agree it is not as obvious though. I also don't see the use case for the RHS syntax though, so I'm just as happy without having it.

MMeent commented 7 years ago
let foo = decorator(function foo() {
})

If you're only working on function wrapping/decorating, why not directly decorate the function? instead of

@decorated let foo = function foo() {}

or

@decorated let foo = () => {}

the following looks much cleaner in my opinion.

@decorated function foo() {}

I'm not sure I understand your variable descriptor example.

My variable descriptor for variable decorators was an example for how variable decorators should work in my opinion: they can only be used on the initial declaration that that variable exists (so @decorated var varName, @decorated let letName = variable, but not let letName; @decorated letName = variable), which would then return a descriptor for how that variable can be used, like properties on a class, but then for the local scope. It would have a 'initialValue' field which has the value of the assignment in it, and could have settings like exportName (to export a variable with some name), readonly (true for const, false for let), etc. etc.

I'm also not sure what accessor would refer to.

I meant getters and setters, or something like that. I was just typing out a very basic proposal without using the correct terms, my apologies.


The class wrapper example is good to know about though though we still won't be able to decorate React functional components

I think I saw a TC39-proposal for direct function decorators and their descriptor shape, so you can just check what you're decorating, and act accordingly. inner can then be a function, which wouldn't make react act different from normal.


Oh and regarding RHS decorators being used on classes, class decorators would only apply to class statements

Currently class decorators are designed to work with both class statements and class expressions (see here for formal syntax), so that's be a difficult problem to fix for the syntax.

clarabstract commented 7 years ago

Hang on, I thought decorating plain old functions was problematic (due to hoisting) - as discussed in #26 and wycats/javascript-decorators#4. Am I wrong?

If there can be regular decorator functions then no, we probably don't need this (though I would still like a class decorator to be able to return an entirely different value instead).

MMeent commented 7 years ago

No, there are valid problems with decorating hoisting function statements, which are mainly existential problems with "should we or should we not hoist the function body and/or decoration", and nobody seems to want to pick up this hot issue with function statement hoisting, which was already solved for variables.

let function foo(){} or let func foo() or whatever could be made a valid syntax for this I think it would be worth the cost. Note that es6 added keywords for non-strict mode ('let', thus being practically backwards incompatible), which might indicate adding a syntax for non-hoisted functions is not impossible.


Back to this proposal: I think class/method (+property?) decorators is a good start, so lets finish that first. IMO we can put function decorators in another proposal when there is an acceptable fix for the hoisting problem.

Jamesernator commented 7 years ago

Decorators on arbitrary declarations would be very nice in my opinion, it would allow allow useful name information when generating dynamic classes/functions that can't be simply represented without duplication e.g. I could imagine daggy could be work like this:

const Identity = daggy.tagged('x')
console.log(Identity.name) // "wrapped" not too useful information
Identity(1, 2, 3)
TypeError: Expected 1 arguments, got 3
    at wrapped // "wrapped" definitely not useful here
    at Object.<anonymous>
    ...

Whereas a decorator approach could be something like:

@daggy.tagged const Identity = ['x']
console.log(Identity.name) // "Identity" would be possible now as the decorator would have that info
Identity(1,2,3)
TypeError: Expected 1 arguments, got 3
    at Identity // Much better
    at Object.<anonymous>
    ...

Best part about this would be that it would be opt-in rather than implicit, an idea I had in the past was using a meta property that would be set on function calls, but it's a potential security risk:

function decorator() {
    const result = function() {
        ...
    }
    Object.defineProperty(result, 'name', { value: function.inferredName })
    return result
}

const foo = decorator(function() { ... })
foo.name === 'foo' // true

a.foo = decorator(function() { ... })
a.foo.name === 'foo' // true

Decorating declarations seems a much much better idea as it's opt-in so more secure and easier to transpile (as you only need to transpile explicit uses not every function call).

littledan commented 6 years ago

Would it work to pursue this additional decleared form as a follow on proposal? It seems compatible with class declaration decorators to me.

littledan commented 6 years ago

Let's continue discussion in https://github.com/tc39/proposal-decorators/issues/51 .