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
[Bug]: Stage 3 decorators behavior differences from TypeScript #16117
Comments
Hey @evanw! We really appreciate you taking the time to report an issue. The collaborators on this project attempt to help as many people as possible, but we're a limited number of volunteers, so it's possible this won't be addressed swiftly. If you need any help, or just have general Babel or JavaScript questions, we have a vibrant Slack community that typically always has someone willing to help. You can sign-up here for an invite. |
Thank you very much! If you have time, I would appreciate one examples for the following as I cannot reproduce it: EDIT: I just noticed the linked repo :)
Input code: class A {
@console.log #m() {}
} logs
|
Also this:
Input code: class A {
@console.log accessor x = 2
} logs:
and by looking at the number of params of the I see the test failing in your repo, I'll investigate why. |
From the repo readme (https://github.com/tc39/proposal-decorators/tree/master#adding-initialization-logic-with-addinitializer):
(@pzuraq is this correct?) |
Comment 1
Ah it looks like this was the name on the private method itself, not on the context. That was unclear. Sorry about that. Here's a test case: const dec = (fn, ctx) => {
console.log(fn.name, ctx.name)
}
class Foo {
@dec #foo() { }
} Babel prints Comment 2
Here's a reduced test case: function check(a, b) {
try {
const x = a()
if (x === b) return
throw new Error(`${x} !== ${b}`)
} catch (e) {
console.log(`${a}: ${e}`)
}
}
const getter = (_, ctx) => {
check(() => ctx.access.has({ foo: false }), true)
check(() => ctx.access.has({ bar: true }), false)
check(() => ctx.access.get({ foo: 123 }), 123)
check(() => 'set' in ctx.access, false)
}
const setter = (_, ctx) => {
check(() => ctx.access.has({ foo: false }), true)
check(() => ctx.access.has({ bar: true }), false)
check(() => 'get' in ctx.access, false)
const obj = {}
ctx.access.set(obj, 123)
check(() => obj.foo, 123)
check(() => 'bar' in obj, false)
}
const accessor = (_, ctx) => {
check(() => ctx.access.has({ foo: false }), true)
check(() => ctx.access.has({ bar: true }), false)
check(() => ctx.access.get({ foo: 123 }), 123)
check(() => {
const obj = {}
ctx.access.set(obj, 123)
return obj.foo
}, 123)
}
console.log('getter')
class Getter {
bar = 0
@getter get foo() { return this.bar }
}
console.log('setter')
class Setter {
bar = 0
@setter set foo(x: number) { this.bar = x }
}
console.log('accessor')
class Accessor {
@accessor accessor foo = 0
} TypeScript just prints
Comment 3
Thanks for the link. Sorry, I had missed that. Here's a small test case: const field = (_, ctx) => {
console.log(typeof ctx.addInitializer)
}
class Foo {
@field foo
@field static bar
} Babel prints
The bug about the mismatch is here: tc39/proposal-decorators#501. It sounds like the specification is correct but in either case, one should be updated to match the other. |
@nicolo-ribaudo the Readme is wrong, the spec is more up to date and has addInitializer on all types. I’ll update the README. |
Regarding this, FYI, babel uses a closure to always explicitly call the const Getter = {
bar: 0,
get foo() {
return this.bar;
},
};
const key = "foo";
const desc = Object.getOwnPropertyDescriptor(Getter, key);
babelAccess = {
get: function (_this) {
return desc.get.call(_this);
},
};
tsAccess = {
get: function (_this) {
return _this[key];
},
};
console.log(babelAccess.get({ foo: 123 }));
console.log(tsAccess.get({ foo: 123 })); |
I also noticed a difference when improving the output code size. let addInitializer;
function callCapturedFunc() {
addInitializer(() => null);
}
function decMethod(_, context) {
({ addInitializer } = context);
addInitializer(() => null);
}
expect(() => {
class C {
@callCapturedFunc
@decMethod
m() {}
}
}).toThrow('attempted to call addInitializer after decoration was finished') let addInitializer;
function decMethod(_, context) {
({ addInitializer } = context);
addInitializer(() => null);
}
try {
class C {
@decMethod
m() {}
}
} finally {}
expect(() => {
addInitializer(() => null);
}).toThrow('attempted to call addInitializer after decoration was finished') |
Closing this issue as the reported behaviour differences have been fixed by the aforementioned PRs. Currently Babel is passing most test in https://github.com/evanw/decorator-tests, except tdz-related class binding tests. |
💻
How are you using Babel?
Programmatic API (
babel.transform
,babel.parse
)Input code
See this repo: https://github.com/evanw/decorator-tests
Configuration file name
No response
Configuration
No response
Current and expected behavior
I'm working on adding stage 3 decorators to esbuild. My first step was to write a lot of tests and run them through the Babel and TypeScript implementations. I'm sending you this issue because I figured you might be interested to learn about the behavior differences that I found. This is not a problem for me as I don't use Babel myself, so feel free to close this issue after reading it if you'd like. Here are the unexpected behaviors I found with Babel's implementation (see the repo for details):
access
exposes the underlying getter and setter instead of theGet
andSet
abstract operations. (fix: decorator bindsgetter/setter
toctx.access
for public fields #16201)addInitializer
method. (Allow addInitializer in field decorator context #16132)undefined
instead of throwing aReferenceError
if a class decorator is present. (Class binding is in TDZ during decorators initialization #16138)Caveat: I'm just learning about this feature now, and I'm also not sure how up-to-date the specification is, so it could be that Babel's behavior for some of these is intentional. But if so, that behavior at least differs from TypeScript's implementation, which seems undesirable. FWIW I'm currently mostly planning to align esbuild's behavior to TypeScript since it seems like it matches the specification more closely, so it'd be good to know if Babel's behavior here is actually more correct (e.g. regarding
access
).Environment
I can reproduce this by running Babel using https://babel.dev/repl.
Possible solution
No response
Additional context
No response
The text was updated successfully, but these errors were encountered: