Repository navigation
extends Diagnosticable != with DiagnosticableMixin #49647
Description
Activity
/cc @jacob314
Yes it seems like a bug that we are ever specifying Diagnosticable as the type of a parameter rather than DiagnosticableMixin.
Diagnosticable does not add any APIs.Whoa, interesting. Isn't is the case that anything that mixes in
DiagnosticableMixinwould be considered to "be" aDiagnosticableMixin, so that classes that extendDiagnosticablewould also have the mixin's interface sinceDiagnosticablemixes it in? If not, then consider my mind blown. :-)What makes the inspector stop working? Does it throw an exception?
Of your three choices, I don't think you can do number 2 without removing the benefit of being able to have a mixin version of the interface, so that in
ShortcutManager's case,ChangeNotifierwould have to now implementDiagnosticable, orShortcutManagerwould have to re-implement the whole interface itself (which seems messy).I'd think number 1 is least invasive. Number 3 is probably how it should have been from the beginning though: this kind of interface lends itself to being a mixin.
Sorry, I should've used bullet points. The suggested fix is all three of them.
In order to get the correct typing behavior,
DiagnosticableMixinneeds to declare that it is implementingDiagnosticable's interface. With the code currently, this breaks at compile time because it introduces a circular dependency betweenDiagnosticableMixinandDiagnosticable. So to fix the circular dependency introduced from makingDiagnosticableMixinimplementDiagnosticable, you have to stop using it as a mixin inDiagnosticable. Then, becauseDiagnosticableis just an interface now, every place whereextends Diagnosticableis used needs to be replaced withwith DiagnosticableMixin.This is all because Dart's typing isn't structural. Currently, using
DiagnosticableMixinmeans that a class just happens to have the same exposed interface asDiagnosticable, but doesn't implement it. It's akin to:mixin Foo { void bar() => print('bar'); } abstract class Bar { void bar() => print('bar'); } class BarDerived extends Bar {} class FooDerived = Object with Foo; void main() { print(FooDerived() is Bar); // false print(BarDerived() is Foo); // false dynamic fooBar = FooDerived(); // or BarDerived(); fooBar.bar(); // Works, but they don't share a common superclass other than Object, so unsafe. }
Ahh, OK that makes a lot of sense. I was wondering how number one was going to work on its own. :-)
But wouldn't just eliminating
Diagnosticableentirely and having everything use the mixin also solve the problem (i.e. just do number 3)?Yep, I (personally) feel that referring to a mixin as a type is kinda weird, but that works too.
Here's a reproduction snippet
import 'package:flutter/widgets.dart'; void main() { try { print(Shortcuts(manager: ShortcutManager())); } catch (ex, stack) { print(ex); print(stack); } }
Ahh. I was wondering why this wasn't tweaking tests, but I guess I don't have a test that prints the diagnostics of a
Shortcutswidget that has amanagerset.So as long as you don't set a manager, then https://github.com/flutter/flutter/blob/master/packages/flutter/lib/src/widgets/shortcuts.dart#L371 doesn't try to do anything with the manager, so it doesn't fail.
So, @ds84182 are you working on a fix, or should I take a look at fixing it?
I'm not working on a fix right now
- added a commit that references this issue
on Jan 30, 2020 I did upgrade to the libraries(pubspec.yaml) and it function.
Reacted by jimmysaltoThis thread has been automatically locked since there has not been any recent activity after it was closed. If you are still experiencing a similar issue, please open a new bug, including the output of
flutter doctor -vand a minimal reproduction of the issue.- locked as resolved and limited conversation to collaborators
on Aug 18, 2021
In many places, Diagnosticable is refered to directly instead of the mixin itself. This has severe implications when it comes to class hierarchies, because classes that use the mixin are incompatible with the Diagnosticable subsystem and will instead cause TypeErrors.
As a suggested fix:
implements DiagnosticabletoDiagnosticableMixinDiagnosticableto stop usingDiagnosticableMixin(and treat it as an interface, copy the signatures from the methods exposed fromDiagnosticableMixin)with DiagnosticableMixininstead ofextends DiagnosticableThis is, unfortunately, a breaking change for anyone using
extends Diagnosticable.Right now, the Flutter Inspector stops working when a
ShortcutManageris specified in aShortcutswidget.cc @gspencergoog @goderbauer