Is mixin for non-supermixins a good idea?

[yjbanov]

I'm filing this issue as a place to discuss whether using the new mixin syntax for non-supermixins is a good idea. I do not wish to pollute issue #32 with this discussion, as I'd like that issue to be focused on the semantics of extends w.r.t. mixin.

The arguments presented so far:

@Hixie (#32 (comment)):

Personally I have no objection to being able to use "with" with a true class. I would not want a lint that discourages such use.
I don't see much value in the "mixin" keyword. I'm ok with having it if it makes the language easier to process in the context of supermixins. I would not encourage people to move to that keyword except where required by the supermixin restrictions.
In particular, I would not change WidgetsBindingObserver to use the "mixin" keyword.

@yjbanov:

mixin comes certain restrictions on what you can do with the class, such as:

• mixins cannot extend other classes
• mixins cannot have constructors and factories
• mixins cannot be instantiated directly

These properties protect mixin authors from accidentally breaking their users. I believe this is an important property for keeping the Flutter/Dart ecosystem healthy. Mixin usage is much higher than mixin authorship and therefore the cost of using mixin vs class is dwarfed by the stability guaratees the mixin semantics provide.

Some examples in Flutter that may be used as super-classes or as mixins:

• LocalHistoryRoute
• WidgetsBindingObserver

Updates to the original issue description:

• Added examples from Flutter

[yjbanov]

More arguments can be found in flutter/flutter#22290. For example:

My worry is that in the future we may accidentally break the API by adding a seemingly innocent super call or attempt to extend another class.

The solution to that is to add tests. I would certainly support adding more tests to make sure we don't break that particular pattern.

The solution to that is to add tests.

This solution works for highly disciplined engineering teams. But I don't think it works for the ecosystem as a whole, where you will have users of varying levels of expertise. Using classes as mixins is known to be quirky. If there is a better way, I say we should accept it. I think dart-lang/language#31 (comment) is a better way.

[lrhn]

The problem with allowing any (no-constructor, Object-superclass) class to be used as a mixin is that it makes more changes into breaking changes for that class.

Adding tests cannot catch this.

If someone else, in a package you never heard of, is using your class as a mixin (maybe even on a const class), then you break their code if you

• Add a generative constructor (e.g., by changing a factory constructor to be generative).
• Add a super-class.
• Add a first field (if used on const class, and someone uses the result as const)

These are not changes that would normally count as breaking, so most likely you won't do a major-version increment when you refactored the class to share some functionality with another class through a private super-class - but if they update their dependencies, their code stops working.

So, any API containing any class that can be used as a mixin, should do a major-version increment whenever they change such a class so it can no longer be used as a mixin.
Obviously nobody wants to do that, which is why there is this request to ensure that a non-mixin class cannot be used as a mixin.

[eernstg]

Why don't we simply answer "Yes, it is indeed a good idea to use mixin for a declaration M that is intended to be used as a mixin, even in the case where no method implementation in its body performs any superinvocations!"

The point is that this is a contract that involves the author(s) of M and the clients of M, and the software artifacts containing M are simply going to be more maintainable when such a tightly coupled piece of reusable code as a mixin is marked explicitly as such.

Do we need a 'resolution-yes' label? ;-)

[Hixie]

I don't see why mixins are special here. Any class can be used as an interface too, with the same problems.

[munificent]

Any class can be used as an interface too, with the same problems.

That's one of the reasons we're looking into sealed classes and the ability to express a class that cannot be implemented. The language already supports this for a hand-picked set of magic classes — bool, double, num, int, and String. If it's useful for those types, it's likely useful for user types.

[Hixie]

Sure, but most classes can't be sealed since they're intended to be subclassed, and almost all classes need to be implementable for mocking purposes.

[munificent]

Sure, but most classes can't be sealed since they're intended to be subclassed, and almost all classes need to be implementable for mocking purposes.

As far as I know, we don't actually have any real empirical data to justify a claim like that.

[yjbanov]

Adding tests cannot catch this.

More than that. In practice, few developers write tests.

I don't see why mixins are special here. Any class can be used as an interface too, with the same problems.

One special thing about mixins is that we are trying to fix them now. We are currently not trying to fix interfaces, which, I agree, also have problems. I guess I don't understand what conclusion this argument is trying to reach.

I'm also not sure if the problem are the same. For example, if a class is also used as an interface, then adding a factory or a generative constructor, extending another class, and adding super calls are rarely breaking changes. OTOH, these changes are guaranteed breaking changes in the case when a class is also used as a mixin.

Classes-used-as-interfaces do have issues though, and I think they should be addressed at some point. Some background is documented here.

most classes can't be sealed since they're intended to be subclassed, and almost all classes need to be implementable for mocking purposes.

Perhaps, but also most classes do not need to be fast. In practice, only a small portion of code is on the hot performance path, and that's where you will see benefits from things like sealed, static dispatch, memory-locality, etc. For example, Dart draws a lot of performance benefits from keeping int, double, String and bool sealed. What's changed recently is our realization that this could have impact beyond built-in classes. C++ is not fast because every single line of code your write is fast, but because it gives you the tools to make the 5% of code that does need to be fast fast.

Not that this particular issue has to do with performance. I'm just doubting the logic in the claim that a feature should be rejected because of low usage count. I think a feature's inclusion should be evaluated according to its overall impact.

[mit-mit]

@yjbanov @lrhn did we reach a conclusion here? Can we close this issue?

[yjbanov]

AFAICT, no conclusion has been reached yet.

[lrhn]

My opinion is definitely that, yes, it is a good idea to use mixin for all mixins.

I'm not sure what it would take to close this issue, though.

[munificent]

My opinion is definitely that, yes, it is a good idea to use mixin for all mixins.

Me too. But, either way, I think this is a question about best practices and not something actionable in the language repo. Maybe it would be worth filing a bug for what "Effective Dart" should say here, but that's a separate repo.

[yjbanov]

The goal of the issue is to get a recommendation from the language team so we could follow it in Flutter and steer our users towards it via analysis_options_user.yaml.

If the recommendation is to use mixin for all mixins, then we're back to issue dart-lang/sdk#57178, which prevents us from being able to follow the recommendation.

[rodion-m]

And what is the solution? effective_dart abuses about using WidgetsBindingObserver as a mixin, but it's a code from the example: https://api.flutter.dev/flutter/widgets/EditableTextState-class.html

[lrhn]

My strong recommendation is to never use a class as a mixin and never design a class as intended to be used as a mixin.
My long time goal is to remove the ability to use classes as mixins. It's not necessary now, except for backwards compatibility.

If you change all current classes used as mixins to be declared as mixins, and change all uses of those classes as superclasses from extends TheClass to with TheClass, then everything should keep working, and we could remove the ability to use classes as mixins tomorrow (and I would do that!)
The only thing holding us back is that we need to change all the existing extends ClassAlsoUsedAsMixin to with ClassAlsoUsedAsMixin before being able to change its declaration to mixin ClassAlsoUsedAsMixin { ... }. That's a migration step with no obvious enforcement. Changing class to mixin first breaks the existing code (easy to fix, but still broken). Changing extends to with is hard to automate because there is no obvious hint on the class declaration that it's intended to be used as a mixin. It looks like a perfectly normal class and your code is just extending it. You have to be able to see another instance of with TheSameClass to be able to decide that the extends should also be with.

We could introduce an intermediate @asMixin annotation that can be put on the class TheClass declarations, then we can automate the migration of all instances of extends TheClass to with TheClass, and finally we can remove the annotation again.
Or we can do as I propose in dart-archive/linter#1643 and use language versioning.

[Hixie]

I don't really see the value in losing the ability to extend a mixin or mix in a class.

Can you elaborate on what the problem is with ChangeNotifier being a class that is mixed in?

[lrhn]

The problem with allowing you to derive mixins from class declarations is that it's not properly signaled and contained. It's opt-out instead of opt-in, and changing it later is a breaking change (aka. accidental lock-in).

You can derive a mixin from a class declaration iff

• The class declaration has Object as superclass, and
• The class does not declare any generative constructor.

Nothing about that screams "intended as a mixin", just "not incompatible with being a mixin".

The resulting mixin can be applied to any class, but cannot do super-invocations except on Object methods, as if the corresponding mixin was declared as mixin Whatever on Object.

That's much more restricted than what you can do with a mixin declaration, and it means that you could just as well declare the thing as a mixin. You have the same power (you can use it as a mixin, and you can effectively extend it by writing with Whatever instead of extends Whatever), but you get much more guidance, both as an author and as a user of the mixin.

We'd be able to say that the only thing you can mix in is a mixin, and you should migrate all classes extended as mixins to be mixins.

Needing to be declare as a mixin automatically prevents any non-mixin-intended class from being used as a mixin.
You can't just use any class satisfying the rules above as a mixin, whether it's intended for it or not.
If anyone does use a class as a mixin, as they can with the current rules, adding a constructor or a superclass becomes breaking changes. (Our breaking change policy explicitly calls out that we allow ourselves to break code using classes as mixins if the class isn't documented as being intended as a mixin). It's also breaking to add a non-final field, if the mixin is used in a way which forwards a const constructor.

So, using classes as mixins is dangerous because you can use classes that are not intended so as mixins. Because the rules are enabling being used as a mixin from secondary properties of the class, not a deliberate documented desire to be a mixin, it's just bad language design. You can accidentally enabled your class being used as a mixin, and you won't find out until someone uses it and you break them. It should be opt in.

The SDK has deliberately added constructor declarations to some classes to prevent it from being used as a mixin (after realizing someone was doing it). It's easy to forget to do that (and also, why should you), but we were not in a position where we could just ignore it and possibly break people later.

And since a mixin declaration is just as powerful as a any class declaration which can be used as a mixin, with only one small change of syntax needed in how you "extend" it, you should just use mixin every time.
(Now, that one small change of syntax, and the migration needed to get to it, and the very low number of mixins in actual use, means that we haven't disallowed deriving mixins from classes yet. I still want to. I'll make another push when the time is right.)

tl;dr: The real problem is not that ChangeNotifier is a class that can be mixed in. It's that other classes, that are not intended as mixins, can also be mixed in. ChangeNotifier would also work if declared as a mixin.

[Hixie]

I hear what you're saying. There's downsides too, though. Things "are" change notifiers, which is expressed by saying "extends" (gives an "is-a" relationship, as opposed to mixins, which is conceptually more of an "includes-a" relationship). It seems like the concern you describe can be entirely resolved by just having the annotation and lint behaviour proposed in dart-lang/sdk#58543, without needing to change the language. That way, people don't use arbitrary classes as mixins except those that opted-in.

[Levi-Lesches]

@Hixie, is the problem of extending a ChangeNotifier that Flutter wants users to be able to include ChangeNotifiers even when extending other classes? Because based on your comment, I went through my project and changed with ChangeNotifier to extends ChangeNotifier and got no errors, but my models don't extend anything else.

[Hixie]

Yeah sometimes people just want to mix in ChangeNotifier to other elaborate class hierarchies.

[Levi-Lesches]

So it seems like ChangeNotifier (and I assume other similar classes) is used in one of two ways:

• by Flutter to make a more refined type of listening (class ValueNotifier extends ChangeNotifier)
• to users to connect the backend logic to the frontend (class MyModel with ChangeNotifier)

These two seem similar enough to merge into one. ValueNotifier can be seen as "a class that includes the ability to notify others when its value changes" instead of "a more specific type of the class ChangeNotifier". Does something else go wrong with changing the Flutter classes to use with instead of extends? It's not like ValueNotifier can be mixed in anyway.

[Hixie]

The vast majority of uses of ChangeNotifier that I'm aware of are just using it as a superclass with extends. They have an "is-a" relationship with that class. They "are" change notifiers. Dart, unlike other languages, is powerful enough that you can also in cases where just inheriting from ChangeNotifier doesn't work, mix it in using with.

[munificent]

The vast majority of uses of ChangeNotifier that I'm aware of are just using it as a superclass with extends. They have an "is-a" relationship with that class. They "are" change notifiers.

That's equally true of mixins. Consider the class hierarchy: List > Iterable > Object. I think most of us would agree that a List "is-a" Iterable.

But if you apply a mixin, you get the exact same thing: MyClass > ChangeNotifier > Object. So a MyClass "is-a" ChangeNotifier just as much as a List is-a Iterable. I understand many users don't have that mental model, but that's I think largely because mixins are uncommon and we haven't worked hard to give users a mental model for them.

Mixins are superclasses. They're just superclasses whose own superclass is pluggable.

[Hixie]

While that's technically true, it's not a mindset that's supported by the language. The language uses "extends" vs "with". Extending something sounds like an "is-a" relationship. Saying "with" does not, it sounds more like a "has-a" or "includes-a" relationship. I think we'd be fighting an uphill battle which would make the language and framework harder to understand if we tried to push people to mix in ChangeNotifier instead of extending it.

[leafpetersen]

I think I'm somewhat in agreement with @Hixie here. Extending is by far the simplest and most familiar way for a user to get access to some shared functionality in an OO language, and if that affordance works for users in the 95% case, then it feels like we should provide the ability for library authors to offer it (e.g. mixin class), even if in the 5% case they have to switch to using with. I don't think it's hard for users to get to "normally I'd extend this, but in multiple inheritance cases I can mix it in instead" when they really need to.

[munificent]

if that affordance works for users in the 95% case, then it feels like we should provide the ability for library authors to offer it (e.g. mixin class), even if in the 5% case they have to switch to using with.

Do we need mixin class (i.e. mixins that are classes) for that, or do we just need to allow extends SomeMixin as syntactic sugar for extends Object with SomeMixin?

I'm not keen on supporting mixin class because if we let type authors have that, they will expect to be able to do all the things with it that a class can do, like give it a superclass. But if you give your mixin class a non-Object superclass, then its behavior gets really surprising when the type is used as a mixin, since the declared superclass (and all the methods the person using it might have thought they were inheriting) disappear in a puff of smoke.

[Hixie]

I'd be fine with the compiler complaining if you tried to mix in a class where there are concrete members in superclasses of the class you're mixing in.

[Hixie]

(FWIW the declared superclasses don't seem to completely disappear, only their implementations do; for example, you still have to implement those interfaces.)

[Levi-Lesches]

I think we'd be fighting an uphill battle which would make the language and framework harder to understand if we tried to push people to mix in ChangeNotifier instead of extending it.

When using ChangeNotifier in my Flutter apps, I always use the following structure:

// model.dart
abstract class Model with ChangeNotifier {
  /// Initialize data here.
  Future<void> init();
}

// user.dart
class UserModel extends Model {
  Future<void> init() => loadUserData();

  Future<void> updateUser() async {
    Firestore.updateUser(await loadUserData());  // some update
    notifyListeners();
  }
}

I always mix-in ChangeNotifier (despite the prefer_mixins warning). In my case, I have a hierarchy of my own, so while I do extend the Model class, I think of it as "UserModel is a Model that is able to notify the UI when it changes" (in other words, "includes the ability to..."). Even before I worked out this structure and mixed in ChangeNotifier directly, I was still thinking "this is a container class for data that has the ability to notify the UI when it changes".

So IMO, getting users to think of ChangeNotifier as functionality they can "include" in their data models (instead of making their models "become" a ChangeNotifier) is pretty intuitive, and my team and I have been going about it that way for a while now.

[lrhn]

We now have mixin class declarations, which can be extended and mixed in (but not declare super-classes, super-interface-constraints, or non-trivial constructors).