Skip to content
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

Class binding references in static field initializers should be resolved to the decorated class #3787

Open
JLHwung opened this issue May 27, 2024 · 3 comments
Labels

Comments

@JLHwung
Copy link
Contributor

JLHwung commented May 27, 2024

Input (playground):

class D {}
@(() => D)
class C {
  static p = C
}
console.log(C.p === D)

Option:

{ target: "es2022" }

Expected: It should print true, as per spec, static field initializers (step 40) should run after the class binding was initialized (step 38).

Actual: It prints false.

@evanw evanw added the classes label May 27, 2024
@evanw
Copy link
Owner

evanw commented Jun 7, 2024

I think it's unclear what should happen here until tc39/proposal-decorators#529 is resolved, as the specification may be changed. Supposedly that may be resolved after the upcoming TC39 meeting (from June 11th to June 13th?).

@JLHwung
Copy link
Contributor Author

JLHwung commented Jun 8, 2024

If I read correctly, the discussion result in tc39/proposal-decorators#529 is that the homeObject of the static accessor might be reverted back to this. But the decision that class decorators should run before static fields was made long ago (c.f. tc39/proposal-decorators#329 (comment)). So the outcome of tc39/proposal-decorators#529 should not affect the behaviour mentioned in this issue.

@evanw
Copy link
Owner

evanw commented Jun 9, 2024

In that case, consider the following test case:

let old
let block
class Bar {}

@(cls => (old = cls, Bar)) class Foo {
  static { block = Foo }

  method() { return Foo }
  static method() { return Foo }

  field = Foo
  static field = Foo

  get getter() { return Foo }
  static get getter() { return Foo }

  set setter(x) { x.foo = Foo }
  static set setter(x) { x.foo = Foo }

  accessor accessor = Foo
  static accessor accessor = Foo
}

let foo = new old
let obj

console.log(
  Foo !== old,
  Foo === Bar,
  block === Bar,

  Foo.field === Bar,
  old.getter === Bar,
  (obj = { foo: null }, old.setter = obj, obj.foo) === Bar,

  foo.field === Bar,
  foo.getter === Bar,
  (obj = { foo: null }, foo.setter = obj, obj.foo) === Bar,

  // These are determined by the outcome of https://github.com/tc39/proposal-decorators/issues/529
  // Foo.accessor === Bar,
  // old.accessor === Bar,
)

I think you're saying they should all return true here. That's currently only the case in Babel, not in esbuild or in TypeScript. I need to fix esbuild's handling of this.

I believe how accessors behave in this scenario is still determined by the outcome of tc39/proposal-decorators#529. Currently old.accessor is specified to throw and Foo.accessor is specified to be undefined, although that may change so it's not really something we can write a test for at the moment. But I think what you're saying is whatever the behavior of accessors ends up being, there will be some way to get the value (whether it's old.accessor or Object.getOwnPropertyDescriptor(old, 'accessor').get.call(Foo)) and that value should be the right one.

evanw added a commit to evanw/decorator-tests that referenced this issue Jun 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants