-
Notifications
You must be signed in to change notification settings - Fork 106
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
Normative updates to the spec, March 2023 #499
Comments
I'm not sure I agree with this one, since you can return a shared function: function noop() {}
function conditional(cond) {
return function (target, context) {
return cond ? target : noop;
};
}
class C {
@conditional(true) static a() {}
@conditional(false) static b() {}
@conditional(false) static c() {}
}
console.log(C.a.name, C.b.name, C.c.name); Without this change, this would log There's no way to tell whether the returned function is safe to be renamed in this fashion. I'd much rather someone explicitly do this instead if changing the name is truly important: function dec(m) {
function x() {
return m.call(this, ...arguments);
}
Object.defineProperty(x, "name", {
...Object.getOwnPropertyDescriptor(x, "name"),
value: typeof m.name === "symbol" ? `[${m.name.toString()}]` : m.name
});
return x;
} Yes, it's a bit more cumbersome, but it's also more predictable. |
I think I'm coming around to that with more discussion that's been going on, I was worried this would result in a lot of problematic debugging behavior but it does seem like changing the name would actually mess with stack traces even more (see discussion here: #502) |
Probably should include static accessors on this list as well. |
All of these changes gained consensus in the plenary today, I will be merging them into the updated spec as soon as I have time |
Hello, @pzuraq, can you at least describe final consensus? Seems like this issues blocks at least esbuild implementation, which is blocks all users of vite from using new decorators... |
As noted above, all of the proposed changes were accepted. I’ve been very busy lately so haven’t had time to update the spec, but I’ll try to get to it today. |
All updates have been merged. For update 6, see the linked PR for more details(comment) |
This issue is a meta issue for tracking a number of normative updates that are being proposed to the spec in the upcoming March plenary.
Remove the dynamic assignment of
[[HomeObject]]
from decorator method application/evaluation (PR, Issue)This dynamic assignment/update of a function's
[[HomeObject]]
is a new behavior that has not occurred before in the spec, and is even asserted against. It results in unexpected behavior and does not have an actual use-case. This decision was likely due to a misunderstanding of what theMakeMethod
s purpose was. This change removes the reassignment entirely.Call decorators with their natural
this
value instead ofundefined
(PR, Issue)Currently decorators are always called with an undefined
this
value, even when it might appear that they should have one (e.g.@foo.bar
should be called withfoo
as the receiver). This change threads the receiver through so the decorator can be called with it.Throw an error if the value passed to
addInitializer
is not callable (PR)addInitializer
expects to receive a function, and all of the spec text downstream from it assumes this as well, so this assertion is necessary to ensure that the value is actually callable.Set the name of the
addInitializer
function (PR)Sets the name of
addInitializer
, which is currently an empty string.Remove
SetFunctionName
from decoration (PR)Currently,
SetFunctionName
is called on the returned decorated methods. This two main problems:SetFunctionName
asserts against ever being called twice for the same method."Bind" static accessors directly to the class itself. (PR pending, issue)
Static accessors are implemented conceptually as a sugar on top of private fields. A getter and setter are generated which access private storage on the same class. For static accessors, this results in them not working when inherited by a child class, since static fields and accessors are not redefined on child classes.
The proposed update is to "bind" static accessors to their original class, e.g. instead of being sugar for:
They would desugar to:
The text was updated successfully, but these errors were encountered: