Ember-simple-auth: Use without mixins

[mike-north]

Is there a plan for allowing consumption of ember-simple-auth in a codebase that bans or restricts use of mixins? Examples include a codebase making use of ES6 classes and/or TypeScript, where mixins are largely unsupported (and will be for the foreseeable future)

One ultra-low-effort solution to this problem might be to expose the functions on the object passed to Mixin.create( ) for ApplicationRouteMixin, AuthenticatedRouteMixin, etc... for direct consumption.

import { beforeModel as simpleAuthBeforeModel } from 'ember-simple-auth/mixins/authenticated-route-mixin'

export default class extends Route {
   beforeModel() {
       super.beforeModel(...arguments);
       simpleAuthBeforeModel(this, a, b, c);
   }
}

[geekygrappler]

I agree we should try and move away from mixins at some point.

[marcoow]

The big advantage of mixins is that they make incorporating ESA's into apps very easy. Of course @mike-north's approach above works well but especially for mixins that add multiple methods (and properties) that's tedious and error-prone. We should investigate closely what options we have for dropping mixins without making integrating ESA much harder than it is today.

Just thinking out loud here but what if we just switched to classes?

import AuthenticatedRoute from 'ember-simple-auth/routes/authenticated'

export default class extends AuthenticatedRoute {
}

There are likely other alternatives…

[Romanior]

One possible way, as well just thinking out loud, could be to wrap Route class with custom creation function, for more cleaner user's class and simple integration, for instance:

import { createAuthenticatedRoute } from 'ember-simple-auth/routes/authenticated'

class ApplicationRoute extends Route {}

export default createAuthenticatedRoute(ApplicationRoute, { /* optional config */ });

[mike-north]

We should investigate closely what options we have for dropping mixins without making integrating ESA much harder than it is today.

I think we can find a way that is no more difficult, but possibly more explicit (a good thing imo) than just adding (mystery) mixins in several places. Let's aim for easiest to understand and maintain, and fight the temptation to optimize for smallest # of lines.

Something like this is easy for developers to understand, and they're free to use inheritance for their own needs instead of extending from some base class

import { redirectIfUnauthenticated } from 'ember-simple-auth/authenticated-route'

class HomeRoute extends Route {

   beforeModel(transition) {
       super.beforeModel(transition);
       redirectIfUnauthenticated(transition, 'auth.login');
   }
}

Or the decorator approach

import { authenticatedRoute } from 'ember-simple-auth/authenticated-route'

@authenticatedRoute
class HomeRoute extends Route {

}

I would like to caution against something like this -- it's essentially an "ES6 mixin", and has even more pitfalls than ember mixins

// ember-simple-auth/authenticated-route.js
export function authenticatedRoute(baseRouteClass) {
   return baseRouteClass extends {
         beforeModel() { .... }
   }
}

// app/routes/home.js
import Route from '@ember/routing/route';
import { authenticatedRoute } from 'ember-simple-auth/authenticated-route.js';

export default class extends authenticatedRoute(Route) {

}

To be clear, my hope is that all this can be accomplished through a little clever refactoring, without disturbing the existing mixin-oriented public API.

[marcoow]

I think we can find a way that is no more difficult, but possibly more explicit (a good thing imo) than just adding (mystery) mixins in several places. Let's aim for easiest to understand and maintain, and fight the temptation to optimize for smallest # of lines.

Totally agree. I'm not worried about number of lines but the amount of complexity we'd be putting on the dev's shoulders if instead of just mixing in one mixin in the right class, they'd have to do lots of wiring themselves.

The problem with the first approach above:

import { redirectIfUnauthenticated } from 'ember-simple-auth/authenticated-route'

class HomeRoute extends Route {
   beforeModel(transition) {
       super.beforeModel(transition);
       redirectIfUnauthenticated(transition, 'auth.login');
   }
}

is that while being relatively easy (and explicit) for the AuthenticatedRouteMixin, that approach would be more complex for something like the ApplicationRouteMixin or the OAuth2ImplicitGrantCallbackMixin. My fear is that we'd end up in a situation where we tell the dev _"you have to do call all these methods in the right places (without necessarily understanding what they are doing) in order for ESA to work as you know it"_.

