-
Notifications
You must be signed in to change notification settings - Fork 107
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
Alternative proposal: property read/write trapping decorators #299
Comments
I think this is where we need a central list of use cases to judge variants against. I don't see this proposal solving several of them. |
Thanks @mweststrate for this detailed proposal. You start with an important assumption:
In my humble opinion class decorators are important and solve many problems and user cases that are not contemplated in your proposal, which is exclusively focused on properties. |
This comment has been minimized.
This comment has been minimized.
@justinfagnani I tried to solicit a list of use cases in #231 . If someone wants to help put that all together to find themes, it'd be really helpful. What I tried to do was list some canonical use cases in the explainer, derived from what I'd generally heard about what was useful. |
In Angular we primarily use decorators for attaching metadata to classes, properties, and constructor parameters. The semantics described here should be good enough for properties, although we really don't need to "trap" them. Regarding classes, we don't have complex runtime requirements, so:
should be good enough on our side even for JIT.
I don't think we'll ever move from custom build & runtime time transforms. We compile the templates declared in the component metadata to a format similar to incremental DOM, so we can speed up rendering and change detection. |
In Aurelia 2 we also use them for this purpose, but I see that as the opposite of "they cover our use cases". Do we really want to force every feature-rich framework down that path? Using decorators purely for static metadata to do runtime rendering (or build time codegen) means you won't get any native performance benefits. You're still relying on the browser's JIT for all the behavior produced by the metadata. What's the point then? So let's talk about a use case that stands out to me the most: Here's what we do in Aurelia with export class NameTag {
// short-hand for: @bindable({ attribute: 'first-name', callback: 'firstNameChanged', mode: BindingMode.toView })
@bindable firstName;
@bindable lastName;
firstNameChanged(newValue, oldValue) {
console.log(`firstName changed from ${oldValue} to ${newValue}`);
}
lastNameChanged(newValue, oldValue) {
console.log(`lastNamechanged from ${oldValue} to ${newValue}`);
}
} What this translates to at runtime, looks something like this: export class NameTag {
set firstName(value) { this.$observers.firstName.setValue(value); }
get firstName() { return this.$observers.firstName.getValue(); }
set lastName(value) { this.$observers.lastName.setValue(value); }
get lastName() { return this.$observers.lastName.getValue(); }
firstNameChanged(newValue, oldValue) {
console.log(`firstName changed from ${oldValue} to ${newValue}`);
}
lastNameChanged(newValue, oldValue) {
console.log(`lastNamechanged from ${oldValue} to ${newValue}`);
}
} In summary, the runtime renderer wires things up in such a way that incoming changes are sent directly (and only) to listeners that need those changes. In the case of a custom element, that would be for example a text node (which is wrapped by a light-weight accessor object to avoid unnecessary DOM reads). This is what incremental dom mentioned by @mgechev more or less boils down to and from a performance point of view this is vastly superior to any VDom approach. There are more useful configuration properties for tweaking performance and control flow but this is the main thing I wanted to touch base on first. How do other people feel about a per-property callback granularity? Or in general, configurability via object literals? |
Thanks for the feedback guys! I updated the original proposal above to clarify more stuff.
@pabloalmunia @justinfagnani let me know if this satisfies all (important) use cases for you. Sorry for the initial confusion, I didn't intend to limit decorators to properties, but just this issue ;-). As my impression the biggest problem with the previous proposal is the possibility to establish properties in to much unpredicatable ways (from VM perspective) @littledan @mgechev with the above changes, attaching meta data only should no be a first-class supported pattern @fkleuver I agree that tracked is the most interesting case. With this proposal the runtime trap would be a s simple as the following, without any further compile step: const bindableTraps = {
get(_target, instance, prop, _value) {
return instance.$observers[prop].getValue()
},
set(_, instance, prop, value) {
instance.$observers[prop].setValue(value)
return undefined
}
}
const bindable = () => bindableTraps |
@mweststrate Thanks for the example. So if I understand correctly, interacting with those properties would give you the power and flexibility as if you were talking to a proxy, but without actually talking to a proxy. Is it also possible to let users pass in arbitrary config into the decorators with your idea? I'm sorry if that's already detailed in your proposal or elsewhere, but I couldn't find it: e.g. bindable.js // Module-scoped lookup
const lookups = new WeakMap();
function getObserver(instance, prop, config) {
if (!lookups.has(instance)) {
lookups.set(instance, {});
}
const lookup = lookups.get(instance);
return lookup[prop] || (lookup[prop] = new Observer(instance, prop, config));
}
// The decorator
export function bindable(config) {
return function () {
return {
get(_target, instance, prop, _value) {
return getObserver(instance, prop, config).getValue();
},
set(_, instance, prop, value) {
getObserver(instance, prop, config).setValue(value);
}
};
};
} date-picker.js export class DatePicker {
@bindable({ mode: BindingMode.twoWay, callback: 'dateChanged' }) day;
@bindable({ mode: BindingMode.twoWay, callback: 'dateChanged' }) month;
@bindable({ mode: BindingMode.twoWay, callback: 'dateChanged' }) year;
dateChanged() {
// ...
}
} If the traps idea is this flexible, then from a property decorator standpoint I'm pretty confident this would address all our needs. EDIT I missed the |
You're correct :). |
Then this has my vote :) @littledan From a framework author's point of view (one that does a lot of fancy stuff with decorators), this proposal looks perfect as far as property decorators is concerned. |
This looks like a really nice proposal and I would love to play with it if there is a babel/typescript plugin. |
I tried to port some decorators i use currently to this proposal and found is not clear what parameters are expected to be received in the decorator. In metadata example Below are some of my cases that (to be used in custom elements):
// usage:
class MyView extends MyBaseElement {
@state({copy: true}) model
} This is how i would implement with this proposal where function state(options) {
return (target, name) => {
registerStateProperty(target.constructor, name, options)
return {
set(t, i, p, v) {
// if (!initializingField) -> possible to do that check?
instance.queueUpdate()
return v
}
}
}
} Is this right? Static implementation: decorator @state(options) {
@initialize((instance, name, value) => instance[`__internal_${name}`] = value)
@register((target, name) => {
registerStateProperty(target.constructor, name, `__internal_${name}`, options);
})
} |
Other use case i have with decorators is delegating some events, done at class constructor. In static proposal i can do: decorator @event(eventName, selector) {
@initialize((instance, name, value) => delegate(selector ? this.renderRoot || this : this, eventName, selector, value, this);)
} Seems is explicitly not possible with this:
I could implement by setting some metadata and in constructor of a BaseElement loop over the events to be delegated (just like i do in the decorator version implemented by Babel) |
During field installation
Correct, this would require the same workaround as in Babel. Unless you use a class decorator, because than you could do something like to wire up logic after the constructor. This approach might or might not be too obtrusive function classDecorator(klass) {
return class extends klass {
constructor() {
super.apply(this, arguments);
fireTheEvents()
}
}
}
@classDecorator class someClass {
} |
Hello. I'm the author of Corpuscule, the web components framework built with decorators. When I created this project, I used the obsolete stage 2 decorators proposal; when it was deprecated, I rewrote it with the emulation of static decorators using the legacy proposal + custom babel plugin. Unfortunately, I cannot say that the trap proposal would work for my project. It is too small; actually, I cannot distinguish its functionality from the legacy proposal for sure. As I can see, all that can be done with the trap proposal can also be done with the legacy proposal (correct me if I'm wrong). For me, the essential part of the obsolete and static decorator proposals is the ability to inject an action to the class constructor. You cannot do the same without Babel plugin because we don't have any constructor reflection in JavaScript. So missing this feature re-creates the same issues we had with the legacy proposal. For example, let's imagine the following example (this is a simplified example of Corpuscule Form, which uses the final-form internally). @form()
class Form extends HTMLElement {
@gear('formApi') form;
} The class decorator defines that the With the static proposal I can do the following: const sharedProps = new WeakMap();
export decorator @gear(role) {
@wrap((klass, name) => {
sharedProps.set(klass, {[role]: name});
})
}
export decorator @form() {
let $formApi;
// necessary to get proper timing because the order of execution is following:
// class @wrap -> prop @wrap -> class @register -> prop @register
@register(klass => {
({formApi: $formApi} = sharedProps.get(klass));
})
@initialize(self => {
// $formApi is known only during the instance initialization!
self[$formApi] = createForm();
})
} This pattern is widely used in Corpuscule; furthermore, it is quite useful and allows connecting all the decorators in a single flexible system. And I'm not sure how to implement it using the trap proposal. I would also like to avoid any unnecessary class extending during the work of the class decorator. Repeated in multiple decorators, it would pollute the prototype system with unnecessary instances of anonymous classes; that is not convenient and, probably, would lead to a performance penalty. The initializers can also be used in property decorators. Here is an example of my implementation of ob const token = createComputingToken();
class Foo {
@observer(token) bar;
@observe(token) baz;
@computer(token) get calculated() {
return this.bar + this.baz
}
} On the first call to the Here is the possible implementation via the static proposal: const registry = new WeakMap()
export function createComputerToken() {
const token = {};
registry.set(token, new WeakMap());
return token;
}
export decorator @observer(token) {
const container = Symbol();
let corrects;
@register(klass => {
corrects = registry.get(token).get(klass);
});
@initialize((self, _, value) => {
self[container] = value;
});
@wrap((klass, name) => {
Object.defineProperty(klass.prototype, name, {
configurable: true,
get() {
return this[container]
},
set(value) {
this[container] = value;
for (const correct of corrects) {
this[correct] = false;
}
}
})
})
}
export decorator @computer(token) {
const correct = Symbol();
const memoized = Symbol();
@initialize(self => {
self[correct] = false;
self[memoized] = null;
});
@wrap((klass, name) => {
tokenRegistry.get(token).set(klass, correct);
const {get} = Object.getOwnPropertyDescriptor(klass.prototype, name);
Object.defineProperty(klass, name, {
configurable: true,
get() {
if (!this[correct]) {
this[memoized] = get.call(this);
this[correct] = true;
}
return this[memoized];
}
});
});
} As you can see, it relies on initializer heavily. I'm not sure if it can be implemented via the trap proposal. Also I would like to mention that the code samples I'm using here is not the current version of the static proposal. I added some things I have talked about in #276. |
I really like the idea of property traps, but I have 2 concerns with this proposal. First is composition, all versions of decorators so far have offered really clean composition. With this proposal you'd have to call another decorator and then use it's traps inside the new one: function foo (target, property) {
const barTrap = bar(target, property);
return {
get (target, instance, property, value) {
const _value = barTrap.get(target, instance, property, value);
console.log('GET:', target, instance, property, _value);
return _value;
},
set (target, instance, property, value) {
const _value = barTrap.set(target, instance, property, value);
console.log('SET:', target, instance, property, _value);
return _value;
}
};
} My second concern is making sure traps don't suffer similar performance pitfalls as proxy. With a proxy if you only define a handler for Object.getOwnPropertyTraps(A.prototype, 'propName');
// { get: [], set: [setLogger] } Separating property traps from decorators could lead to a very versatile set of tools, what I mean by this is that if we treat decorators in a way that allow us to perform any operation on a function setLogger (target, propName) {
Object.addPropertySetTrap(target, propName, (instance, _propName, value) => {
console.log('Set:', instance, _propName, value);
return value;
});
}
const tagged = new WeakMap();
function tag (target, propName) {
let taggedProps = WeakMap.get(target);
if (!taggedProps) {
WeakMap.set(target, taggedProps = new Set());
}
taggedProps.add(propName);
}
function forbid (target, propName) {
Object.defineProperty(target, propName, {
get () { throw new Error(`property "${propName}" is forbidden`); },
set () { throw new Error(`property "${propName}" is forbidden`); }
});
} When decorating a property no return value would be expected. This would drastically simplify the API of decorators, and not favour any specific means of property adjustment. That would open up the possibility of future ideas like property traps. |
I like |
No you don't, they are chained automatically, so the value received by a trap is already transformed by any earlier traps; decorators don't need to know about each other. See 'chaining' in the original proposal for more details
This was addressed in the proposal as well; the hooks can be left out in which cause the default to just passing the value along
That is the original problem that this proposal tries to address.. The first version of the proposal-decorators heavily relied on calling Object.defineProperty from library / decorator code, which is impossible for engines to optimize. The introduction of traps tries to exactly avoid that, eehh... trap by making the object shape predictable and (mostly) avoiding changing object shapes |
@Lodin indeed, with this proposal the way to create initializer logic remains either extending a class (or providing a base class). I'm personally not convinced that having a longer prototype chain is worse than having an initialization mechanism which is hard to optimize by compilers (this was one of the objections against the original proposal) |
Chaining decorators is not equivalent to composing them. A better example would be that you have both a
Static:
Your proposal:
My suggestion of simple decorators:
(Inside foo and bar either traps or modifed descriptors could be used)
Sorry I forgot that property descriptors returned from |
I don't think engines are going to significantly optimize the application of decorators. The exception maybe being static decorators, if the proposal chooses not to allow dynamic code execution inside the decorator definition, which is a useful feature many are hoping for. I also don't think perf matters that much the instant decorators are applied, what matters is the ongoing operating performance of the classes and instances after. The operating performance is where traps would come in most handy. I really want property traps, that's a major reason why I think they should have their own proposal. It would also be really nice to have the ability to apply them without the use of classes / decorators. |
@robbiespeed my understanding is that the performance hit of dynamic stage 2 decorators was on class and instance creation time, not thereafter - once created, it's data and accessor properties same as anything else. A "traps" proposal (using Proxy's definition of "trap"), however, may run the risk of affecting ongoing interaction with a created object. |
@robbiespeed hey, thanks for your comments. regarding: function foobar (...args) {
...
} I have found shorter but not obviously correct ways to write this - can the language just provide a compose method that composes varargs decorators together? |
It depends on what the decorator is doing, if you're wrapping a value with a new descriptor (most value decorators), you have to create a WeakMap to then reference the old value, and access it through getter and setters. Even getters and setters have a negative performance impact you can see in https://jsperf.com/proxy-vs-obj/. In Firefox property access performance is reduced to 30% just by using a getter (the Traps would at the least, remove the need for a WeakMap, but it would also make it possible to only affect the perf of
It would of course still have an impact, but that impact would/could be lower than it is now. Proxy has issues that this proposal could avoid, proxies handlers are mutable, where as we could make property trap handlers immutable and only modifiable using decorators, Similar to how properties are now either flagged as value, or getter/setter, they could in future have 8 possibilities: value, getter/setter, value+traps(get,set), value+traps(set), value+traps(get), getter/setter+traps(get,set), getter/setter+traps(set), getter/setter+traps(get). Each time property access or assignment is performed you'd look at the flag and determine the fastest path for the operation. Ex: a property flagged as |
@benjamingr @mweststrate
|
As I understand what @littledan said, the current proposal suggests that all the decorators are executed before any optimizations done by the engine, so there should be no performance penalty. That's the basic idea of the static decorator proposal. It means that we would get the constructor injections for free without polluting the prototype chain. |
I'm just curious as to when TC39 will be meeting next to discuss decorators, and what is the current status? |
I'm sorry I've been so behind on this thread. I'd like to seriously consider this kind of alternative. I've added a discussion of decorators, including this alternative, to the March 2020 TC39 meeting agenda (though it's a late addition, so I've asked for it to be deprioritized). If we want to restrict the scope of this first decorator proposal to just field decorators, and just turning fields into getter/setter pairs, then that's an option. I understand that this thread isn't requiring that, but I also am not sure what semantics for those other constructs would be. So let's just talk about fields for now. Some high-level benefits I see to this proposal:
Some questions for when it comes down to details:
|
From Angular stand point a getter/setter proposal with runtime semantics would not be useful. Form our perspective, even only standardizing syntax for class, property, method, and parameter decorators would have much higher value. |
@mgechev Thanks for explaining. Overall, on the "completeness" point, it seems to me like the participants of this thread are in agreement that restricting the scope to only field decorators wouldn't be sufficient for use cases that we all care about. Is that accurate? |
@littledan Yes. Restricting decorators to only fields is a huge loss of functionality. IMO, the Javascript community has been waiting for a prowerful decorator definition that allows transform classes, methos and fields. |
@littledan I also think that limiting scope here wouldn't really make sense. One of the most commonly agreed on use cases across frameworks and the ecosystem, My main concern is that we would advance a solution for class fields, only to have the caveats of that solution block method decorators from advancing. For instance, the hooks proposal as outlined implies that methods would also become accessors, despite many method decorators not requiring access to On a different note, this got me thinking about whether on not we could have different semantics per type of class element. One of the downsides of this proposal is that methods will also be transformed into accessors, which likely has some performance implications. I also think that, in general, this isn't actually really necessary, and may actually result in decorator antipatterns. For example: class Foo {
@decorate method() {
// ...
}
}
let foo = new Foo();
// It would be very a very strange, counterintuitive API
// if `method` were not callable like so
foo.method(); In general I think the expectation is that decorators will somewhat respect the shape of the output. That is, methods will remain callable, and fields and accessors will not be (unless their original value was, in fact, a function). If that's true, then the only time methods really need to be transformed into accessors is if they would result in bound method, like So, we could potentially divide it up like this:
So I wonder if we could have an alternative API for methods, something like: function logged() {
return {
bound: false,
// highly bikesheddable name/API
wrappedMethod(target, instance, prop, value, ...args) {
console.log(`starting ${prop} with arguments ${args.join(', ')}`);
let ret = value.call(this, ...args) + 1;
console.log(`ending ${prop}`);
return ret;
},
};
} Where The main issue I can see with this is, if decorators ultimately desugar to standard properties + accessors, I think decorated methods would still need to become accessors in general to support the I also think it might be confusing if we wanted to support decorators that can operate on both methods and fields/accessors. You could imagine it looking like: function logged() {
return {
// Field/Accessor implementation
get(target, instance, prop, value) {
console.log(`getting property ${prop}`);
return value;
},
set(target, instance, prop, value) {
console.log(`getting property ${prop}`);
return value;
},
// Method implementation
bound: false,
wrappedMethod(target, instance, prop, value, ...args) {
console.log(`starting ${prop} with arguments ${args.join(', ')}`);
let ret = value.call(this, ...args) + 1;
console.log(`ending ${prop}`);
return ret;
},
};
} I'm not sure this is too concerning though, since usage is much more common than definition. This would also give us a way to let users know if they're using a decorator on the wrong type of class element, which could result in better DX. |
I have to chime in, and possibly be a contrarian, and say that I don't think that class A {
boundMethod = (a) => {
console.log(this);
};
} Meanwhile, meta-programming over class fields is literally impossible. It's the critical case to solve with something, and decorators seem like the only remotely concrete proposal to address that. Regardless, implementing |
A solution is to make sure that decorators also work on methods, but that they trap on the prototype instead of on the instance (since methods are prototype members, not instance members like fields). So
This trap is established on the prototype (since it is decorating a prototype member), so once we read from it, we bind the original value (the unbound method) to the instance for which we are reading, and the bound method will from then on hide the accessor on the prototype. So I don't see the performance trap, as this creates only one accessor, on the prototype, rather than an accessor on all instances. Like with any existing implementation of bound, the most expensive part of it is most probably the creation of all closures (+ additional fields on the instance). Also I wouldn't a priori assume getters are slow, that is definitely not my experience, as long as you make sure to recycle the handlers. @pzuraq The rest of your post I couldn't really follow, as accessors and call-ability are unrelated things. One describes how things are read / stored, and the other respresents a value type. I think the |
@justinfagnani using an arrow function as a class field inhibits testing and reusability, because you can't spy on (or borrow) a prototype method prior to instantiating an instance. |
@justinfagnani I think the main reason class A {
boundMethod = (a) => {
console.log(this);
};
}
class B extends A {
boundMethod = (a) => {
// this fails, since the parent method has overwritten the child
super.boundMethod();
console.log(this);
};
} Edit: Also the same reasons @ljharb mentioned, which are pretty solid as well.
I agree here in general, it's one of the reasons I liked the traps proposal. It seems like it would cover most common use cases this way. The only downside is, like I pointed out, the majority of method decorators really don't need to be accessors. I don't know of a case currently where you would want to return, say, a class instance or object in place of a function. So, we could in theory try to do something different there. I'm not sure of what the performance implications are for having all decorated methods become (simple) accessors, if any, but if it ends up being problematic we could explore having different semantics for decorating a class field/accessor and method. If not, a simpler mental model and spec is probably better 👍 |
I wish this criteria were applied to class fields. We're already in a situation where you can't do anything to them at all. What would the prototype representation be for class A {
@bound methodOne() {
console.log(this);
}
}
A.prototype.methodOne = function() {
console.log(this);
};
const a = new A();
const m = a.methodOne;
m(); // what does this print? |
I think that would cause a problem in general with updating the shape of the object at dynamically/arbitrarily after construction, no? That's, as I understand it, much worse for performance than any kind of dynamism during initialization 😬 Maybe decorators could also establish a slot on the instance to ensure that they don't mutate the shape arbitrarily, but that seems not ideal.
So, the majority of common use cases for method decorators actually don't bind, in my experience. A good example would be something like class Foo() {
@memoized method() {
console.log('ran!');
}
}
let foo = new Foo();
foo.memoized(123); // ran!
foo.memoized(123); //
foo.memoized(456); // ran! So while you may not be creating an accessor per-instance, you would be creating an accessor per decorated method when it really isn't necessary. Again, I'm not sure what the actual performance implications are there, but I expect these types of use cases for method decorators to be pretty common and think that in a large app, with 100s-1000s of decorated methods, it could have an impact.
My point was mainly that if we limit the accessor functionality specifically for decorated methods, then we might be able to optimize it. Otherwise, it's arbitrary accessor code, which may or may not be optimizable. Assuming that performance isn't an issue though, I agree, the simpler solution is probably better. |
As @littledan noted (if I understood correctly), doing that when establishing the prototype, is fine. That is no different than doing a bunch of (which is already well optimized). If the decoration process is done only once per class, and doesn't change the predictability of the shape (which this proposal doesn't if it always introduces a property under the same name).
I did this with numbers way bigger than that, and that accessor is going to be the least of your perf problems in such cases :) A property descriptor is a property descriptor, regardless whether it is getter or value oriented, the first is just two function allocations more expensive for the entire app (if I simplify things a bit). As long as the getter itself is efficient (like in the bound example) this will work efficiently. Remember people where worried about the same thing with Proxies, and they are way more complex than this proposal, yet they still perform very will (ok granted, that took a lot of effort and time of the engine implementors, but I hope that can be avoided with this proposal) |
I thought this was the gist of the proposal in the first place. The accessors should be installed on the prototype, not the instance, and the induced backing storage should be installed on the instance. This ensures a consistent shape effect for any field defining decorator. |
Ah, I misunderstood your example. If I'm understanding you and @justinfagnani correctly, then in this example, function bound() {
return {
get(_t, instance, property, value) {
return instance[property] = value.bind(instance)
},
set(_t, _i, _p, value) {
return value
}
}
} But you also say:
Which implied to me that it would shadow this accessor and the getter would never fire again. That was what made me think it would modify the shape of the object (I wrote a similar implementation for our version of So I think in this case, the accessor on the prototype would still be called every time the method was looked up, correct?
They certainly perform well, much better than originally expected, but I've found they still incur a cost and at scale that can have a significant impact. Maybe this is simple enough it wouldn't be an issue though, but I had the idea and thought it might be worth mentioning. |
class C {
@bound
foo() {}
} would be equivalent to: class C {
foo = this.foo.bind(this);
foo() {}
} As for why it doesn't apply to class fields for me, that's because fields are for data - functions aren't data. For React components, I don't need to borrow or mock "state" for all instances of a component - so |
I would like to point out that: class C {
foo = () => {}
} does not, in any way, define a method, just so we're all on the same page :D we mustn't lose sight of the prototype and why it exists. |
@NullVoxPopuli bound methods necessarily have to have a different instance per class instance. They can't be stored on the prototype. |
Lets throw the concept of 'methods' out of the window here, as that is the thing that adds more confusing to the convo here than it adds value, as it is a concept that doesn't exist technically at all in JavaScript (semantically speaking, not syntactically): class X {
@decorator1
field = y
@decorator2
method() { }
}
But That is why the proposal distinguishes the So binding is in the end just a value transformation, just as much as a decorator that converts celsius to fahrenheit. If you can express the one, you can express the other. |
Totally agree about your point with semantics here, but I think that the syntactic distinction could still be valuable, in that it gives us a way to change the semantics of decorators if we choose to. We can have different semantics per class element type, and that's really the only wiggle room we have for semantic differences with a non-static-decorators style approach. And again, stepping back, I do agree that the mental model you propose is simpler and easier to grasp if you understand the basics of JS and prototypical inheritance. If everything boils down to an accessor, it is much easier to reason about. But we're here in the first place because of performance, so I think it's important to consider all options, think them through, and consider the tradeoffs.
Ours does not, we use a WeakMap to associate the function with the instance. For bound functions, I think this is an acceptable cost vs changing the shape dynamically. We've found that dynamic object shapes can cause significant performance issues in general. |
The bound method is per-instance, but the method being bound is not and should not be. |
There have already been some long discussions about the bound decorator in other threads in the past. Let's set aside the discussion about whether or not it's a good use case for decorators—the more important question is whether or not the ability to implement things like a bound decorator (i.e. adding a new instance property to an object via a decorator) is something we need decorators to support. For more details on the various motivations for a |
Really? You don't think any of the other use cases that people have suggested, and which are already being used, justify the existence of decorators? This seems like a strange statement coming from you, especially since decorators were often presented as a way of filling in some gaps left by the class fields proposal (although maybe it was more @littledan doing that). Not trying to bring up old debates, just trying to understand your opinion. Also, in terms of React, writing new components using classes will probably be quite rare in a few years if it isn't already, so I'm not sure that a native |
@mbrowne i'm not saying those aren't also important (including "set" vs "define", etc). I'm saying that the feature is incomplete for me without "bound", and it's not worth adding an incomplete syntactic feature. As for transpilers "solving" the problem for now; if that's the case, no language feature would be needed at all - I don't consider code transformations to be a solution. |
Ah, I see.
The only reason I was bringing that up is because by the time native decorators ship in browsers, React classes will likely have mostly fallen out of use anyway (at least on new code). |
I don't plan on stopping use of them; I find them simpler and more readable in many cases than hooks. I can't speak for everyone else. |
(Off-topic) I can understand preferring classes. (I think if personal preference were the only factor to consider, I might continue using them too.) They will probably eventually be moved to a separate package though, and I predict that they'll become a legacy feature that almost no one uses at that point (due to lack of ongoing support/updates and the option to reduce bundle size by not using them). I guess this isn't completely off-topic in the sense that TC39 decisions probably shouldn't be driven very much by React classes going forward, as that use case will no longer be a major focus of the community. |
@mbrowne, but React is not a single framework in the field. Web components are also based on classes, for them the class shape won't go anywhere because it is a standard. And the |
I always thought the idea behind this proposal was to "merge" it with the static proposal. The static proposal for the class-level use cases and this one for the field-level use cases. Did I misunderstand the intent? As for pure metaprogramming, why not keep the idea of runtime-accessible metadata? Angular's AOT can optimize them away, and at the same time we at Aurelia can still offer the decorator+class api in "script tag" scenarios. The syntax is important at class and at field level, not just for AOT, but also for tool-free situations. Tool-free situations are not the common use case but they are an important one, so performance usually doesn't matter nearly as much as it at least being possible. A somewhat contrived example where just metadata (no accessor) on a class field would be great: @customElement('fancy-text')
export class FancyText {
@oneTime label;
@twoWay value;
valueChanged(newValue) {}
} <fancy-text label="foo" value.bind="value"></fancy-text> The In any case, key points for us:
|
Is this proposal still being considered? And I don't quite understand how this would work...is it something that could be added in a second version of decorators, or does the decision need to be made before the first release? I'm still concerned that the current static decorators proposal is not powerful and customizable enough. |
The current proposal in this repo is roughly based on this issue, so for the current state, just check the readme :). To avoid confusion will close this issue now. |
Trapping decorators
Motivation
This proposal is a proposal to introduce decorators that have fewer concepts, and are hopefully easier to implement by JVM authors.
The fundamental difference in comparison is that this proposal does not attempt to process and modify property descriptors on a target. Rather, it recognises that modifying the property descriptors has always been a means to an end for 95% of the decorators: Namely a way to trap reads and writes. To transform values.
The motivation behind this proposal is that we want to trap reads and writes to properties. Very similar to the functionality that Proxies provide so successfully. The main difference is that decorators trap all interactions with the decorated instance, and that an entire 'type' is trapped, rather than specific instances. Things that cannot be done by Proxies.
The rest of the proposal is a proposal to achieve the "property trapping". Probably any other proposal would work for me personally as well. If engine or library authors want to adjust this proposal for whatever benefit: I think the wiggle room is there, as long as it can achieve the goal of trapping property reads and writes per instance, but being able to declare such traps for an entire type.
A big benefit of traps is that they often avoid the need of introducing additional properties. E.g. if there is a getter / setter to normalize input or make some assertions, currently it is needed to introduce an additional property to store the getted / setted value. With traps. this can collapsed into one property, avoiding the need of patterns like
_name = "x"; get name() { return this._name }}
etc.The proposal
Property traps
A property trap is an object that provides two functions, named
get
andset
. The goal of these traps is to be able to trap any read and write to a property. Traps can be used to achieve two effectsThe signature of a
PropertyTrap
object is:Field Decorators
A decorator syntactically is a prefix of a class or object member.
Syntactically:
@<DECORATOR1> <...DECORATORN> <MEMBERDECLARATION>
Everyone of those expressions is evaluated and should resolve a function that returns a PropertyTrap. The functions get's passed in the target that will receive the property definition, and the property name. In other words, the signature of a field decorator is:
If a field decorator does not return a trap, the decorator does not install additonal traps. However, the decorator could still be valuable as it can have side effects, such as storing meta data. (example:
@serializable
,@deprecated
etc)A first example
Semantic notes:
When construction the initial property descriptor, the traps are already applied, so that means that the
@logged
traps are applied during the construction of memberx
on classC
above, and it is the transformed value that ends up in the initial property descriptor. Which is the reason we can observe side effect(1)
above.In the traps
target
represents the (to be) owner of the property descriptor that is being created.instance
represents thethis
context that is used during a property access. When constructingC.x
above, there is atarget
(C.prototype
) but not athis
context, hence noinstance
argument.On the first read, the read is handled (or intercepted upon) the
C.prototype
, and the context isc
, which we can observe in the output of(2)
.When writing to
c.x
, this will result in a new property descriptor on the instancec
. This is again represented in the arguments passed to the traps. Note that again the traps are applied during the computation of the new descriptor forc
(the traps are inherited, similar to other properties of the original property descriptor, such asenumerable
andwriteable
).Note that the original decorator expression
@logger
is not re-evaluated again! Rather, the traps in which this originally resulted are reused and inherited by the new property descriptor (see below).If we read from
c.x
again, this read is now handled by the prop descriptor on the instance, which is reflected in the arguments passed to the traps: the property owner equals thethis
now.Storing traps
Traps are stored as the
traps
property on a PropertyDescriptor. When copying a property descriptor from one object to another, thosetraps
are preserved. So in the above example the following would hold:(Note, could also be stored somewhere else, as long as semantics are similar to here, and traps can be requested through reflection)
Reflection
As shown above traps are detectable trough reflection (this is different from Proxies).
Also, traps can be by-passed at any time, for example:
This is intentional: it makes sure that developer tools such as the debugger don't accidentally trigger side effects, makes it easy to inspect the underlying data structures, and provides an escape hatch from the traps when needed (either by libraries or users). it is very well conceivable that traps themselves use this trick to bypass themselves (e.g. a
set
trap might use this to get the original value, but not trigger the side effect of it's correspondingget
trap)Getters and setters
Traps are just pass-through things, so they don't necessarily operate on just
value
based property descriptors, but work forget / set
based descriptors as well. Except that the last category will not hit the traps when the property is originally declared (since there is no value to be initialized)Trap initializer
Decorators can be parameterized by creating functions that return decorators. For example:
Note again that in the property descriptor for
obj.x
the resolved value of the decorator expression is stored, in this case a logger trap object whereenabled=true
is trapped in its closure.Chaining
Decorator can be chained, which means that all the traps are applied from inside out (or, right to left):
In the
traps
property of the descriptor this results in an array with the order of the traps as they will be applied:Classes and function decorators
Class and function decorators can be large kept as they are currently implemented in babel and typescript
For example
@a class X
is desugared toconst X = a(class X)
just as currently implemented already bybabel-plugin-legacy-decorators
and typescript'sexperimentalDecorators
flag.Two open questions raised by this, on which I don't have a strong opinion:
export @something class
(see next question)(if somebody could point me to a more accurate write down of the currently implemented semantics, that would be great)
Engine impact (and summary of the proposal)
This proposal tries to keep the impact on JS engines very limited:
set
handlers of the traps this results in are applied to the value the property descriptor would be initialized withvalue
slot of the descriptortraps
slot of the descriptor (only if there were any)set
handlers of those traps are applied first, and the resulting value is stored in.value
of the descriptor / passed to theset
handler of the (potentially new) descriptor.value
of the descriptor (or: the value that is returned from theget
handler of the descriptor) is passed trough allget
traps of thetraps
stored in the descriptorReference examples
Not all examples of the current readme are here:
@defineElement
decorating entire classes is not in this proposal (see below).@metaData
didn't work it out, but should be doable@frozen
,@callable
applies to a class, so skipped@set
cannot be done in this proposal. So field initializers in subclasses would need to receive the same decorators as the superclass if needed.@defineElement
@metadata
@logger
@Frozen
@bound
@Tracked
@syncTracked
Non goal: modify signature
In previous decorator proposal, it is possible to significantly modify the property descriptor, or even the entire shape of the thing under construction. That is not the case with this proposal; decorators can not influence where the descriptor ends up, it's
enumerability
,configurability
, it's type etc.The only thing that the decorator can influence is
value
of the property descriptor (except for getters / setters)target
, however, this is considered bad practice and is typically best postponed until we trap a read / write with a knowninstance
.Non goal: run code upon instance initialization
This proposal doesn't offer a way to run code guaranteed when construction a new instance, although code might run when a trap is hit during a read or write in the constructor
TypeScript
Traps don't modify the type signature at this moment. Technically, they can be used to divert the type of
x.a
fromObject.getOwnProperty(x, "a").value
, but that doesn't seem to be an interesting case to analyse statically.More interestingly, traps can be used to normalize arguments, which means that the type being read could be narrow than the type to which is written. For example:
In the above example, the "write type" of
age
property would beany
, while the read type would benumber
. However, TS can at the moment not distinguish between the readable and writeable property, and the same limitation already applies to normal getters and setters.Edge cases
It is possible to omit
set
orget
from a trap. The absence of such trap represents a verbatim write-trough (just like Proxies). See also theasInt
example aboveInstead of storing the decorators on the property descriptor, it probably suffices to be at least able to detect them trough reflection, e.g.:
Reflect.getOwnDecorators(target, property): PropertyTrap[]
Optimization notes
Like proxy handers, property traps are shared as much as possible, so they are not bound to a specific instance, type or property, but rather receive those as arguments.
An assumption in this proposal is that when writing to a property that is defined in the prototype, the original descriptor is copied onto the instance, and we could just as well copy the traps with it. However, if this assumption is wrong, this proposal doesn't strictly require this copy-traps mechanism; as the trap on the prototype could take care itself of the copying the traps in cases where this is need (for example, in
@tracked
it is, but in@bound
it isn't).The reason that both the decorator and the traps themselve receive the target is that this makes it much easier to reuse traps among different properties and classes. Probably this will be much better optimizable, just like proxy traps are a lot more efficient as soon as the handlers are reused. So for example, it would be adviseable to hoist traps from the decorator definitions whenever possible:
Note that this optimization notice applies to most examples above
update 27-nov-2019:
@frozen
,@defineElement
,@metaData
The text was updated successfully, but these errors were encountered: