Best way to replace Mixins

We have a lot of existing code that makes heavy use of mixins - these provide 100s of actions and lifecycle hook functions for controllers, components and routes. Some of these mixins are many 100s of lines long, and some mixins include mixins.

I’d love to replace them as we move to a more Ember Octane view of the world, but I’m not sure what the best / canonical / recommended approach is?

In the past I’ve recommended moving everything into a service, importing into the the original route/controller/component and have the stub action/lifecycle hook immediately call the function on the service. It works because the explicitness of where the function is declared is obvious, but given the amount of places the mixins are used, and the size of them, this is still quite a lot of work and duplication.

We could inherit instead, but I’m not sure this makes for a much better world.

Or we could kick the can down the road given that classic Ember syntax is still supported for the foreseeable future. I actually expect that this will be part of any solution we adopt.

Suggestions welcome - thank you.

1 Like

The short answer is that there isn’t one best way to replace mixins. In fact, so far as I’ve worked out in prepping the app I work on to migrate, there are five basic patterns for successfully migrating away from mixins: services, standalone classes, standalone functions, class decorators, and inheritance. Which is the best to use depends on what the existing mixin does. The last two, class decorators and inheritance, should be your options of last resort in my experience.

When you should choose each:

  • If the mixin is a way of sharing long-lived state, extract to a service and inject it wherever it was being used before. This is, as you note, sometimes painful—but in my experience that pain tends to highlight bad design decisions and over-coupling you want to undo anyway; this is a good opportunity for refactoring.

    In my experience, this is also pretty rare, though: if you actually needed long-lived state, you very probably already had a service. This is the thing people (myself in the past included!) tend to reach for first, but it’s rarely the right choice.

  • If the mixin is a way of supplying non-shared state or behavior, only meant to live for as long as whatever it’s mixed into, you can replace it with another class which is simply instantiated at the time the class is constructed. This is a much more common pattern, and this solution is a fairly standard one in traditional object-oriented solutions: it’s delegation.

    The class you use for this does not need to (and should not) be an EmberObject subclass; it should nearly always just be a normal ES6 base class itself.

    When doing this, you’ll just import the class from the module it lives in and set it up on your class:

    import Component from '@glimmer/component';
    import UtilityClass from 'my-app/lib/utility-class';
    
    export default class MyComponent extends Component {
      constructor() {
        super(...arguments);
        this.utility = new UtilityClass(this.args.useful);
      }
    }
    
    {{this.utility.someThing}}
    

    As with the service-oriented refactor, this does entail a lot of change through your app. However, it’s also straightforward, largely mechanical. With both the service approach and this one, you can likely write a codemod for it.

    (You can also make this configurable if you need to, by registering the class with the DI system. In practice, it’s rare that you need to do that, and I would recommend against starting with that. Do it only if you find you actually need it!)

  • When the mixin is only supplying shared behavior—no state—you can extract it to utility functions. Those functions can just be imports from a module; people with long Ember experience or coming from Java or C# backgrounds tend to reach for classes as namespaces, and you can do that but you really don’t need to. import { someHelper } from 'my-app/lib/fun-helpers' works just fine, and is more tree-shaking-friendly (in a Coming Soon™ tree-shaking world) to boot!

  • Very rarely, you’ll find that what you really want is to add functionality or data to an existing class without using one of the above methods—presumably because you take it to be too much boilerplate. Here, you can (but still probably shouldn’t!) use a class decorator. This is basically the ES6 class equivalent of a mixin, and because it’s a way of rewriting the class internals to add behavior or data, it has many of the same pitfalls that mixins do.

    That includes the fact that if you use more than one class decorator (as many people use many mixins) the order they’re applied matters, and you will lose visibility into where the behavior and state actually exist. The spec is also still in flux, three years along, and so you’re taking on extra risk if you write your own decorators: this is something that you will have to change later.

    My personal take is that decorators should almost never be the solution app code reaches for, and library code (i.e. addons, whether internal or open source) should reach for them only very, very rarely.

  • Equally rarely, you may find you want to reach for inheritance to share the same set of behavior and data fields among classes. Long experience in the industry has shown that inheritance is rarely the right way to manage this, though, and that you’re usually better reaching for a mix of DI (with services), delegates (with regular classes), and utility functions.

For a bit more detail (including code samples on some of these scenarios) you may find @pzuraq’s post Do You Need EmberObject helpful reading.

7 Likes

Great question @JonForest and thank you for starting the thread. I and team paddle a similar boat. I’m bookmarking @chriskrycho’s great answer here for when I and team start our migration. Until then, I’m taking the partly-worry-free-wait-for-the-ember-community-tide to raise my boat. Pardon my excessive nautical references.

I say we chronicle refactors in the mixins section of the ember atlas section: https://www.notion.so/Converting-Classes-with-Mixins-5dc68c0ac3044e51a218fa7aec71c2db. (What is the ember atlas? See here)

I feel this topic fits perfectly within the Atlas because there are many variations, each slightly different. Despite the overwhelming combinations similarities must converge. A communal database of examples, however limited, will reduce the development cost dramatically.

FWIW as we migrate our app I expect to fill out a bunch more of that section as well! It’s an explicit goal of the native class migration for us to generate documentation that’s applicable both internally and externally and to share it externally. So hopefully that’ll be improving over the next few months!

1 Like

Thanks for this @chriskrycho - that’s a really complete answer. You’ve added a lot of flesh to the vague ideas I had about the options - I guess there are no real surprises in there except the recommendation to use a non-EmberObject for delegation. I’d not considered that tbh.

