Sprinkle some mixin magic incantations #10442
Conversation
|
cc @HansMuller |
| @@ -20,7 +20,11 @@ import 'view.dart'; | |||
| export 'package:flutter/gestures.dart' show HitTestResult; | |||
|
|
|||
| /// The glue between the render tree and the Flutter engine. | |||
| abstract class RendererBinding extends BindingBase implements SchedulerBinding, ServicesBinding, HitTestable { | |||
| abstract class RendererBinding extends BindingBase with SchedulerBinding, ServicesBinding, HitTestable { | |||
There was a problem hiding this comment.
HitTestable.hitTest() is abstract, which means that reference to super.hitTest() at line 287 below ends up flagged as unresolved by DFE.
There was a problem hiding this comment.
RendererBinding itself is abstract and is a mixin. This should just resolve to whatever ends up being the superclass of RendererBinding in the concrete class, surely.
There was a problem hiding this comment.
not sure how to add Peter to this comment, he should be able to confirm this, but from what I can see DFE needs concrete instance methods(this does match what the spec says if I read it right: https://github.com/gbracha/lessRestrictedMixins/blob/master/proposal.md#16151-method-lookup)
There was a problem hiding this comment.
Yes, DFE expects to find a concrete method in the super class of the mixin with a super call. Remember that the RendererBinding is also a regular class, and when we're compiling it, we may not know yet that someone intends to use it as a mixin.
My understanding is that the language team is considering this aspect of mixins and allow you to declare intent.
There was a problem hiding this comment.
I'm not sure how to express what we're doing today within these constraints.
|
I hate to point this out, but. I think it's "mixin" not mix-in, see for example https://www.dartlang.org/articles/language/mixins The changes LGTM. |
This prevents some of our mixins from being subclassed. Also, move mixins to using 'extends' instead of 'implements' for future compatibility with Dart changes. Also, rename a class that had Mixin in the name but was not a mixin.
|
Fixed mix-in => mixin. |
|
I will land this when Travis goes green. I'll follow-up with additional patches to fix the remaining DFE problems once we know how to solve them. |
This prevents some of our mixins from being subclassed.
Also, move mixins to using 'extends' instead of 'implements' for
future compatibility with Dart changes.
Also, rename a class that had Mixin in the name but was not a mixin.