@classic Decorator

[pzuraq]

Rendered

[NullVoxPopuli]

is this a linter warning or a deprecation style warning?

[pzuraq]

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

[Gaurav0]

Will it actually be a deprecation, with an identifier and an until field? Or just use warn from 'ember-debug'?

[NullVoxPopuli]

should this also be a code mod? to add the @classic decorator to each EmberObject class?

[pzuraq]

Absolutely! I think a codemod for this would be very helpful

[Gaurav0]

I'm not so sure. Adding @classic is fairly quick and it might behoove developers to evaluate whether each class needs it manually.

[piotrpalek]

I think it would be wise to separate EmberObject and native class syntax. There are too many edge cases and weird behaviors. From what I've seen trying to support EmberObject usage with native classes also makes Ember's API unnecessarily complex and worse that it would be otherwise. It would also lead to an easier way of teaching people: "look, this is how Ember is build, but we have this new world where we use native classes and which has different behavior, but it doesn't work for all things that classic Ember supports yet". Worked great for functional components in the React world.

[pzuraq]

@piotrpalek that's fair, in fact our recommended upgrade path will be:

• Keep classic components as classic classes until they can be converted to Glimmer components
• Keep utility classes that extend EmberObject as they are, using classic syntax, until you can convert them directly to native classes.

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 init vs constructor. Given we don't have any native class alternatives for these yet, and that the difference is much much smaller between the two, it makes sense to recommend users to convert over to the newer syntax and incrementally remove these differences. In most cases, I think it will be a pretty straightforward conversion.

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.

[piotrpalek]

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! 🙏

[pzuraq]

It's really no problem! This is going to be a tricky transition, and we're trying to strike the right balance - feedback that this could be too complicated is exactly what led to this RFC, and the simplifications in #467 / #451. It's always good to get feedback, thanks for giving us your thoughts 😄

[mixonic]

I believe this RFC needs to identify a package that @classic is imported from

[rwjblue]

I believe this RFC needs to identify a package that @classic is imported from

Ya, totally agree. The current implementation is done in an addon, and I don't see any reason not to keep it that way...

[pzuraq]

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.

[rwjblue]

👍 from the core team at today's meeting, moving this into final comment period...

[MelSumner]

At today's core team meeting, we have decided to move forward with this RFC. 👍