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

Static/member methods cannot access the replaced decorated class #477

Closed
xaviergonz opened this issue Aug 4, 2022 · 5 comments
Closed

Comments

@xaviergonz
Copy link

xaviergonz commented Aug 4, 2022

Given the following

function decorator(value, context) {
  return class X extends value {
    a() {}
  }
}

@decorator
class C {
  static staticCreate() {
    return new C()
  }
  
  methodCreate() {
    return new C()
  }
}

const c = new C() // this one creates an instance of C "decorated"
console.log(c.a)
const c2 = C.staticCreate() // this one creates an instance of C "undecorated"
console.log(c2.a)
const c3 = c.methodCreate() // this one creates an instance of C "undecorated"
console.log(c2.a)

I'd expect all of them to return an instance of the decorated class, but new returns the decorated one and the others the undecorated one (at least in the independent playground).

Typescript decorators do return the decorated class in every case. I'm not sure what's the intended behaviour, but whichever it is, maybe it should be documented in the spec?

@xaviergonz xaviergonz changed the title Static methods cannot access the decorated class Static/member methods cannot access the decorated class Aug 4, 2022
@xaviergonz xaviergonz changed the title Static/member methods cannot access the decorated class Static/member methods cannot access the replaced decorated class Aug 4, 2022
@senocular
Copy link
Contributor

This should work. In the spec text the binding of the class name comes after running class decorators

35. Let newF be Completion(ApplyDecoratorsToClassDefinition(F, decorators, classExtraInitializers)).
...
37. Set F to newF.[[Value]].
38. If classBinding is not undefined, then
a. Perform classScope.InitializeBinding(classBinding, F).

@pzuraq
Copy link
Collaborator

pzuraq commented Aug 5, 2022

@senocular is correct here, this is the behavior of the spec, the bound name of the class is the same, decorated version of the class everywhere. @xaviergonz which transform are you using? I wasn't aware that TS had shipped the most recent version of the spec yet, are you using the legacy/experimental version from before?

@xaviergonz
Copy link
Author

Good to know! The transform I was using to test it out (and therefore the one that apparently is wrong) is the one at https://javascriptdecorators.org/
When I mentioned TS I was referring to the old experimental decorators implementation.

@pzuraq
Copy link
Collaborator

pzuraq commented Aug 8, 2022

For sure, so that transform is likely out of date at this point, @pabloalmunia it may be a good idea to put a notice on the site that users should update to using the latest Babel or Typescript transforms once they're available.

@xaviergonz that transform is not going to be supported going forward. You should use the 03/22 transform from Babel once it ships: babel/babel#14836, or use the Typescript transform once it ships, as those will be the most accurate relative to the actual spec.

Closing this for now since it appears to be addressed.

@pzuraq pzuraq closed this as completed Aug 8, 2022
@pabloalmunia
Copy link
Contributor

Of course. The experimental transpiler is outdated. It had its usefulness, but it will soon become meaningless.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants