[flutter_tools] Fix type error in ChromiumDevice.startApp #111935
Conversation
| @override | ||
| Future<LaunchResult> startApp( | ||
| covariant WebApplicationPackage package, { | ||
| WebApplicationPackage? package, { |
There was a problem hiding this comment.
not directly relevant to the fix, but this type doesn't need to be covariant, as sub-classes will use the exact same type
There was a problem hiding this comment.
Is this true? Or is it because it has been labeled covariant in the base class definition here:
flutter/packages/flutter_tools/lib/src/device.dart
Lines 591 to 604 in b70ff4f
There was a problem hiding this comment.
Is what true? That the type doesn't need to be covariant? Or that it's not relevant to the fix?
There was a problem hiding this comment.
Is it true that a subtyped class can be passed as a valid parameter to a method.
Meaning I don't think because WebApplicationPackage is a subtype of ApplicationPackage that we can pass it as a valid type to the startApp method. But it works because we marked the type on super class a covariant
There was a problem hiding this comment.
We sync'd offline. To clarify my original statement, I can remove the covariant from the ChromiumDevice.startApp implementation because the super class has it marked covariant. TIL that covariant can be placed in either the superclass or subclass: https://dart.dev/guides/language/sound-problems#the-covariant-keyword
|
LGTM |
Fixes #111877
The issue was that the method declaration in the super class (
Device.startApp) specifiescovariant ApplicationPackage?, while the sub-class (ChromiumDevice.startApp) declares itcovariant WebApplicationPackage, which is non-nullable. However, because of #55531, it is possible to run a Flutter app on a browser device even if the project does not specify a web target; thus, package could legitimately be null. However,ChromiumDevice.startAppspecifies a non-nullable parameter, it is a runtime error. This was likely introduced during a null-safety migration, so this restores this functionality by making the parameter nullable (note thatChromiumDevice.startAppdoes not even reference the argument).