Most of it I think will be utility functions. The downside for us will be the mechanical act of repeating function names and action handlers in multiple places to then call the utility function. As frustrating as this is, and as much as it does incur some duplication, I think at the end the more explicit code will pay dividends in terms of readability.

:+1: Glad it’s helpful! For what it’s worth, my experience has been that the slight increase in repetition does indeed yield massive dividends in readability and maintainability. :sweat_smile:

Hi, I’m also trying to migrate mixins, and although I managed to move most of them to services, or completely get rid of them, there are some mixins that are very tightly coupled with the components, meaning I do stuff like:

export default Mixin.create({
    classNameBindings: ['someBinding'],
    doSomething() {
        this.set('key', 'value'); // where key is rendered in template like {{key}}
    },
    init() {
        this._super(...arguments);
        this.set('otherKey', 'otherValue');
        this.initComponent();
    } 
});
  • running stuff on the DOM, using this.element

So, what is the best way to refactor such a mixin? :slight_smile:

I have to say, the functionality that is abstracted in this mixin is shared across 4 components, which have very different behaviour one from another, but still, a good portion of code is shared, thus I found a mixin to be perfect.

I like the idea of delegates, but I’d have to pass almost the entire this of the component. Also, I don’t know how to deal with properties that are rendered in the templates. I mean, how do I write code that works, on Ember 3.12 , where I’m doing some of the migration and where I need to use this.set

Let me start with the easiest part first:

I mean, how do I write code that works, on Ember 3.12 , where I’m doing some of the migration and where I need to use this.set

This you can solve pretty easily by updating every use of this.set to use the standalone function version of set:

  import Component from '@ember/component';
+ import { set } from '@ember/object';

  export default Component.extend({
    doSomething() {
-     this.set('key', 'value')
+     set(this, 'key', 'value');
    }
  });

Once you’ve made that transformation (and the same for get), then you don’t need to worry about whether the object in question is descended from EmberObject or not. I don’t know if there’s a codemod out there that can do this, but even if not you can actually do it safely with a relatively simple regex find and replace.

Second, as regards the specific kind of refactoring you’re doing: this is going to be a bit of work, there’s no way around it. It’d be nice if we could just code-mod this kind of thing, but unfortunately, mixins are so dynamic that every use case would look different.

I’d recommend starting by identifying the intent of the shared code and breaking it apart as much as possible. For example: repeating the classNameBindings is worth just copying into each usage—not least because when you migrate to Glimmer components, you won’t be using classNameBindings anyway, but will be doing all of that in the template! The same goes for attributeBindings.

Anything that isn’t stateful, you should just pull out into standalone functions. And a lot of times things that are stateful can be turned into stateless functions. For example, if you are wrapping around a this.store.query(...) because you want to have the shared configuration for it, you can pull the this.store.query(...) invocation back out to the components’ actions, but use a function to build the configuration for it. That standalone function becomes very easy to test: no need for the store, or mocking API endpoints, or anything, just “Did I get out the configuration I should have given the arguments I passed in?” Then the components can just do something like:

  import Component from '@ember/component';
+ import { inject as service } from '@ember/service';
+ import { buildConfig } from 'app/lib/my-config-builder';

- export default Component.extend(TheMixin, {
+ export default Component.extend({
+   store: service(),
+
    actions: {
      doSomething() {
-       this.mixinStoreMethod();
+       this.store.query(buildConfig(this.someValue));
      }
    },
});

(If the action itself is shared you’d also need to add it to the components, but you get the idea.)

In other cases, you may also be able to begin by using a delegate and passing this into its methods, and then iterating from there. The best way to shift to using a delegate, though, would be to pass an object derived from this instead. That way you can clearly and explicitly define what’s actually required by the delegate, rather than just coupling yourself to the innards of this permanently. That in turn gives you more tools for refactoring, because you can clearly see what you might want to whittle down or break into separate pieces later.

One last note: it helps me to remember when tackling things like this that I also don’t have to accomplish the whole refactor at once—and in fact probably shouldn’t! Instead, isolate one piece at a time, identify the right solution for that, test that one small change well, and ship it. Then repeat! Eventually you’re done, but each step was small, self-contained, and manageable.

Thank you so much for the help. But if I use this.set inside a mixin, on a property that is being rendered in the component’s template, what is the best way to replace this?

On Ember 3.12, I guess I could create a delegate that receives this.set of the component, and calls that function, thus the template would update.

But on Ember Octane, I would have to create a function on the component, like set(property, value) { this[property] = value;} , that I would pass to the delegate, so that the template would update, right?

I’m assuming property is a primitive, like an integer, so the delegate constructor would receive a value copy, making it impossible to update the template without calling a function on the component.

Nope, you can use set with that the way I showed. The Octane system is designed so it’ll interop correctly that direction. You can always safely use set safely (not this.set! The standalone function version), and while you’re transitioning the codebase, that’s what you should do.

The other thing here is that you should probably reevaluate (in the medium term, at least) whether you want the delegate setting properties back in the host. (Hint: you really, really don’t! That kind of coupling will kill you later.) It may be a necessary intermediate refactoring step, but you should aim to move away from it as possible.

Depending on the nature of what you’re doing, it’s also worth note that you may end up with multiple delegates as well as multiple helper functions, etc. out of a single original mixin!

Thank you! I have no idea how I’m gonna refactor some of the mixins :smiley: , but I’ll try

Such a useful thread. Thank you @chriskrycho

1 Like