I'm not 100% convinced switching to classes would lead to a lot of pain in reality actually. I understand, forcing people to extend another class than Route makes setting up custom class hierarchies more tedious but I wonder how that would affect people in reality. I have rarely seen people using custom base classes in Ember much in reality and even if they did, having another layer of extension would usually not lead to much pain I think?

Lastly, I'm not sure whether there's some sort of mechanism that's establishing in JS outside of Ember that could replace mixins? I guess that would likely be

// ember-simple-auth/authenticated-route.js
export function authenticatedRoute(baseRouteClass) {
   return baseRouteClass extends {
         beforeModel() { .... }
   }
}

// app/routes/home.js
import Route from '@ember/routing/route';
import { authenticatedRoute } from 'ember-simple-auth/authenticated-route.js';

export default class extends authenticatedRoute(Route) {
}

although the authenticatedRoute(Route) might be too much for some people maybe and something more _"standard"_ might be preferable.

So long story short, if we could make the first approach work without it becoming too complex, I'd totally be in favor of that as it's probably _"the right thing to do"_ anyway but I'm not sure that's easily possible…

[mike-north]

The problem with the first approach above is that while being relatively easy (and explicit) for the AuthenticatedRouteMixin, that approach would be more complex for something like the ApplicationRouteMixin or the OAuth2ImplicitGrantCallbackMixin. My fear is that we'd end up in a situation where we tell the dev "you have to do call all these methods in the right places (without necessarily understanding what they are doing) in order for ESA to work as you know it".

Looks like there's already a mixin-free approach for ApplicationRouteMixin,

https://github.com/simplabs/ember-simple-auth/blob/76e0eacf94f1d1d4d1194b3ef93ceae0bc5e000e/addon/mixins/application-route-mixin.js#L17-L37

which makes sense since there's not much need for it to be tightly coupled with a route. It just needs to:

• Execute some code pretty early, regardless of which URL the user is entering the app through
• Setup some service:session event handlers
• Trigger transitions (can now be done via RouterService)
• Have access to the session service and some auth-specific options (routeAfterAuthentication)

Come to think of it, the number of reasons why ApplicationRouteMixin's job must involve routes may be zero!

The initializer approach commented above is 💯, particularly if the router was used for .transitionTo instead of the route:application! I'd love to help w/ continued work that allows a more flexible way of using simple-auth (it may even open the door to great patterns you never considered before).

Looking at OAuth2ImplicitGrantCallbackMixin -- the only point of interaction it requires with a route is Route#activate() -- everything else can live in an arbitrary place (i.e., they're already pure functions and have no need to live on an object at all ). Consumers would add one function in one hook, and would _know_ that simple-auth is doing some work in the activate() hook.

Lastly, I'm not sure whether there's some sort of mechanism that's establishing in JS outside of Ember that could replace mixins

Great abstractions show just enough information about the problem being solved (and how it's being solved) that you can have an idea of how it works -- mixins often expose almost no information about what they do or how they do it. As such, I'd caution against looking for "another way to mixin".

Many teams who have been plagued by mixin-related pain have undertaken efforts (at LinkedIn this is called "Project: Mixout", at Yahoo it was called <sound of head slamming against keyboard>) to refactor such that composition ("Route has a thing") replaces inheritance ("Route is a thing")

I'd be happy to put my money where my mouth is, and take a stab at some of this myself (the codebase for this addon is so refined and well-documented that it doesn't seem like a big job), with the promise of keeping the public API surface intact. A good start would be

• [ ] Move all pure functions off of mixins and into ES6 module space
• [ ] Favor use of service:router over route:* for any need to .transitionTo
• [ ] Move some of the private functions currently on Mixins into their own dedicated Ember.Object subclasses

at which point, we'll see where the true points of coupling are between Mixin and Thing-That-Is-Mixed-Into

[geekygrappler]

I'd be interested in helping out with this issue.

[marcoow]

I think the main reason (besides there being no router service back then) for using mixins was that they made it easy to get the default implementations of methods into classes where they could easily be overridden. When you mix in the ApplicationRouteMixin into the application route, you don't only get the event mapping but also the possibility to do

