packages icon indicating copy to clipboard operation
packages copied to clipboard

[google_maps_flutter] Fix Circle update regression

Open stuartmorgan-g opened this issue 1 year ago • 0 comments

Fixes a regression introduced in the last PR that did shallow Pigeon conversions of the maps calls, where the new Pigeon wrapper object was being passed directly to one of the not-yet-converted methods that was expecting the JSON data (i.e., circle.getJson() rather than circle). Also adds a native unit test exercising that codepath, which reproduced the cast error.

As a secondary step to improve safety is this hybrid state, for all of the map object controllers, I converted most of the methods taking Objects to taking Map<String, ?> instead, which is in practice what the JSON serialization of all of those objects is, and further pushed that through the Pigeon layer so that instead of casting throughout the Java code, the cast happens once on the Dart side. While the Dart cast is technically unsafe:

  • It's the same level of unsafe as the previous situation, just centralized in a small amount of Dart code instead of spread through the Java code.
  • In practice, the JSON serialization in the platform interface package, which is what this cast and all the previous Java code was making assumptions about, can never change, because it is a de-facto API surface. (The fact that we are relying on the JSON serialization code from what is now a different package was a mistake during federation, which has existing TODOs to fix; in practice the fix will almost certainly be to finish the Pigeon migration rather than to copy the JSON serialization.)

Part of https://github.com/flutter/flutter/issues/117907

Pre-launch Checklist

stuartmorgan-g avatar Jul 03 '24 18:07 stuartmorgan-g