-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Regression in 0.18.4 when using static class properties inside of decorators #3230
Comments
Hmm. It looks like this is a difference between legacy TypeScript decorators and standard JavaScript decorators. Accessing the class name before its declaration is finished works with legacy TypeScript decorators but will no longer work with standard JavaScript decorators (compare this to this). Running this code using standard JavaScript decorators will throw this error, which is effectively what esbuild is doing here:
So it looks like what esbuild is doing here is correct for standard JavaScript decorators. Perhaps esbuild's parser needs to mess with scope only when parsing legacy TypeScript decorators to allow them to access the class object before it has been declared. |
This is confusing. I'm trying to figure out whether the evaluation of decorators is supposed to happen inside or outside the class body. It seems like the answer is "it depends on the code being compiled" and I'm not sure what is a bug and what is intended behavior. You are allowed to call async function foo() {
console.log('before')
class Foo {
@(await new Promise<PropertyDecorator>(r => setTimeout(() => r(() => {}), 1000)))
static foo = 'foo'
}
console.log('after')
}
foo() You are also allowed to use a private name inside a decorator, which implies that decorators are evaluated inside the class body: class Foo {
static #bar = '#bar'
@((() => { console.log(Foo.#bar) }) as PropertyDecorator)
static foo = 'foo'
} The use of private names appears to have been broken until TypeScript 4.8+ since TypeScript 4.7 and earlier generate nonsense code that references private names outside of the class body. Then TypeScript 4.8+ moved decorator evaluation into the class body, which makes this work. But attempting to do both of these things causes TypeScript to generate nonsense code even with the latest release, because of course async function foo() {
console.log('before')
class Foo {
static #bar = '#bar'
@((() => { console.log(Foo.#bar) }) as PropertyDecorator)
@(await new Promise<PropertyDecorator>(r => setTimeout(() => r(() => {}), 1000)))
static foo = 'foo'
}
console.log('after')
}
foo() It seems like TypeScript wants decorators to be evaluated both inside and outside the class body. So I guess TypeScript decorators don't really have well-defined semantics? Or at least I can't figure them out. |
This PR is relevant: microsoft/TypeScript#50074. It introduced the behavior of sometimes putting decorator evaluation inside a static block in the class body. TypeScript's condition for doing this is currently whenever a class member has either a decorator or a parameter decorator that contains an private name (even if the private name is unrelated to the class with the decorator). |
First off I just wanted to say thank you for esbuild and all the work you do maintaining it, it's made such an enormous improvement to our developer productivity ❤️
When upgrading to 0.18.x we found a bug introduced by #3167 where using static class properties inside of decorators causes errors to be thrown at runtime. Playground link here.
Due to
export let DocumentSearchOptions = _DocumentSearchOptions;
being set after the call to__decorateClass
, it means thatDocumentSearchOptions.MaxFiltersSize
will throw an error at runtime.Please let me know if you need any more info!
The text was updated successfully, but these errors were encountered: