-
Notifications
You must be signed in to change notification settings - Fork 209
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
Allow mixins in "extends" clauses #1942
Comments
As someone who took quite a while to understand mixins, I think this will be too confusing for several reasons:
IMO, it might just be simpler to attack the root of the problem: the user is trying to share functionality with a mixin but thinks
How about a similar error when mixing in a class?
This way, the compiler explains to the user
As for the timeline, I think that with |
This depends a little on what you mean by "extend". When you have a "with" clause, the mixin does end up as the superclass of the class you're declaring. So, in some sense, mixins are always extended by the subclass. It's just a question of what goes in the superclass's superclass slot.
It would. It would allow exactly this. Flutter can change ChangeNotifier to use
It would still allow that by using a
I agree that it does obscure the semantics of mixins, which is why I wasn't in favor of this approach initially. But... the reality is that a lot of users just don't understand mixins and a compile error saying "write
This was my initial thinking too. But it does seem cruel to have the compiler say, "I know exactly what you meant to do here, but I'm going to yell at you and make you change your code anyway." There is a more pragmatic angle to this too. If we allow mixins to appear in |
My criticisms have to do with the fact that this workaround is meant specifically for users who don't know much about mixins and using
I mean the behavior you get when using the
But the point is new users would be using
Right, but again, there would be no way for users to know they should be using
Sort of did for me! The mixins that beginners use like
I guess it would make sense if this were a lint or warning then, because those pretty much cover the lines of "you're probably wrong about this, and you probably meant to do this instead". I proposed an error because I was afraid someone would try using a class as a mixin and it wouldn't work (because it has a constructor or superclass), then they'd wonder why it ever worked before and realize they were doing it wrong. A warning is "your code works but this is likely not what you want", but an error is "you're doing this wrong". Either is fine in this case.
Like I said before, I don't think this is that big of an issue with versioning and |
Yes, but that's arguably a feature. In general, it's nice if users don't have to learn every corner of the language at once. We provide both If a user wants to use ChangeNotifier and only knows about
I think we could also add a lint that suggests users use
I'm talking about change going the other way. Because mixins are not widely known in Dart, there are a lot of abstract classes out there that extend Object and have a default constructor. Many of those classes could become mixins and the author might be happy to turn them into mixins (using an explicit |
All of the issues I listed are about what the "default" choice is -- I suppose it would be weird having a supported language feature immediately throw up a warning, but there are other cases like using |
I agree that it would be better to have a clean separation of mixins and classes, where you can only The primary goal is to disallow mixing in classes. That's just fundamentally problematic because there is no way to declare that a class is or is-not intended to be used as a mixin, it's derived from secondary characteristics which can accidentally be true of a class that is not intending it, and can be changed by what would otherwise be non-breaking changes to a class. That's just bad. What's currently blocking us from disallowing mixing in classes is that people are still using it. We could then try to force those classes to be declared as mixins instead, but some of the classes are used as both superclasses and mixins. Even if we could convince Flutter to change So, by allowing you to extend mixins, we increase our chance to make Flutter change And also remember that you can only If we get to Dart 3.0, and nobody (or almost nobody) has any That's the goal. This is a first step towards that goal. Removing the ability to extend a mixin again can be a stretch goal, which we only do if it ends up making sense and everybody being happy about it. I'll be happy even if we don't get there. (Heck, maybe everybody will then love being able to do |
Yeah, I like this gradual changes proposal. I only would change the last step to completely disallow "extending" mixins, as we will be going to Dart 3.0, where breaking changes makes sense.
IMHO, I think in Dart 3.0 we should enforce it eve if there are people extending mixins... Obviously, we should gently guide them in the meantime (with lints, recommendations etc). If they don't follow the recommendations, it's their fault to have breaking changes in their system. |
FWIW, if the goal here is to allow Flutter to migrate ChangeNotifier from a class to a mixin, I don't think the proposal here helps that. |
@Hixie Why not? Our end goal is to disallow mixing in |
|
(I also don't really understand the benefit of allowing |
We don't currently have a fix for that, but we certainly could. |
I'm a little confused. The docs don't show it having any superclass other than What am I missing here? |
Sorry I was thinking ValueNotifier, ChangeNotifier is just one step down. The inheritance hierarchy is:
(The top link is done via I don't really understand what problem we're trying to solve here, though. As far as I can tell, everything works fine today. The only issue is that |
That distinction matters here. If ChangeNotifier extended Listenable, then users wouldn't be able to use it as a mixin (unless they manually implemented the Listenable interface themselves).
To fix 3, we can simply allow mixins in
There's no material benefit to allowing a type to be used as both a mixin and superclass. They both result in a new subclass whose superclass is the mixin/class thing. The only capability you get is being able to use it in an |
Is it? It doesn't seem like that much of a burden. Do we have data supporting this? This really does not seem like a high priority change to the language. There are far bigger fires for us to deal with, IMHO. |
I've been wondering about this: Can you make a mixin that shares functionality with another mixin? The closest I could get to this was: mixin A { void a() => print("a"); }
/// Trying to implement `B extends A`, such that `A` is an implementation detail
mixin B on A { void b() => a(); }
/// But A is not an implementation detail, C has to explicitly mix it in
class C with A, B { }
void main() => C().b(); // "a" |
I don't know if we have numeric data, but we've heard from API maintainers over the years that Dart's flexibility around how classes can be used makes it harder to evolve APIs without breaking them.
Sure, but it's also a tiny change. |
@Hixie It is absolutely a burden for library maintainers, and we have data to support it in the form of extensive experience as library maintainers. The current best practice in numerous Dart team maintained libraries is to document classes in with comments indicating that a class is not intended to be mixed in. Example. But the problem with comments is that if someone ignores them, then making a change that breaks a use as mixin still breaks the downstream customer, and so we are still on the hook to either fix the issue on a roll (internally) or to deal with the breakage (externally). |
No, not at all. You can't use a class as a mixin if it has a superclass other than So, if your class has an |
The problem we are trying to fix is that you can mix in classes that are not intended to be mixed in. Allowing that is a problem. If anyone uses the class as a mixin, it becomes a breaking change to add a constructor or a superclass. Obviously, you can just say "I don't care" and add a supertype or constructor anyway. Even our own breaking-change policy does not consider programs doing mixin application of classes not intended as mixins as valid programs. (We're a little too lenient for my taste: "must not mixin a class clearly documented as not intended to be used as a mixin", where it really should be "not clearly documented as intended to be used as a mixin"). In practice, it would be better to not allow you to mix in those classes at all, instead of allowing the foot-gun. So, we need to either introduce new syntax to declare classes that can be used as mixins, and disallow mixing in any other class, or to just disallow mixing in classes entirely, and have a migration path for the classes which are currently being used as mixins, so they can be come mixins. The former would be a declaration like The latter would just make it an error to mix in a class. Whoever wrote the class that was intended as a mixin would have to change it to a So, different proposals: In a new language version:
Assume we choose one of these, and do it in 2.16, which one would you prefer? (In either case, you'd have to change |
It seems like one of the questions we circle back to (and if I'm reading it right, the difference between options 2 and 3), is whether you should be allowed to extend a mixin, barring migration conveniences. Using the example of Flutter's |
Since |
Right, my comment was addressing the earlier question of whether |
Seems to be handled by the language-versioned switch in Dart 3.0 to needing We could still allow |
Yeah, I'm going to go ahead and close this because I think the language is more or less where we want it to be now with |
We would like to deprecate and eventually stop supporting using
class
declarations as mixins. There are a couple of proposals out for how to do that: #1529 and #1643. The main sticking point for simply making it a warning or error to mix in a class is Flutter.Flutter has a number of class declarations that are widely used as both mixins (in
with
clauses) and as superclasses (inextends
clauses). Flutter can't change those class declarations to be explicitmixin
declarations because it would break everyextends
use of the classes in the wild today.We could potentially have a deprecation period where we encourage Flutter users to change all of those
extends ___
toextends Object with ___
, but it's not clear that that's actually a desirable ecosystem change. Most Dart users seem to be unaware of mixins so forcing them to learn aboutwith
before they can use common functionality like ChangeNotifier might be punishing for little benefit.Here is a simpler proposal:
Allow using
extends ___
on mixins and treat it as syntactic sugar forextends Object with ___
. Unlike the previous proposals, we don't make this a temporary language feature. Instead, we simply allow that sugar henceforth and forever.With this, Flutter classes like
ChangeNotifier
can immediately be changed tomixin
instead ofclass
without breaking or effecting any users.After Flutter and other key packages have migrated their mixin-in-able classes to use
mixin
, add a linter rule that warns when users mixin a type declared usingclass
. Eventually promote that to a warning.In Dart 3.0, remove support for mixing in classes. Continue to allow mixins to appear in
extends
clauses.The text was updated successfully, but these errors were encountered: