Skip to content
This repository has been archived by the owner on Feb 22, 2023. It is now read-only.

[google_maps_flutter] Migrate to NNBD #3548

Merged
merged 23 commits into from
Feb 19, 2021
Merged

[google_maps_flutter] Migrate to NNBD #3548

Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
8ad7f8f
Deps and version updates
stuartmorgan Feb 10, 2021
74af2db
Initial cleanup and annotation
stuartmorgan Feb 10, 2021
31f3487
Initial apply
stuartmorgan Feb 10, 2021
675c88f
Replace Mockito and get the tests passing
stuartmorgan Feb 11, 2021
40b9700
Fix from migrating example
stuartmorgan Feb 11, 2021
0afb179
annotations
stuartmorgan Feb 11, 2021
f7a1a66
Initial migration
stuartmorgan Feb 11, 2021
54663a1
Compilation fix
stuartmorgan Feb 11, 2021
70f47f7
Adjust for last review changes to interface
stuartmorgan Feb 14, 2021
f08ab99
Point to published interface
stuartmorgan Feb 15, 2021
7c0a00c
Minor cleanup
stuartmorgan Feb 15, 2021
55dcb8a
Revert bogus tool change
stuartmorgan Feb 15, 2021
9afdc93
Remove maps from non-NNBD list
stuartmorgan Feb 15, 2021
b60eee9
Autoformat
stuartmorgan Feb 15, 2021
b5d4514
CHANGELOG fix
stuartmorgan Feb 15, 2021
05089e8
Merge branch 'master' into maps-nnbd-appfacing
stuartmorgan Feb 16, 2021
8dc2ae2
Pick up deserialization fix
stuartmorgan Feb 16, 2021
99c36f3
Don't build maps web with NNBD plugins
stuartmorgan Feb 16, 2021
ede5244
Merge branch 'master' into maps-nnbd-appfacing
stuartmorgan Feb 16, 2021
6bc0a2f
Fix incorrect 'late's
stuartmorgan Feb 16, 2021
41963c8
Review fixes
stuartmorgan Feb 17, 2021
a5598a9
Address final review feedback
stuartmorgan Feb 17, 2021
77a1348
Remove pointless assert
stuartmorgan Feb 17, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions packages/google_maps_flutter/google_maps_flutter/CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,3 +1,11 @@
## 2.0.0-nullsafety

* Migrate to null-safety
* BREAKING CHANGE: Passing an unknown map object ID (e.g., MarkerId) to a
method, it will throw an `UnknownMapObjectIDError`. Previously it would
either silently do nothing, or throw an error trying to call a function on
`null`, depneding on the method.

## 1.2.0

* Support custom tiles.
Expand Down
1 change: 1 addition & 0 deletions ...oogle_maps_flutter/google_maps_flutter/example/integration_test/google_map_inspector.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright 2019, the Chromium project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// @dart=2.9

import 'dart:typed_data';
import 'package:flutter/services.dart';
Expand Down
7 changes: 3 additions & 4 deletions ...es/google_maps_flutter/google_maps_flutter/example/integration_test/google_maps_test.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// Copyright 2019, the Chromium project authors. Please see the AUTHORS file
// for details. All rights reserved. Use of this source code is governed by a
// BSD-style license that can be found in the LICENSE file.
// @dart=2.9

import 'dart:async';
import 'dart:io';
Expand Down Expand Up @@ -954,10 +955,8 @@ void main() {
final BitmapDescriptor scaled = await BitmapDescriptor.fromAssetImage(
imageConfiguration, 'red_square.png',
mipmaps: false);
// ignore: invalid_use_of_visible_for_testing_member
expect(mip.toJson()[2], 1);
// ignore: invalid_use_of_visible_for_testing_member
expect(scaled.toJson()[2], 2);
expect((mip.toJson() as List<dynamic>)[2], 1);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: assert size before reading the 2nd element

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While updating these I decided it wasn't worth adding complexity to the test code to assert incremental steps; if toJson returns a one-element array this test will fail with an out-of-bounds index exception, which gives us the same information than a length check would give us.

(The bigger problem is that this is testing code from the interface package; this test doesn't belong here at all. But moving it was out of scope for this change.)

expect((scaled.toJson() as List<dynamic>)[2], 2);
});

