[google_maps_flutter_platform_interface] Convert BitmapDescriptor to typesafe subclasses#7699
Conversation
There was a problem hiding this comment.
Since the bitmaps are no longer just wrapping JSON, toJson will not return the same reference as was passed to fromJson, though they will still be equal by comparison.
There was a problem hiding this comment.
I thought this was a breaking change but a spot check of the public usages in github show I am incorrect.
There was a problem hiding this comment.
The only ways to construct a BitmapDescriptor without constructing one of its subclasses was either through this private _ factory, which is removed here, or the below fromJson public factory, which this PR converts to a static method, so unless we are expecting public usages of this package to add new subclasses of BitmapDescriptor, I believe that neither making it abstract nor removing the _ factory should impact any public uses.
There was a problem hiding this comment.
Why not use the : _json syntax?
There was a problem hiding this comment.
This PR removes the _json field from BitmapDescriptor, and since it also makes BitmapDescriptor an abstract class with a concrete subtype for each possible set of member data, fromJson needs to return an instance of one of those subtypes.
There was a problem hiding this comment.
This should be private so we're not increasing the API surface of the package.
There was a problem hiding this comment.
Would you prefer we mark it @visibleForTesting, or remove the test https://github.com/flutter/packages/pull/7699/files#diff-a8066076d538f1e1e21b27bac466302cb9317b643500ce81b991102a94611070R685?
There was a problem hiding this comment.
My preference would be visible for testing and keep the test.
There was a problem hiding this comment.
That would be my thought as well, @stuartmorgan do you agree with this decision?
There was a problem hiding this comment.
Yes, that's fine; from a semver standpoint we consider those equivalent.
There was a problem hiding this comment.
Please use named parameters so that we can change the parameter set in the future without breaking changes (or being stuck with a growing positional list, which doesn't scale).
stuartmorgan-g
left a comment
stuartmorgan-g
left a comment
There was a problem hiding this comment.
LG once the @visibleForTesting annotation is added.
yaakovschectman
force-pushed
the
maps_bmd
branch
from
1ff2c26 to
cdcae66
Compare
| import 'dart:async' show Future; | ||
| import 'dart:typed_data' show Uint8List; | ||
|
|
||
| import 'package:flutter/cupertino.dart'; |
There was a problem hiding this comment.
Looks like your IDE chose a weird auto-import here; this should be a new show below instead.
There was a problem hiding this comment.
(Specifically, from foundation.dart)
…iptor` to typesafe subclasses (flutter/packages#7699)
flutter/packages@05bf1d4...bb00d34 2024-10-07 [email protected] [google_sign_in] Update Pigeon for non-nullable generics (flutter/packages#7785) 2024-10-07 [email protected] [path_provider] Update Android Pigeon for non-nullable generics (flutter/packages#7783) 2024-10-07 [email protected] [rfw] Increase tolerance for material widget tests (flutter/packages#7148) 2024-10-05 [email protected] [various] Update Java compatibility version to 11 (flutter/packages#7795) 2024-10-04 [email protected] [video_player] Update Pigeon for non-nullable generics (flutter/packages#7790) 2024-10-04 [email protected] [go_router] Added missing implementation for the routerNeglect parameter in GoRouter (flutter/packages#7752) 2024-10-04 [email protected] [google_maps_flutter_platform_interface] Convert `BitmapDescriptor` to typesafe subclasses (flutter/packages#7699) 2024-10-04 [email protected] [google_maps_flutter_platform_interface] Convert `PatternItem` and `Cap` to typesafe structures. (flutter/packages#7703) If this roll has caused a breakage, revert this CL and stop the roller using the controls here: https://autoroll.skia.org/r/flutter-packages-flutter-autoroll Please CC [email protected] on the revert to ensure that a human is aware of the problem. To file a bug in Flutter: https://github.com/flutter/flutter/issues/new/choose To report a problem with the AutoRoller itself, please file a bug: https://issues.skia.org/issues/new?component=1389291&template=1850622 Documentation for the AutoRoller is here: https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
…o typesafe subclasses (flutter#7699) `BitmapDescriptor` is currently just a wrapper around JSON, with the exception of two of its subclasses. This converts all cases to typesafe structures without breaking the current API. Part of flutter/flutter#155122 ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] page, which explains my responsibilities. - [x] I read and followed the [relevant style guides] and ran the auto-formatter. (Unlike the flutter/flutter repo, the flutter/packages repo does use `dart format`.) - [ ] I signed the [CLA]. - [x] The title of the PR starts with the name of the package surrounded by square brackets, e.g. `[shared_preferences]` - [x] I [linked to at least one issue that this PR fixes] in the description above. - [x] I updated `pubspec.yaml` with an appropriate new version according to the [pub versioning philosophy], or this PR is [exempt from version changes]. - [x] I updated `CHANGELOG.md` to add a description of the change, [following repository CHANGELOG style], or this PR is [exempt from CHANGELOG changes]. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. <!-- Links --> [Contributor Guide]: https://github.com/flutter/packages/blob/main/CONTRIBUTING.md [Tree Hygiene]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md [relevant style guides]: https://github.com/flutter/packages/blob/main/CONTRIBUTING.md#style [CLA]: https://cla.developers.google.com/ [Discord]: https://github.com/flutter/flutter/blob/master/docs/contributing/Chat.md [linked to at least one issue that this PR fixes]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#overview [pub versioning philosophy]: https://dart.dev/tools/pub/versioning [exempt from version changes]: https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#version [following repository CHANGELOG style]: https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changelog-style [exempt from CHANGELOG changes]: https://github.com/flutter/flutter/blob/master/docs/ecosystem/contributing/README.md#changelog [test-exempt]: https://github.com/flutter/flutter/blob/master/docs/contributing/Tree-hygiene.md#tests
3 participants
BitmapDescriptoris currently just a wrapper around JSON, with the exception of two of its subclasses. This converts all cases to typesafe structures without breaking the current API.Part of flutter/flutter#155122
Pre-launch Checklist
dart format.)[shared_preferences]pubspec.yamlwith an appropriate new version according to the pub versioning philosophy, or this PR is exempt from version changes.CHANGELOG.mdto add a description of the change, following repository CHANGELOG style, or this PR is exempt from CHANGELOG changes.///).If you need help, consider asking for advice on the #hackers-new channel on Discord.