-
Notifications
You must be signed in to change notification settings - Fork 127
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
different angle on functions decorators: classes should not be decoratable #48
Comments
👎 |
@kobezzza Any constructive feedback to offer? @saurik I'm with you. I get the argument for them, and I can see them potentially being useful as a declarative mechanism, but it is also important that the syntax be consistent, and it seems totally reasonable to me to limit the discussion to only descriptor-level decoration for now so that it's possible to reach a clear consensus on the proposal. I'll be curious to hear what others think. I'd be very in favor of leaving statement-level decoration for the future. |
Angular2, Iridium and other amazing projects uses class decorators and it's real powerfull and sugar feature. Do you want to kill it? |
@kobezzza is right, decorators it is powerful tools in ja |
The question for this specific case is, is
so much more powerful than
that it would prompt rushing to add inconsistency to the language. No-one is arguing that they aren't powerful. My motivation for siding with this is entirely that it's not clear how it could be added consistently with everything else. If it can be done in a consistent way that won't lead to confusion, I'm all for it, but I'm not convinced that is the case. "People are using it" is a useful metric for making decisions, but it is not the only thing that should drive the addition of something into a language. There is nothing provided by class decorators that you can't already get without them, whereas that is not the case with method decorators, which is why they are cool. |
I do not see a problem in having both free function decorators and class decorators work exactly the same way, e.g.
vs
While I can see that from the context alone the decorator cannot decide whether it is decorating a class or a free function, additional information needs to be passed to the decorator, e.g. {isclass:boolean} as the last parameter. Of course, one can still declare standard functions as classes, e.g.
Yet, I do not see any problem here as the developer who uses these idioms will know that this is a class rather than a free function. And I also do not see why one cannot simply have both notations, as it is perfectly fine with Python, provided of course, that the @ notation will also be made available for free functions, e.g.
The latter two, however, are indeed more complicated as decorate() could also expect a callback, and return any value or none at all. While the last example could be solved by the compiler using some kind of back tracking and traversing the AST upwards, the decorated function will go unnoticed by the compiler. Yet, I do not see any problem here. And the same is true for Python. The syntactic sugar of having @decorate will be resolved by injecting a call to the decorator in the AST. As for the direct use of the decorator, the user will already provide for that call and the compiler will have no need of knowing whether the function gets decorated or not. |
@silkentrance Supporting decorators on function declarations has been discussed tons in #4 I'd suggest starting there to understand the concerns around supporting them. Essentially it comes down to hoisting being problematic though. |
@loganfsmyth sure there are a lot of things involved, yet I wanted to shed some light as the issue whether class decorators are useful or more powerful than by decorating a class in the way of
and why that notion would even be considered at all. Looking at the above code I would just say no and stick to class decorators as they are and not question their existence. Instead, focus should lie on whether it is desirable to not have the same feature for free functions as well, and of course, always also allowing the user to call decorators directly. In Python the co-existence of both approaches has worked very well. |
I was primarily responding to
We can potentially have both, but function declaration hoisting makes the behavior of decorated function declarations potentially unexpected and complex. The question is then, if we don't have it on functions, is it worth having on classes, and then needing to have a potentially unexpected difference in syntax.
Yes, you'd need to understand the function signature when calling the function, rather than relying on it to be called automatically for you, but that's already something all developers are already accustomed to, for the callback portion, and |
@loganfsmyth In my project, https://github.com/coldrye-es/inxs-common, and https://github.com/coldrye-es/inxs, I refrained myself and also users of that framework from using class/constructor decorators. The reasoning here being that one is not able to replace the constructor function, at least, I did not see any way to make this possible, given the current implementation of the decorator transformer, legacy or not. But, and since I am working on this in my early/late night spare time, I might be overlooking something in the generated code. However, with some indirection provided by babel I might be able to support class/constructor decorators. The main purpose of such constructors then being to (a) inject parameters into the constructor and to (b) enhance the class and (c) altogether override its declaration. Considering (c) this would allow us to have Meta Types, e.g. the decorators would handle the Meta Type and then will return an enhanced/accommodated version of that class, or target when talking decorators. |
@loganfsmyth As for function hoisting, I do not remember Javascript for doing that for a long time. Instead I remember it to have functions declared before they are being used, lacking the prototypes that for example C or C++ offers. As such I do believe that the decision made towards introducing function hoisting was a bad one. Yet this is not to be discussed here. |
Yeah, hoisting has its uses but it does make this difficult.
You can in fact. Just as a method decorator can mutate the descriptor passed in or return a new descriptor, class decorators can mutate the constructor passed in or return an entirely new constructor, e.g.
will print |
@loganfsmyth so class decorators ftw? And thanks for pointing this one out, as I was unable to find this one out by myself. |
@loganfsmyth That did not replace the constructor in the same way I think (though maybe I am wrong) @silkentrance meant: it replaced the "entire class" (which I put in quotes, as there is now a terminology clash from having added class syntax to JavaScript). @silkentrance On "so class decorators ftw", that is of course identical to `var Bar = replaceWith(Foo)(class {});": the class decorator here is adding only the minimal possible syntax sugar, and is doing it with semantics that are different from method decorators. I am someone who programs (often sadly) a lot in Python 2. And I use decorators a lot ;P. I have been known to teach classes where I just go into advanced usages of decorators. I layer decorators on decorators and even use them to provide something similar to Ruby block syntax. I have, on many occasions, used decorators in ways that are barely defensible. In fact, I'm started putting together examples of how I've been using decorators to put into this comment (to show the crazy corner cases), and it started to become kind of embarrassing ;P. The reason this pattern is so compelling in Python is because "everything is the same".
The result of this is that if you implement a naive version of memoize, it "just works". It always "just works". I will actually say it is epic just how well it "just works" in every situation. Here is a demo of memoizing, in Python, all of a function, a class, an instance method, and a static method. import pickle
def memoize(storage, prefix):
def wrapped(method):
def replaced(*args, **kw):
key = prefix + ":" + pickle.dumps((args, kw))
if key in storage:
print "cached!", prefix, ','.join(str(arg) for arg in args)
return storage[key]
value = method(*args, **kw)
storage[key] = value
return value
return replaced
return wrapped
store = {}
@memoize(store, "function")
def fib(value):
if value < 2:
return 1
return fib(value - 1) + fib(value - 2)
@memoize(store, "class")
class Test:
def __init__(self, value):
self.value = value
@memoize(store, "instance")
def add(self, inc):
return self.value + inc
@staticmethod
@memoize(store, "static")
def wow(thing):
return thing * 10
def __str__(self):
return "Test=" + str(self.value)
print fib(5)
print Test(0)
bob = Test(5)
print bob
print bob.add(10)
print bob.wow(20)
print
print "ROUND 2"
print
print fib(5)
print Test(0)
fred = Test(5)
print fred
print fred.add(10)
print fred.wow(20)
print
for item in sorted(store.items()):
print item
Yes: I'm seriously memoizing the add instance method there without caring that self is an argument, and that's correct behavior as the return result of an instance method will depend on the value of that object (which I'm obtaining in this case using pickle, so as to memoize to memcached.) (In case anyone reads this and questions the general applicability of this because I happened to use pickle to get a class "value", I could have alternatively implemented a custom comparator on Test: then I could have stored (Also, I actually did end up getting burned by pickle on this thought process: after posting this I decided to go back and add some new functionality to this memoize implementation, which would save a pickled copy of the object, making this really work with memcached, and that ended up requiring me to add a default The result of trying to wedge this syntax into JavaScript simply doesn't have this level of beauty. Python just so happened to be designed in a way where this ended up making sense: there is no "static" keyword, instead class Bob:
def __init__(self, value):
self.value = value
@staticmethod
def one(value):
return value + 1
print one
def two(self):
return self.value + 2
print two
Bob.one = one
Bob.two = two
bob = Bob(10)
print bob.one(20)
print bob.two()
Yes: I can use (You can actually use JavaScript, in comparison, is a mish-mash of different semantics. But thankfully, JavaScript also just doesn't need them in all these cases, given that JavaScript has good support for anonymous functions, and even decided to maintain that tradition by supporting anonymous classes. But trying to wedge in the various forms of decorators, without the underlying "wow, this is all the same" that Python has, leads to a fractured set of decorators that not only are slightly incompatible in where they can be used, but actually take different numbers of arguments ;P. I do think the concept adds a lot of value to the method examples, as there is otherwise no way to modify the descriptor, and being able to replace functions with getter and setter pairs is an interesting use case (though #2 :/). But class decorators do not, to me, seem to be a positive addition. |
@saurik thanks for the exhaustive example on how decorators should not be used and Python lacking proper sanity checks in the built in decorators 😀. However, and for the sheer bang of syntactic sugar, I still do believe that class decorators are a valid option over first declaring a class and then replacing its declaration with a decorated version of it, e.g.
The latter is much more easier to read and author while the first will have the developer thinking of how to make this work and introduce additional code that can well be generated by the transpiler. To sum it up, not having the class decorators will likely make the code more unreadable and less understandable. And, given the nature of class decorators, a developer is free to choose between the two approaches based on personal preference. But I strongly believe, that most would opt for the @ instead of the direct call approach. And, actually, in my framework I provide the user with the And this is what decorators are made for, providing a simple API to from the most simple to the most complex of frameworks. Of course, as I mentioned before, I currently do not support class decorators due to my lack of knowledge on the inner workings of the transpiler. But thanks to @loganfsmyth I am now able to implement these as well. |
@silkentrance So, I honestly don't know how you decided the takeaway of "a naive implementation of a memoize decorator in Python works in all cases, whereas the implementation provided for JavaScript barely even worked in the one case" was "exhaustive example on how decorators should not be used and Python lacking proper sanity checks in the built in decorators". Even if you chose to ignore the brunt of that argument and decided to focus on my comment about how Regardless, I can't even tell if you are arguing in good faith anymore, as you seem to have gone out of your way to come up with the most brutally awkward way of manually composing decorators that you could have, even defining temporary names for Foo instead of reusing the same identifier (to the point where you even caused yourself to have a mistake in your naming). The code that I, and as far as I can tell also @loganfsmyth, were using as an alternative was not "first declaring a class and then replacing its declaration with a decorated version" but was quite clearly just moving the name of the class to a variable and then directly applying decorators: the result has the application in the same order, and has the same overall structure. let Foo =
registerimpl(
mixin(iface1, iface2)(
class {})); This is the direct translation of decorators into JavaScript: this is what I would expect any sane compiler to generate. The "non-whitespace" character count of the direct translation is +1 per decorator (due to "()" instead of "@") and an additional +4 overall (due to "let="). There is none of the complex boilerplate or repetition or overall awkwardness that you are seeming to insist is present. |
I would agree that the decorator definition is very inconsistent, whether it be between classes and methods, or Should class decorators be removed because of how decorators are currently? IMO, no. The definition should be fixed. The current state is fragile and leaves little room for expansion. Should a decorator simply be a function? Why not do something like this: var decorator = Object.createDecorator({
// regardless of how decorator is used, @decorator or @decorator(), use the method
// below and pass args when @decorator(...) is used
classMethod: (target, name, descriptor, args = []) => {
// do decorator stuff
return descriptor; // should always return a descriptor
}
});
class Foo {
@decorator
method() {}
}
class Bar {
@decorator(1)
method() {}
} How class decorators are defined does seem a bit odd though, specifically that they precede the class definition. @decorate({
foo: "bar"
})
class Foo {...} When using decorators I find myself using them on methods 99+% of the time. I actually don't have a case where I've used class decorators. So on the merit of they seem unnecessary and that part of decorators should be postponed, I agree. |
|
@kobezzza Ah. That is probably the biggest use case. Seems a bit odd they chose to do components like that. React does I currently use Angular 1.x on some projects, but I'm actually migrating those to React instead of updating to Angular 2.x. |
@saurik I will dig right into
First, overall readability is IMO still reduced and, second, documenting will become more difficult to near impossible as existing new tools such as Next, commenting out individual decorators becomes quite a task and will lead to even more reduction in readability.
The latter comment that comments out the extraneous whereas using the class decorators as proposed by the spec
will let And commenting out individual decorators becomes a rather simple task. |
@lukescott what you are describing can for example be found in Python, here one can define classes as decorators that have a Now, in ES this would definitely require a 'meta' decorator such as @decorator to indicate that a given class or function is a decorator function or a decorator class. |
http://aurelia.io/ also uses decorators on classes ;-)
I get the feeling that this is a clash of two worlds.
I think most use cases here within this proposal, as well as the use-cases frameworks like Angular 2 and Aurelia have, would also benefit from the second approach. I just don't see the need that decorators do strictly involve property definitions at all. It should be up to the decorator if he wants to redefine an object property. Honestly, after working with decorators for some time, I really appreciate the current approach because of its simplicity. But it's a simplicity that turns quickly into complexity and bloat if we expand it to all the demands of metaprogramming in JavaScript like functions, function parameters, constructors, classes and maybe even arrays and plain objects? Wouldn't it be much easier to stick to a style that is more of a meta information by syntactic sugar and reflection, rather than the current design that is very specific for classes and properties? @x
@y('B')
class A {} Would desugar to ES6: class A {}
Reflection.registerDecorator(A, 'x');
Reflection.registerDecorator(A, 'y', () => ['B']); Additional reflection functions can then be used to perform certain tasks: // Reactive
Reflection.onDecorate('y', (obj, ...params) => {
obj.reflectedB = params[0];
}); // Imperative
Reflection.getDecorators(A)[0].name === 'a'; // true
Reflection.getDecorators(A)[1].params[0] === 'B'; // true You'd loose typing since decorators will not be functions anymore but desugar into a register call using the name specified in the decorator. But is it really necessary that decorators represent a type? I don't know guys... I'd love to see something that just works out nicely :-( |
For now i use decorators for annotating purpose, like in Java, so for me it looks usefull and consistent to use same syntax for annotating both fields and entire class. |
Additionally to the idea of using |
@gionkunz Not being able to add new or alter existing properties at decoration time and before that the class gets defined is problematic as you are then not able to alter non enumerable instance methods into say enumerable ones. The same would be true for a say |
@silkentrance which I think should be possible using Reflection.onDecorate('readonly', (obj, field, ...params) => {
if (Object.getOwnPropertyDescriptor(obj, field).configurable === false) {
Reflection.makePropertyConfigurable(obj, field);
}
Object.defineProperty(obj, field, {
enumerable: true,
configurable: false,
writable: false
});
}); class A {
@readonly a = 100;
} For cases where Objects are frozen, sealed or extension is prevented or for cases where properties are non-enumerable or non-configurable I'd expect that Why do we have things like freeze, seal, preventExtension, enumerable and configurable? The only reason is to create restrictions that will prevent developers from making errors in the future by accident. However, if we don't allow them to revert those changes by a mechanism that can't be an accident (using reflection), we're simply creating barriers and making our lives hard. Why would you like to have a process that can't be undone? I believe reflection should have the power of un-doing everything that was done to an object. In Java you can use reflection to access private members too. |
@gionkunz Sure, if that were possible. See https://developer.mozilla.org/de/docs/Web/JavaScript/Reference/Global_Objects/Reflect for the API and a compatibility matrix. Most of the features Reflect provides are derived from Object, so while it might be emulated on most browser engines, it will still be limited to the behavior that Object implements. |
Please close as invalid as we definitely need class decorators and also in the way they are specified not only in Python but also in this spec. |
I agree with the OP. Don't close this issue. |
So what about this issue? We need class decorators or java-like annotations for class (see angular2, aurelia and other frameworks), but they are not presented in new spec. |
So, I've read through the issues asking for functions to support being decorated, and I have a different take: I don't think classes should support decorators, which also solves that semantic divide. The only reason to have decorator syntax at all is because of the limitations of object property syntax with respect to being able to alter the value separate from the name and being able to modify the descriptor. This is why the meat of decorators take three arguments: the target, the name, and the descriptor.
I can totally appreciate why developers would feel the need to have this syntax for these cases. I see @nonenumerable and I'm like "wow, yeah!! I'd love to be able to have that, instead of sitting around with the full verbosity of Object.defineProperty". Or being able to alter a function and turn it into a getter and setter: that also adds real power that I didn't have before without pretty painful workarounds where I stop using anything remotely straightforward and refactor to use Object.defineProperty.
However, for both bare functions and classes, there is very almost no difference from just calling the "decorator" directly on the value. The reason that we need this syntax in Python is because the way indentation kills the ability for that language to have anonymous functions, an issue JavaScript does not have. Meanwhile, attempting to support both class decorators and method decorators leads to two forms of decorators with different arguments and different semantics, which just seems strange.
Further, supporting decorators for classes begs the question of supporting them on functions, and in addition to opening the Pandora's Box of function hoisting scope, one realizes that it further emphasizes the semantic difference between the two different kinds of decorators: the various decorators in the draft specification for this feature all sound reasonable to also use with bare functions, but they all assume that there is a target object and a descriptor involved, which isn't often the case with functions.
(And, if we want to compare to Python for a second, we will see that while it supports decorators on various kinds of things, including functions and classes, in all cases the decorator has the same semantics: it returns a function that takes a single value argument and returns a replacement value that will be used for the binding. All of the "meta" information about class members is stored on the member, not in a descriptor of the class, so there is no semantics split between the use cases.)
I thereby will argue that this feature should be postponed for classes (as again: it is effectively the same syntax to just call the decorator passing the class as an argument, it will make people stop asking for this feature to be implemented on bare functions, and, and I really think this is important, it means that there is only a single semantics for what arguments a decorator takes: a decorator fundamentally acts on a target, name, and descriptor) but implemented for members (where it seems valuable).
(BTW: the people who are arguing that people will create degenerate classes in order to get access to decorators, to me, are actually helping provide an argument for this, as they are failing to notice that the different semantics for decorators means that a lot of the decorators people will want to define will only work for class/object members and won't work for bare functions as there will not be any descriptor to modify; a lot of these use cases for decorators, not just the syntax, actually require an object.)
(I constructed this as a separate issue because it is asking for the exact opposite result from the other issues talking about this overall concept. I'm terribly sorry if someone fundamentally disagrees that this is the correct approach to organizing these conversations. FWIW, without threads, I'd think that having this idea in the same issue as the opposite idea would probably make it fundamentally more confusing and, if nothing else, clutter the conversation of people talking about how to support bare functions.)
The text was updated successfully, but these errors were encountered: