@classic Decorator
#468
@classic Decorator
#468
Conversation
| own code, or are not passed any arguments other than injections, so for those | ||
| classes the usage of `create` is an implementation detail. | ||
|
|
||
| These classes will _always_ warn users if they are not decorated with |
There was a problem hiding this comment.
is this a linter warning or a deprecation style warning?
There was a problem hiding this comment.
This would be a deprecation style warning, since it’s not information we can actually gather through static analysis. We need to actually run/construct the class to get this information
There was a problem hiding this comment.
Will it actually be a deprecation, with an identifier and an until field? Or just use warn from 'ember-debug'?
| an edge case, such as using `init` in a parent class and `constructor` in a | ||
| child class. | ||
|
|
||
| This should allow users who wish to adopt native class syntax to do so |
There was a problem hiding this comment.
should this also be a code mod? to add the @classic decorator to each EmberObject class?
There was a problem hiding this comment.
Absolutely! I think a codemod for this would be very helpful
There was a problem hiding this comment.
I'm not so sure. Adding @classic is fairly quick and it might behoove developers to evaluate whether each class needs it manually.
|
I think it would be wise to separate |
|
@piotrpalek that's fair, in fact our recommended upgrade path will be:
However for Routes/Controllers/Services, there are already far fewer differences between native class and classic class syntax. Really, the only major ones are using mixins, and using There are also some users who must use native class syntax everywhere, such as users who wish to adopt TypeScript. This is different from the React conversion, because in that case, React was converting from one TS compatible form (native classes) to another (functional components). Since those users already exist, and have to deal with these problems anyways, this would provide better tooling for them to detect possible bugs. |
|
Right, I didn't consider TS users. I can understand the reasons, I've also re-read the RFC and understand it a little better now. My comment was probably less enthusiastic about this because I both didn't understand it enough, as well as a drive-by comment after #467 :) Tl;dr I now consider it a good solution to a tough problem. Sorry if it came out overly aggressive/negative! 🙏 |
|
I believe this RFC needs to identify a package that |
Ya, totally agree. The current implementation is done in an addon, and I don't see any reason not to keep it that way... |
|
Added the import path and addon name to the RFC, and updated some of the language. We found during implementation that it wasn't possible to restrict all usage of classic APIs in non-classic classes, so we used lint rules for these instead. The general restrictions are the same as before though. |
…to classic-decorator
|
👍 from the core team at today's meeting, moving this into final comment period... |
|
At today's core team meeting, we have decided to move forward with this RFC. 👍 |
Rendered