-
Notifications
You must be signed in to change notification settings - Fork 16
Proposal: Instead of decorating function statements, decorating assignment expressions #32
Comments
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 ( 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 |
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
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 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'? |
@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 ?
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. edit: Forgot to mention also that the |
@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:
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 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 |
Also, I was unaware |
@rubyruy for functions and classes, you are correct: function functionName(){} But with an assignment, that is different. This proposal proposes that Something I want to ask you: how can my decorator failproof decide what it decorates? right now I check arguments[0].type, but if 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 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. |
Decorating the assignment is still saving you duplicating the function name three times because of the special case implicit function naming of I will concede that you can get the repetition down to only two by in-lining the decorator:
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 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 ( 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 I'm also not sure what 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? |
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. |
If you're only working on function wrapping/decorating, why not directly decorate the function? @decorated let foo = function foo() {} or @decorated let foo = () => {} the following looks much cleaner in my opinion. @decorated function foo() {}
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
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.
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.
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. |
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). |
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.
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. |
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). |
Would it work to pursue this additional decleared form as a follow on proposal? It seems compatible with class declaration decorators to me. |
Let's continue discussion in tc39/proposal-decorators#51 . |
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:
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:
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:
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:
On that note, it might also be handy (but certainly not required) to allow export variable declaration statements to be decorated as well:
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):
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.
The text was updated successfully, but these errors were encountered: