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

Intent to RFC: Deprecate Mixins #534

Closed
pzuraq opened this issue Aug 30, 2019 · 4 comments
Closed

Intent to RFC: Deprecate Mixins #534

pzuraq opened this issue Aug 30, 2019 · 4 comments
Labels
Seeking Co-author T-deprecation T-framework RFCs that impact the ember.js library

Comments

@pzuraq
Copy link
Contributor

pzuraq commented Aug 30, 2019

  • Proposed Target: Ember v4.0.0
  • Alternative: Native Classes, Services, helper functions, class decorators

Mixins have been considered an antipattern in Ember for quite some time, for a variety of reasons. To sum them up:

  1. They were used for sharing state between instances of classes very often, early on in Ember before Services existed. This was a dubious use case before, and with Services now a major part of the programming model, it doesn't make sense at all.
  2. They were used to share functions frequently before JavaScript modules existed or were in common use. Now that helper functions can be defined independent of classes or global scope, this use case no longer makes sense.
  3. They were used for multiple inheritance. In some cases, this made sense, but in many cases it would lead to developers prematurely attempting to DRY things up by extracting a few shared or similar functions/methods, and ultimately splitting up class definitions and making them harder to follow and comprehend overall.

Ultimately, the mixin pattern was overused in Ember applications, and Ember's particular implementation of that pattern had quirks that were used (such as state sharing) when the probably should not have been.

Migration Path

Generally speaking, we should encourage users to migrate away from the mixin pattern. Multiple inheritance is tricky in every language, and especially so in JavaScript. If a problem can be solved by sharing helper functions or services, this should be the recommended and idiomatic approach.

However, there are still (rare) cases where the mixin pattern would make sense. For these cases, we should recommend that users write class decorators. Class decorators, in the current stage 1 transforms, are functions that receive the class definition, and return a new class definition. Users can use dynamic class definitions inside of these to extend the class they are passed:

function withFeature(c) {
  return class WithFeature extends c {
    someFeature() {}
  }
}

These can be applied today using class decorator syntax:

@withFeature
export default class SomeClass {}

In the future, when decorator syntax moves to stage 3 and changes, these class decorators will likely need to be updated. However, since these definitions are functions, they can also be called directly:

class SomeClass {}
    
export default withFeature(SomeClass);

Making them pretty future proof.

Deprecation Timeline

This pre-RFC suggests we remove mixins in Ember v4.0.0, but that is a bit of an aggressive timeline. While mixin usage is not as common as it once was, it still exists throughout the ecosystem and may take some time to migrate away from, especially if we want users to migrate away from the mixin pattern altogether. If we push for a quick deprecation, we may see a lot of class decorators being written.

The alternative would be to deprecate mixins for removal at the same time as EmberObject, which will likely be in v5.0.0 at the earliest.

@webark
Copy link

webark commented Sep 1, 2019

This pre-RFC suggests we remove decorators in Ember v4.0.0.

Was this supposed to be decorators?

@pzuraq
Copy link
Contributor Author

pzuraq commented Sep 2, 2019

Definitely not, thanks for catching that, updated!

@wagenet
Copy link
Member

wagenet commented Jul 23, 2022

Leaving this open since it's high priority. We should fast track an RFC for this.

@wagenet
Copy link
Member

wagenet commented Jul 29, 2022

I believe this falls under the umbrella of #832. I'm going to close this issue to push any discussion there. Despite being closed, we will certainly use this issue for reference and to inform the future RFC. Thanks @pzuraq!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Seeking Co-author T-deprecation T-framework RFCs that impact the ember.js library
Projects
None yet
Development

No branches or pull requests

4 participants