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

Sharp notation do not allow decorator .. #50602

Closed
anymos opened this issue Sep 2, 2022 · 12 comments
Closed

Sharp notation do not allow decorator .. #50602

anymos opened this issue Sep 2, 2022 · 12 comments
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed

Comments

@anymos
Copy link

anymos commented Sep 2, 2022

Bug Report

πŸ”Ž Search Terms

Sharp notation method decorator

πŸ•— Version & Regression Information

Trying to use decorators

⏯ Playground Link

Playground link with relevant code

πŸ’» Code

@decorator()
#method

πŸ™ Actual behavior

TS1206: Decorators are not valid here

πŸ™‚ Expected behavior

Decorators shall works

@MartinJohns
Copy link
Contributor

You forgot to provide a valid playground link and a self-sufficient code example.

@Jamesernator
Copy link

Sharp notation method decorator

Private fields/methods/accessors don't support decorators, because the current implementation of decorators in TypeScript is based on a very old version of the decorators proposal, that old proposal doesn't really integrate properly with private fields.

The current decorators proposal actually has implementers support, and does support private fields/methods/accessors. There's currently an effort to support the current proposal in typescript. However do note that old decorators can't be used in the new decorators proposal, so any such decorators would need be rewritten for the new proposal.

@jakebailey
Copy link
Member

jakebailey commented Sep 2, 2022

Here's an example for posterity: Playground Link

Decorators as they exist now (we implemented an old spec and are stuck with it) don't work on private fields. I don't think this is something we're going to support. If we don't downlevel the private field to the WeakMap implementation, there's no way that we could put our experimental decorators on it. I don't think we'd go and allow our old-style decorators if-and-only-if your target is old enough to not emit private fields directly.

There is #49705, which does support --experimentalDecorators on accessors, which is different and possible, as the PR shows. Once the new, revised decorators spec is ratified, then we'll have a clear path.

@jakebailey jakebailey added the Design Limitation Constraints of the existing architecture prevent this from being fixed label Sep 2, 2022
@anymos
Copy link
Author

anymos commented Sep 4, 2022

understood, thank you very much for the detailed answer, I was naively thinking that decorator would works ootb on private method.

A pity really as it limits the decorator usefulness a lot from my perspective.

@anymos
Copy link
Author

anymos commented Sep 24, 2022

By the way, it sounds lie it is working when using a // @ts-ignore, ofc it s probably kind of a luck and it works too on private method.

So now I am looking for a way to ignore the TS1206 - decorator not allowed here in tsconfig.json, is there a way to obtain this ? so I can use the feature that is not supposed to work ( lucky me) without having to add so many ts-ignore ? - finger crossed

@jakebailey
Copy link
Member

If it's working, that's a fluke; my playground link above shows that the decorators are not applied to the method named with a private identifier, even when we downlevel it into a WeakMap. If you have it working, I'm curious, but // @ts-ignore is definitely a bad idea.

@anymos
Copy link
Author

anymos commented Sep 25, 2022

If it's working, that's a fluke; my playground link above shows that the decorators are not applied to the method named with a private identifier, even when we downlevel it into a WeakMap. If you have it working, I'm curious, but // @ts-ignore is definitely a bad idea.

I agree it is a bad idea and probably not going to work for ever, actually it is not even working on stackblitz, which make me considering to not use the sharp notation at all anymore, a pity I liked it, I will be back to the old school _ prefix I guess.

https://stackblitz.com/edit/angular-u9s5bb?file=src%2Fapp%2Fbutton-toggle-overview-example.html

In the man time is there a better way to replace the decorator on a private sharp notation use method in 2022 ? maybe by coding a particular proxy on the class itself. The pb is that I use those decorator to provide extremely useful logging capability so I need it for each of the method in the code, it is not just for one or a few of the private methods.

Making those method public would solve the issue, but I believe it is even worse than using the 'fluke' of it working on the private methods.

All ears to find a proper/better solution :)

At least my choie is easy now I will remove the sharp notation and then no need for the ts-ignore anymore, it will be cleaner, pity for the sharp notationthat I liked, maybe I could try to find a ts-lint the enforce the '_' private method pre fixing to keep it in some way.

@jakebailey
Copy link
Member

jakebailey commented Sep 26, 2022

The better solution is to just wait for ECMAScript decorators, i.e. #48885/#50820.

@anymos
Copy link
Author

anymos commented Sep 27, 2022

Agreed, do you have any idea of the timeline ? even years will not make me scared or cry do not worry ;-) just to have an idea, I was not following much the typescript evolutions until now as it was always fitting my needs until that edge case

@MartinJohns
Copy link
Contributor

MartinJohns commented Sep 27, 2022

@anymos Decorator support is part of the 4.9 iteration plan, so likely in November. #50457

According to Jake it's likely part of 5.0, so probably in February.

@jakebailey
Copy link
Member

Not in November, no: #50457 (comment)

Probably 5.0.

@anymos
Copy link
Author

anymos commented Oct 17, 2022

Cool thank you for the feedback, with some buffer, I believe in May it shall be possible, I will review around that time, thank you guys !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Design Limitation Constraints of the existing architecture prevent this from being fixed
Projects
None yet
Development

No branches or pull requests

5 participants