rfcs icon indicating copy to clipboard operation
rfcs copied to clipboard

Deprecate using tracked in classic classes

Open pzuraq opened this issue 4 years ago • 14 comments

Rendered

pzuraq avatar Mar 18 '21 20:03 pzuraq

@rwjblue waiting until 5.0 would lock us into having to provide a shim around @glimmer/tracking until then, which I would prefer not to. Ideally, @glimmer/tracking could be completely independent of Ember sooner rather than later.

Does the alternative of exposing trackedClassic() from @ember/object/compat work as a short term solution? Effectively provide an easy-to-codemod alias for the existing functionality?

pzuraq avatar Mar 19 '21 16:03 pzuraq

Is this really unused? I seem to remember us using it, though I would have to check to be really sure.

It also seems somewhat arbitrary to fall off a cliff on this particular feature and this feature only, I think we got a lot of millage out of the flexibility and simplicity of the decoupled migration model. It also breaks the general expectations about decorators in the framework (on this particular decorator only).

If it’s just about not loosing out on a good naming, this seems like too heavy-handed to me. Certainly there would be other just as good options like new tracked(...) (detect new.target) or just make it a different spelling (like Tracked to signify that it is a constructor).

I am also not sure if it’s actually a fundamentally good idea to overload this to mean two pretty different things. We did it with {{action}} and that’s kind of a mixed bag outcome (probably mostly bad). It would be somewhat harder to ask questions/google/lookup documentations, and things like typescript/vscode would have a somewhat less ideal UX.

chancancode avatar Mar 19 '21 16:03 chancancode

I am also not sure if it’s actually a fundamentally good idea to overload this to mean two pretty different things. We did it with {{action}} and that’s kind of a mixed bag outcome (probably mostly bad). It would be somewhat harder to ask questions/google/lookup documentations, and things like typescript/vscode would have a somewhat less ideal UX.

@chancancode I believe this is an orthogonal point of discussion. Whether or not we decide to use tracked({}) or tracked([]), it would be useful to have the option to, which is partially what this RFC is about enabling.

pzuraq avatar Mar 19 '21 17:03 pzuraq

@rwjblue waiting until 5.0 would lock us into having to provide a shim around @glimmer/tracking until then, which I would prefer not to. Ideally, @glimmer/tracking could be completely independent of Ember sooner rather than later.

I don't think this is correct? The definition of what the "invoke" does here (e.g. non-decorator use case) could absolutely be provided by @glimmer/global-context or any number of other things to avoid the coupling.

rwjblue avatar Mar 19 '21 18:03 rwjblue

That's a good point. It would be unfortunate to have to tie Ember in that way I think, specifically for a to-be-deprecated feature (classic classes), but it would work to solve that issue.

pzuraq avatar Mar 19 '21 19:03 pzuraq

In order to use tracked properties, you must convert to using a native class.

Does that mean that this will work:

import Component from "@ember/component";

class ClassicComponent extends Component {
  @tracked foo;
}

Or by native class you mean non-classic Ember stuff? If so, that should be re-worded.

boris-petrov avatar Mar 22 '21 08:03 boris-petrov

@boris-petrov that would work! This would only deprecating using the classic syntax, any classic classes that work as native classes would continue to work with @tracked

pzuraq avatar Mar 22 '21 16:03 pzuraq

Export a new value, trackedClassic, from @ember/object/compat. This would only be usable as a classic decorator in classic classes, and the current form of tracked() would still be deprecated.

Sounds like a good idea!

My gut feeling is that this is indeed rarely used, so the churn cost should be pretty low. Especially if trackedClassic would be around.

Speaking for our app, we've still got a good deal of classic components. We're gradually migrating, when we have other reasons to refactor code we'll rewrite with glimmer plus angle brackets and tracked.

Just my view though, others may have another perspective.

sandstrom avatar Mar 23 '21 21:03 sandstrom

The classic syntax is indeed heavily used by many apps I've encountered, most commonly when migrating a very large app when the native class codemod cannot yet be run. It seems to be a transient middle state although a very useful one: the native class codemod doesn't take a lot of apps very far until they've cleaned up certain other patterns that can take some time, and many codemods were written to only mod classic syntax to better classic syntax patterns prior to moving to native classes so these teams wait to run the native class codemod until they've completed this other work.

I don't know that i agree with overloading the signature but from the perspective of whether the classic syntax is used think the deprecation until should be the same as for classic syntax as many of these mid-migration apps are on latest Ember versions. Folks don't always upgrade right away and it would suck for there to be a mismatch here where they needed to downgrade or pin at an ember version because classic still worked but tracked on classic didn't.

runspired avatar Apr 10 '21 18:04 runspired

Well this was not part of 4.0. Is the only thing blocking the version we do it in?

wagenet avatar Jul 23 '22 23:07 wagenet

@wagenet I think so

runspired avatar Jul 24 '22 00:07 runspired

@chriskrycho would you think this falls under #832 or is it separate?

wagenet avatar Jul 29 '22 23:07 wagenet

Roughly: classic classes themselves will go away. We could remove this earlier than those, though, so please do include it in that list!

chriskrycho avatar Jul 30 '22 02:07 chriskrycho

I’ll add this to the #832 list but also leave it open for now.

wagenet avatar Jul 30 '22 02:07 wagenet

This fell off my radar until I was doing a triage pass, but it pairs nicely with #876; I'm going to bring it to Framework this week. For good or for ill, it looks like it's going to end up targeting 6.0, since we are now in the 4.10 beta cycle and we freeze deprecations at the .10 minor release.

chriskrycho avatar Dec 01 '22 15:12 chriskrycho

We talked about this today at Framework, and rather than pushing this direction (and deprecating things piecemeal) we are going to FCP to close—by getting rid of it holistically as part of what we're discussing around Classic on #832.

chriskrycho avatar Dec 02 '22 19:12 chriskrycho