testWidgets('testTakeSnapshot', (WidgetTester tester) async {
Expand Down
22 changes: 11 additions & 11 deletions packages/google_maps_flutter/google_maps_flutter/example/lib/animate_camera.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ class AnimateCamera extends StatefulWidget {
}

class AnimateCameraState extends State<AnimateCamera> {
GoogleMapController mapController;
GoogleMapController? mapController;

void _onMapCreated(GoogleMapController controller) {
mapController = controller;
Expand Down Expand Up @@ -56,7 +56,7 @@ class AnimateCameraState extends State<AnimateCamera> {
children: <Widget>[
TextButton(
onPressed: () {
mapController.animateCamera(
mapController?.animateCamera(
CameraUpdate.newCameraPosition(
const CameraPosition(
bearing: 270.0,
Expand All @@ -71,7 +71,7 @@ class AnimateCameraState extends State<AnimateCamera> {
),
TextButton(
onPressed: () {
mapController.animateCamera(
mapController?.animateCamera(
CameraUpdate.newLatLng(
const LatLng(56.1725505, 10.1850512),
),
Expand All @@ -81,7 +81,7 @@ class AnimateCameraState extends State<AnimateCamera> {
),
TextButton(
onPressed: () {
mapController.animateCamera(
mapController?.animateCamera(
CameraUpdate.newLatLngBounds(
LatLngBounds(
southwest: const LatLng(-38.483935, 113.248673),
Expand All @@ -95,7 +95,7 @@ class AnimateCameraState extends State<AnimateCamera> {
),
TextButton(
onPressed: () {
mapController.animateCamera(
mapController?.animateCamera(
CameraUpdate.newLatLngZoom(
const LatLng(37.4231613, -122.087159),
11.0,
Expand All @@ -106,7 +106,7 @@ class AnimateCameraState extends State<AnimateCamera> {
),
TextButton(
onPressed: () {
mapController.animateCamera(
mapController?.animateCamera(
CameraUpdate.scrollBy(150.0, -225.0),
);
},
Expand All @@ -118,7 +118,7 @@ class AnimateCameraState extends State<AnimateCamera> {
children: <Widget>[
TextButton(
onPressed: () {
mapController.animateCamera(
mapController?.animateCamera(
CameraUpdate.zoomBy(
-0.5,
const Offset(30.0, 20.0),
Expand All @@ -129,31 +129,31 @@ class AnimateCameraState extends State<AnimateCamera> {
),
TextButton(
onPressed: () {
mapController.animateCamera(
mapController?.animateCamera(
CameraUpdate.zoomBy(-0.5),
);
},
child: const Text('zoomBy'),
),
TextButton(
onPressed: () {
mapController.animateCamera(
mapController?.animateCamera(
CameraUpdate.zoomIn(),
);
},
child: const Text('zoomIn'),
),
TextButton(
onPressed: () {
mapController.animateCamera(
mapController?.animateCamera(
CameraUpdate.zoomOut(),
);
},
child: const Text('zoomOut'),
),
TextButton(
onPressed: () {
mapController.animateCamera(
mapController?.animateCamera(
CameraUpdate.zoomTo(16.0),
);
},
Expand Down
6 changes: 3 additions & 3 deletions packages/google_maps_flutter/google_maps_flutter/example/lib/map_click.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,9 +31,9 @@ class _MapClickBody extends StatefulWidget {
class _MapClickBodyState extends State<_MapClickBody> {
_MapClickBodyState();

GoogleMapController mapController;
LatLng _lastTap;
LatLng _lastLongPress;
GoogleMapController? mapController;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this be late similar to animate_camera.dart?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

build kicks off async maps creation, and also uses mapController if present, so it does need to be nullable.

I went back and took a closer look at the late cases, and all but one were actually wrong, and had a (latent) bug where there was a race condition where someone could potentially press a control before the map controller was actually created. (I was originally fooled because one of them has a boolean for whether creation has completed and gates showing the controls on that boolean, so late is correct, and I missed that others that used the controller unconditionally in callbacks didn't have the same structure.)

I think it's not ideal that there are at least three different patterns for handling the async construction in these example files, but don't want to make changing that part of nullability migration.

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ah I see. Yeah, I didn't have that context. Thanks

LatLng? _lastTap;
LatLng? _lastLongPress;

@override
Widget build(BuildContext context) {
Expand Down
4 changes: 2 additions & 2 deletions packages/google_maps_flutter/google_maps_flutter/example/lib/map_coordinates.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ class _MapCoordinatesBody extends StatefulWidget {
class _MapCoordinatesBodyState extends State<_MapCoordinatesBody> {
_MapCoordinatesBodyState();

GoogleMapController mapController;
GoogleMapController? mapController;
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto: in some cases, late is used

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See other comment; this is correct.

LatLngBounds _visibleRegion = LatLngBounds(
southwest: const LatLng(0, 0),
northeast: const LatLng(0, 0),
Expand Down Expand Up @@ -87,7 +87,7 @@ class _MapCoordinatesBodyState extends State<_MapCoordinatesBody> {
child: const Text('Get Visible Region Bounds'),
onPressed: () async {
final LatLngBounds visibleRegion =
await mapController.getVisibleRegion();
await mapController!.getVisibleRegion();
setState(() {
_visibleRegion = visibleRegion;
});
Expand Down
7 changes: 3 additions & 4 deletions packages/google_maps_flutter/google_maps_flutter/example/lib/map_ui.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ class MapUiBodyState extends State<MapUiBody> {
bool _myLocationEnabled = true;
bool _myTrafficEnabled = false;
bool _myLocationButtonEnabled = true;
GoogleMapController _controller;
late GoogleMapController _controller;
bool _nightMode = false;

@override
Expand Down Expand Up @@ -249,10 +249,9 @@ class MapUiBodyState extends State<MapUiBody> {
});
}

// Should only be called if _isMapCreated is true.
Widget _nightModeToggler() {
if (!_isMapCreated) {
return null;
}
assert(_isMapCreated);
return TextButton(
child: Text('${_nightMode ? 'disable' : 'enable'} night mode'),
onPressed: () {
Expand Down
26 changes: 14 additions & 12 deletions packages/google_maps_flutter/google_maps_flutter/example/lib/marker_icons.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ class MarkerIconsBody extends StatefulWidget {
const LatLng _kMapCenter = LatLng(52.4478, -3.5402);

class MarkerIconsBodyState extends State<MarkerIconsBody> {
GoogleMapController controller;
BitmapDescriptor _markerIcon;
GoogleMapController? controller;
BitmapDescriptor? _markerIcon;
Comment on lines +32 to +33
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ditto

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There are flows in the existing logic that assume _markerIcon can be null.


@override
Widget build(BuildContext context) {
Expand All @@ -48,7 +48,7 @@ class MarkerIconsBodyState extends State<MarkerIconsBody> {
target: _kMapCenter,
zoom: 7.0,
),
markers: _createMarker(),
markers: <Marker>{_createMarker()},
onMapCreated: _onMapCreated,
),
),
Expand All @@ -57,17 +57,19 @@ class MarkerIconsBodyState extends State<MarkerIconsBody> {
);
}

Set<Marker> _createMarker() {
// TODO(iskakaushik): Remove this when collection literals makes it to stable.
// https://github.com/flutter/flutter/issues/28312
// ignore: prefer_collection_literals
return <Marker>[
Marker(
Marker _createMarker() {
if (_markerIcon != null) {
return Marker(
markerId: MarkerId("marker_1"),
position: _kMapCenter,
icon: _markerIcon,
),
].toSet();
icon: _markerIcon!,
);
} else {
return Marker(
markerId: MarkerId("marker_1"),
position: _kMapCenter,
);
}
}

Future<void> _createMarkerImageFromAsset(BuildContext context) async {
Expand Down
22 changes: 11 additions & 11 deletions packages/google_maps_flutter/google_maps_flutter/example/lib/move_camera.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class MoveCamera extends StatefulWidget {
}

class MoveCameraState extends State<MoveCamera> {
GoogleMapController mapController;
GoogleMapController? mapController;

void _onMapCreated(GoogleMapController controller) {
mapController = controller;
Expand Down Expand Up @@ -55,7 +55,7 @@ class MoveCameraState extends State<MoveCamera> {
children: <Widget>[
TextButton(
onPressed: () {
mapController.moveCamera(
mapController?.moveCamera(
CameraUpdate.newCameraPosition(
const CameraPosition(
bearing: 270.0,
Expand All @@ -70,7 +70,7 @@ class MoveCameraState extends State<MoveCamera> {
),
TextButton(
onPressed: () {
mapController.moveCamera(
mapController?.moveCamera(
CameraUpdate.newLatLng(
const LatLng(56.1725505, 10.1850512),
),
Expand All @@ -80,7 +80,7 @@ class MoveCameraState extends State<MoveCamera> {
),
TextButton(
onPressed: () {
mapController.moveCamera(
mapController?.moveCamera(
CameraUpdate.newLatLngBounds(
LatLngBounds(
southwest: const LatLng(-38.483935, 113.248673),
Expand All @@ -94,7 +94,7 @@ class MoveCameraState extends State<MoveCamera> {
),
TextButton(
onPressed: () {
mapController.moveCamera(
mapController?.moveCamera(
CameraUpdate.newLatLngZoom(
const LatLng(37.4231613, -122.087159),
11.0,
Expand All @@ -105,7 +105,7 @@ class MoveCameraState extends State<MoveCamera> {
),
TextButton(
onPressed: () {
mapController.moveCamera(
mapController?.moveCamera(
CameraUpdate.scrollBy(150.0, -225.0),
);
},
Expand All @@ -117,7 +117,7 @@ class MoveCameraState extends State<MoveCamera> {
children: <Widget>[
TextButton(
onPressed: () {
mapController.moveCamera(
mapController?.moveCamera(
CameraUpdate.zoomBy(
-0.5,
const Offset(30.0, 20.0),
Expand All @@ -128,31 +128,31 @@ class MoveCameraState extends State<MoveCamera> {
),
TextButton(
onPressed: () {
mapController.moveCamera(
mapController?.moveCamera(
CameraUpdate.zoomBy(-0.5),
);
},
child: const Text('zoomBy'),
),
TextButton(
onPressed: () {
mapController.moveCamera(
mapController?.moveCamera(
CameraUpdate.zoomIn(),
);
},
child: const Text('zoomIn'),
),
TextButton(
onPressed: () {
mapController.moveCamera(
mapController?.moveCamera(
CameraUpdate.zoomOut(),
);
},
child: const Text('zoomOut'),
),
TextButton(
onPressed: () {
mapController.moveCamera(
mapController?.moveCamera(
CameraUpdate.zoomTo(16.0),
);
},
Expand Down
10 changes: 5 additions & 5 deletions packages/google_maps_flutter/google_maps_flutter/example/lib/padding.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class MarkerIconsBody extends StatefulWidget {
const LatLng _kMapCenter = LatLng(52.4478, -3.5402);

class MarkerIconsBodyState extends State<MarkerIconsBody> {
GoogleMapController controller;
GoogleMapController? controller;

EdgeInsets _padding = const EdgeInsets.all(0);

Expand Down Expand Up @@ -152,10 +152,10 @@ class MarkerIconsBodyState extends State<MarkerIconsBody> {
onPressed: () {
setState(() {
_padding = EdgeInsets.fromLTRB(
double.tryParse(_leftController.value?.text) ?? 0,
double.tryParse(_topController.value?.text) ?? 0,
double.tryParse(_rightController.value?.text) ?? 0,
double.tryParse(_bottomController.value?.text) ?? 0);
double.tryParse(_leftController.value.text) ?? 0,
double.tryParse(_topController.value.text) ?? 0,
double.tryParse(_rightController.value.text) ?? 0,
double.tryParse(_bottomController.value.text) ?? 0);
});
},
),
Expand Down
Loading