Fix type errors in iOS simulator discovery. #18960
Conversation
devoncarew
left a comment
There was a problem hiding this comment.
I noticed this while playing around with Flutter on the latest VM internally.
Cool, I was wondering how you were running flutter_tools in --preview-dart-2.
I have some pretty specific comments in-line, which closely parallels work that I have locally.
This code feels pretty loose with types in general
Yeah, this was written before strong mode at runtime was a thing. The types on the left are more an indication of what we'd want the types to be, rather than what they actually are.
There was a problem hiding this comment.
I'd change the definition of _list() to return Map<String, dynamic> instead of a raw dynamic.
There was a problem hiding this comment.
Perhaps something like:
final List<dynamic> devicesData = devicesSection[deviceCategory];
for (Map<String, dynamic> data in devicesData.map(castStringKeyedMap)) {
devices.add(new SimDevice(deviceCategory, data));
}
and castStringKeyedMap would look like:
Map<String, dynamic> castStringKeyedMap(dynamic untyped) {
final Map<dynamic, dynamic> map = untyped;
return map.cast<String, dynamic>();
}
The casting will ensure that the items in the list we iterate over are Map<String, dynamic> typed, and not Map<dynamic, dynamic>.
There was a problem hiding this comment.
I think we can avoid converting to the toString() values here and below, and assume that the values in the map are strings.
4aa63d7 to
482ac57
Compare
482ac57 to
73f46b6
Compare
|
Thanks for the review! Squashed and re-uploaded, PTAL. |
devoncarew
left a comment
There was a problem hiding this comment.
Looks great! Thanks for the contribution.
I noticed this while playing around with Flutter on the latest VM internally.
The IntelliJ plugin was silently failing to discover my iOS simulators. It was due to this code hitting runtime type errors under
2.0.0-dev.66.0:I added a test that passes without the
simulators.dartchanges when run with2.0.0-dev.63.0, and fails with2.0.0-dev.66.0.With the
simulators.dartchanges the test passes when run with either2.0.0-dev.63.0or2.0.0-dev.66.0.This is due to the way the JSON is represented here as a
Map<String, dynamic>. This code feels pretty loose with types in general and could probably be cleaned up a bit more, but I'm just looking to take the path of least resistance to getting the IntelliJ plugin's device discovery working again.