Route.extend(ApplicationRouteMixin, {
  invalidationSucceeded() {
    // do my custom thing here
    this._super(...arguments); // make sure all of ESA runs
  }
});

I think maintaining an easy way for extending ESA's out of the box functionality is important when we look at new solutions.

I'm looking forward to what you'll come up with @mike-north! The plan you laid out above looks 💯

[jfelchner]

I personally would like to see a decorator approach. I think it makes it explicit and easy to reason about.

[tschoartschi]

Since Ember is moving to native classes it would be interesting to hear the current status of this issue 🙂

I also think that a decorator approach would fit better to modern Ember.

[mike-north]

@tschoartschi - class decorators have some of the same pitfalls as mixins, and with the decorator spec in flux, there's significant risk and maintenance overhead around using decorators for things like this. I'd like to see a flexible "regular function API" as the means for consuming ember-simple-auth in an ember octane app.

[tschoartschi]

@mike-north thanks for the reply :-) so, for now, we would need to do something like:

import ApplicationRouteMixin from 'ember-simple-auth/mixins/application-route-mixin';

export default class ApplicationRoute extends Route.extend(ApplicationRouteMixin, {}) {

}

Or are there better solutions?

[mike-north]

Or are there better solutions?

See the top post in this thread for a "regular function API" example. Mixins and decorators are both abstractions that hide very important details from consumers.

[tschoartschi]

@mike-north it's not totally clear to me if this is a solution which already works and should be used (and is supported by the maintainers of ember-simple-auth) or if this relies on internals of ember-simple-auth which are subject to change. Right now I just need a solution which works with native classes yet and which is "easy" to refactor if ember-simple-auth changes its mixin syntax.

[mike-north]

@mike-north it's not totally clear to me if this is a solution which already works and should be used (and is supported by the maintainers of ember-simple-auth) or if this relies on internals of ember-simple-auth which are subject to change. Right now I just need a solution which works with native classes yet and which is "easy" to refactor if ember-simple-auth changes its mixin syntax.

The exact example I cited is a goal, not something that currently exists. Some things are possible already w/ no mixin use (i.e., https://github.com/simplabs/ember-simple-auth/issues/1619#issuecomment-403178107)

Once we have more of this new part of this initiative fleshed out the plan is to unveil and support a new (additional) portion of the public API that allows for consumption w/o mixins.

[jfelchner]

@mike-north for arguments sake, all a decorator is is a function that wraps another function, so _technically,_ it would still be a functional API. ;)

[tschoartschi]

For now we just use the solution I outlined here: https://github.com/simplabs/ember-simple-auth/issues/1619#issuecomment-471614567 it works fine also together with TypeScript but nevertheless, it would be nice to see the same APIs across the whole ember ecosystem.

[josemarluedke]

What is the status of this? Are there any plans to move this forward and remove mixins from the programming model? Ember Octane is approaching fast, and it would be super helpful not to need mixins anymore. Also, there is a Pre-RFC to deprecate mixins: https://github.com/emberjs/rfcs/issues/534.

[marcoow]

This will require a bunch of work. We're currently focussing on getting a 2.0 release out which would allow us to remove a bunch of cruft. Once that's done, we can focus on future initiatives again like this one.

[bitwolfe]

Has there been any progress on this now that 2.1 has been out for a while?

[davideferre]

I've updated today my in-development app to ember-source 3.17.3 (from ember-source 3.17.1) and I've received a linter error Don't use a mixin (2:1 error Don't use a mixin ember/no-mixins).

[oliverlj]

I confirm the linter error with package eslint-plugin-ember v8, not error in v7
The error is specified here : https://github.com/ember-cli/eslint-plugin-ember/blob/master/docs/rules/no-mixins.md

[marcoow]

Quick update for everyone: There is currently no way to use ESA without mixins and thus that linter error. You'll have to disable that warning wherever you use the mixins that ESA comes with. We are working on a decorators-based version of ESA and will release that soon (🤞). Until then continue using the mixins as described in the docs – there are no immediate negative consequences of that.

[cmitz]

Thanks Marco!
For the rest who haven't seen the emberconf vid, here's a sneak preview of what it could look like:
https://youtu.be/qHkY8Uyd5TE?t=1999

[marcoow]

mixins are deprecated now – see #2198