Skip to content

4.0.0: full audit — close all open issues, supersede all open PRs, 160/160 pub score - #73

Merged
itsarvinddev merged 15 commits into
masterfrom
feat/v4-audit-overhaul
Sep 12, 2026
Merged

itsarvinddev merged 15 commits into
masterfrom
feat/v4-audit-overhaul

Conversation

@itsarvinddev

Copy link
Copy Markdown
Owner

Full audit and 4.0.0 release. Closes every open issue, merges or supersedes every open PR with credit, brings the dependency stack current, and fixes the parts of the package that were quietly broken.

pub.dev score: 120/160 → 160/160. Tests: 0 → 54. flutter analyze --fatal-infos clean; web JS and WebAssembly both build.


Issues closed

Closes #70searchFilter replaced the whole autocomplete request, so the typed text never reached Google. Filters are now merged.
Closes #68 — build failure from google_maps_apis 4.0.0 vs retrofit ≥ 4.9.0. Fixed by moving to 5.1 and deleting the retrofit pin that caused it.
Closes #65 — the picker always returned its own Scaffold, so it could not be embedded. New MapLocationPickerView has none.
Closes #14 — web search and place details now work, with no proxy.

Pull requests

PR Disposition
#67 (@vkourtis) Superseded. The diagnosis was right, but the file it patches (lib/src/autocomplete_service.dart) had been orphaned by commit b55ba58 — nothing imported it, so the fix would never have executed. Fixed in the live file, plus the session-token clobber the PR would have introduced.
#71 (@CarmeloBeeapp) Superseded. The #68 diagnosis was correct and is taken. Not taken: google_maps_flutter: ^2.17.0 (requires Flutter 3.38 while the pubspec still claimed 3.27 — unresolvable for everyone in between), and commenting out hideWithKeyboard, which silently turned a documented option into a no-op. It is now @Deprecated pointing at hideOnUnfocus.
#72 (@demon36) Taken, rewritten. Renamed to onMainMarkerPositionChanged and retyped ValueChanged<LatLng> (the value could never be null). Moved out of _createMarkers, a pure builder, and paired with each position assignment so it fires on drag, tap, current-location and suggestion alike. The dart format hunk is kept as its own commit.
#66 (@Elbarae1921) Taken, corrected. Colons must be stripped, but Google documents Base16 — case does not matter. The surrounding snippet also passed geoCodingApiHeaders, which is not a parameter. Both fixed in the README rewrite. Credit also to @dend456, whose comment the PR cites.

Two findings that reversed earlier assumptions

The web CORS problem no longer exists. Issue #14 is from 2023 and was filed against the legacy /maps/api/place/... endpoints. Probing the current ones:

OPTIONS https://places.googleapis.com/v1/places:autocomplete   → 200
  access-control-allow-headers: content-type,x-goog-api-key,x-goog-fieldmask
OPTIONS https://places.googleapis.com/v1/places/{placeId}       → 200
  access-control-allow-headers: x-goog-api-key,x-goog-fieldmask
GET     https://maps.googleapis.com/maps/api/geocode/json       → 200
  access-control-allow-origin: *

google_maps_apis sends the key as x-goog-api-key, which is in the allow-list. So the REST transport works in a browser, and the JS-interop shim was a net negative: it forced web onto google.maps.places.AutocompleteService, closed to new Google Cloud customers since 1 March 2025, dropped filter/sessionToken/fields/cancelToken, and never covered getDetails at all — so on web you got suggestions and then nothing happened when you tapped one. Deleted. One service, one transport, every platform.

The SDK floor is 3.38, not 3.32. google_maps_apis 5.x needs meta ^1.17.0, and the Flutter SDK pins meta exactly:

Flutter pinned meta
3.32 – 3.37 1.16.0
3.38+ 1.17.0

So 3.32–3.37 cannot resolve this package at all. This is invisible when testing on 3.47, which is why CI now pins the floor explicitly and runs pub downgrade against it.


Bugs fixed

  • The "my location" button did nothing the first time permission was granted. The guard read permission != whileInUse || permission != always — no enum value can fail that, so it always returned immediately after the OS dialog.
  • Two crash sites in the bottom card and nearby sheet: .substring(0, 1) on an empty string and .first on an empty list, both reachable from ordinary geocoding responses.
  • Overlapping lookups returned the wrong address. Two quick taps on a slow link could leave the older response overwriting the newer one — so Confirm returned a location the user never picked.
  • Every keystroke was billed as its own Places session. A fresh session token per request, never passed to the details call.
  • Confirm was a dead filled button whenever geocoding failed. It now returns the raw coordinate.
  • Supplying a searchConfig silently discarded config.apiKey.
  • Awaiting the map controller had no timeout — a missing API key pinned the spinner forever.
  • Tapping a nearby entry fired onNext and popped twice.
  • Async continuations wrote disposed state; http.Client leaked per request; additionalMarkers['main'] collided with the built-in id; LatLng(0,0) was an "unset" sentinel; the Google logo was covered, contrary to the Maps Platform terms.
  • Web overlays were unclickable. flutter_typeahead 6 dropped the transitive pointer_interceptor; it is now explicit.
  • The example could not compile from a clean clone (key.dart was gitignored) and declared an unsatisfiable SDK range.

Added

PickerPinMode.centerPin · MapLocationPickerController · showMapLocationPicker() returning PickedPlace · MapLocationPickerStrings (all 17 strings, translatable) · typed MapLocationPickerException + onError · nearby places on the Places API (New) · countries/placeTypes search restriction (the README documented components for years; it never existed) · startWithCurrentLocation with timeout and last-known fallback · back button · draggable marker with onMainMarkerPositionChanged · GeocodingResultParts accessors · configurable FAB hero tags · log level and sink.

Docs

README rewritten — every example in the 3.x one referenced types renamed two releases earlier. All samples are now compiled by example/lib/readme_samples.dart, and the public API surface by example/lib/api_surface_check.dart, so docs and exports cannot drift silently. Added MIGRATION_GUIDE.md 3.x→4.0.0 and CONTRIBUTING.md.


Review notes

  • The barrel now exports far less. It used to re-export all of geolocator, google_maps_flutter and both google_maps_apis libraries — hundreds of symbols and nine collisions, including two LatLng. This is the most likely source of breakage for existing users; the migration guide covers it.
  • MapLocationPickerConfig deliberately keeps its flat shape. Grouping the fields would block every fix behind a mechanical rename in a release that already carries a Flutter floor bump.
  • cloudMapId stays (with an ignore) rather than moving to mapId, because mapId does not exist below google_maps_flutter 2.15 and the range reaches back to 2.13.1.
  • Desktop is not supported and now says so — google_maps_flutter has no macOS/Windows/Linux implementation.

Left for you: closing #67/#71/#66 as superseded and thanking the authors, since those are your call.

🤖 Generated with Claude Code

itsarvinddev and others added 9 commits September 9, 2026 10:17
… clean clone

The example never compiled from a fresh clone: `example/lib/key.dart` was
gitignored and pubignored, so `flutter analyze` failed with 9 errors on
`package:example/key.dart`. Commit a placeholder key file instead, matching
how `YOUR_API_KEY` is already committed in AndroidManifest.xml and index.html.

Also fixes `example/pubspec.yaml`, which declared an unsatisfiable
`sdk: ">=2.17.5 <3.0.0"` (the example is a Dart 3 app).

The new workflow runs analyze, format, generated-code drift detection, tests,
Android/iOS/web/wasm example builds, `pub publish --dry-run` and pana.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…sed deps

Fixes #68. google_maps_apis 4.0.0's CustomParseErrorLogger.logError takes three
positional arguments, but retrofit >=4.9.0 widened ParseErrorLogger.logError to
four, so any app that resolved a newer retrofit failed to compile. google_maps_apis
5.0.0 fixed the signature; the floor is set to 5.1.0 so exactly one copyWith
generator semantic is possible (5.0.0 still used copy_with_extension's CWProxy,
where an explicit null nullifies a field, while 5.1.0 uses adeptgenerator's
`??` extension). 5.1.0 also carries the upstream ResponseStatus parsing fix.

Removes `retrofit` and `web` as direct dependencies -- neither is imported
anywhere in lib/. The `retrofit: ">=4.7.3 <4.9.0"` pin was the direct cause of
#68 and also blocked consumers from adopting newer retrofit.

Dependency constraints are written as wide ranges rather than caret pins so
apps on older Flutter still resolve a compatible google_maps_flutter; pub picks
2.18.0 on Flutter 3.38+ and an older 2.x below that. PR #71 proposed
`google_maps_flutter: ^2.17.0`, which requires Dart 3.10 / Flutter 3.38 and
would have been unresolvable for every user below 3.38 while the pubspec still
claimed `flutter: ">=3.27.0"`.

The declared SDK floor moves to Dart 3.8 / Flutter 3.32. The previous
`>=3.6.0` / `>=3.27.0` was already unsatisfiable: google_maps_apis 4.x
requires Dart 3.8.

flutter_typeahead 6.0.0 removed `hideWithKeyboard` (closing the keyboard also
drops focus, which `hideOnUnfocus` already covers). Rather than silently
commenting out the call site as PR #71 did, `SearchConfig.hideWithKeyboard` is
now @deprecated with a pointer to `hideOnUnfocus`, and the new
`SearchConfig.constrainWidth` from typeahead 6 is exposed.

Supersedes #71. Thanks to @CarmeloBeeapp for the diagnosis and to @jansvanda
for the report.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Mechanical only, no behaviour change. Raising environment.sdk to >=3.8.0 in the
previous commit switches the formatter to tall style, so this lands as one
format-only commit to keep later feature diffs readable. Also absorbs the
indentation PR #72 corrected in map_location_picker.dart.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
… sessions

Fixes #70, #14. Supersedes #67.

Three copies of `AutoCompleteService` existed: `lib/src/autocomplete_service.dart`
plus `autocomplete/autocomplete_service_io.dart` and `_stub.dart`. The top-level
copy was orphaned by b55ba58 -- nothing imports or exports it -- which is why
PR #67's fix for #70 would never have executed. It is deleted, `_stub.dart` now
just re-exports the REST implementation, and there is exactly one place to fix.

Issue #70: the request was built as `filter ?? AutocompleteSearchFilter(input:
query, ...)`, so any caller-supplied `searchFilter` replaced the request wholesale
and Google never received the typed text. It is now merged with `copyWith`, and
a `sessionToken` the caller set on their own filter is preserved rather than
overwritten. Covered by test/autocomplete_filter_test.dart.

Issue #14 (web): `places.googleapis.com` sends no CORS headers, so the REST
transport cannot work in a browser. The web implementation is rewritten to use
`google.maps.places.AutocompleteSuggestion.fetchAutocompleteSuggestions()` and
`Place.fetchFields()`. The previous implementation used
`google.maps.places.AutocompleteService`, which Google closed to new customers
on 1 March 2025 -- so it was not merely suboptimal, it was unusable for any
project created after that date. Place *details* now also route through the
service, so the details call no longer hits the CORS-blocked REST endpoint on
web. The web path forwards languageCode, regionCode, includedRegionCodes,
includedPrimaryTypes, inputOffset and origin, and logs a warning for the two
filter fields it cannot forward instead of dropping them silently.

Billing: a fresh `SessionTokenHandler` was minted per request, so every keystroke
opened and abandoned its own Places session and was billed on the per-request
SKU. One handler is now hoisted into `PlacesAutocomplete` and threaded through
both the autocomplete calls and the details call, which concludes the session.

Errors are no longer swallowed. `MapLocationPickerException` /
`MapPickerErrorKind` classify network, cancelled, invalidRequest, requestDenied,
quotaExceeded, noResults, location and platform failures, and are reported via
the new `onError` callback -- previously an invalid API key, an exceeded quota
and "no matches here" were all an empty list.

Also: selecting a suggestion now writes the selected text into the field (it
previously only moved the caret); a caller-supplied CancelToken is no longer
reused across requests, where a single cancellation killed autocomplete
permanently; and the service is rebuilt when the API key changes.

Thanks to @vkourtis for #67 and @CaptainRiley for the #70 root-cause analysis,
and to @Bylinkk for #14.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Fixes #65. Supersedes #72.

State moves out of the widget's `useState` hooks into a new
`MapLocationPickerController` (a ChangeNotifier). That single change is what
makes the rest of these fixable, because the async paths now have somewhere to
check "am I still current?" before writing.

Crashes and silent failures:
- The location button never worked on first grant. The guard read
  `newPermission != whileInUse || newPermission != always`, which no enum value
  can fail, so the method always returned right after the OS dialog. It is now
  `&&`, `deniedForever` is handled, and every branch -- services off, denied,
  denied forever, timeout -- reports a distinct MapPickerErrorKind instead of
  returning silently.
- The bottom card and the nearby-places sheet crashed on
  `(addressComponents?.first.longName ?? "").substring(0, 1)`: RangeError when
  the list was null (the `?? ""` guaranteed it) and "Bad state: No element"
  when it was empty, which the Geocoding API returns for plus-code-only results.
  Replaced by `addressTitle()`, covered by 8 tests.
- Overlapping reverse-geocodes are now sequenced by request id, so a slow
  earlier response can no longer overwrite a newer address.
- Async continuations no longer write disposed state ("used after being
  disposed" when leaving the screen mid-request).
- Awaiting the GoogleMapController had no timeout, so a missing Android API key
  or a missing web script tag left every tap hanging with the spinner stuck on.
  It now times out into MapPickerErrorKind.mapUnavailable, and the address
  lookup no longer depends on the camera animation succeeding.
- `attachMap` tolerates the platform view being recreated; completing a
  Completer twice used to throw from a platform callback.

Issue #65: `MapLocationPicker` always returned a `Scaffold`, so dropping it into
an existing screen laid it out against unbounded constraints and squashed it
into a corner. The Stack is extracted into a new `MapLocationPickerView` that
omits the Scaffold; `MapLocationPicker` still wraps it. Note that b55ba58's
`SizedBox.expand` + `Positioned.fill` were layout no-ops -- a non-positioned
child of `Stack(fit: StackFit.expand)` already gets tight constraints -- so the
issue was never addressed.

The search bar was built twice per frame and the copy handed to
`bottomCardBuilder` was a `Positioned`, which asserts outside a Stack. It is now
built once, wrapped only for the Stack, and `showSearchBar` lets a consumer who
renders it themselves avoid two live fields sharing one SuggestionsController.

Also fixed: supplying `searchConfig` silently discarded `config.apiKey`, so
autocomplete ran with an empty key; `additionalMarkers['main']` collided with
the built-in marker id and tripped a duplicate-id assertion; picking a nearby
place fired `onNext` and double-popped instead of updating the selection;
`GoogleMapsGeocoding` (and its http.Client) was allocated per request and never
closed; `LatLng(0,0)` was a magic "unset" sentinel, now `skipInitialGeocode`;
selecting a suggestion paid for a second reverse-geocode that could overwrite
the address it just fetched; `hasFocus` was read from FocusManager during build
and forced an unfocus on every camera frame; horizontal safe-area insets were
stripped, pushing controls under the notch in landscape; and the map now gets a
bottom inset so the Google logo stays visible, which the Maps Platform terms
require.

New: `MapLocationPickerStrings` externalises every user-visible string so the
picker can be translated; the main marker is draggable by default with
`onMainMarkerPositionChanged` reporting every move (map tap, drag, current
location, suggestion, programmatic); `GeocodingResultParts` adds city/postalCode/
countryCode/etc. accessors; and the FABs and confirm button carry Semantics.

Thanks to @demon36 for #72 and @Brandon2255p for #65.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…c API

Closes #14.

Web (#14): reversed course after probing the endpoints directly. Contrary to
the 2023 report, `places.googleapis.com` now answers CORS preflights with
`Access-Control-Allow-Headers: content-type, x-goog-api-key, x-goog-fieldmask`
for both `:autocomplete` and place details, and `maps.googleapis.com/maps/api/
geocode/json` returns `Access-Control-Allow-Origin: *`. google_maps_apis sends
the key as `x-goog-api-key`, which is in the allow-list. So the REST transport
works in a browser and the JS-interop shim was a net negative: it forced web
onto `google.maps.places.AutocompleteService` (closed to new Google Cloud
customers since 2025-03-01), dropped filter/sessionToken/fields/cancelToken,
and never covered getDetails at all -- so on web you got suggestions and then
nothing happened when you tapped one. Deleted, along with the conditional-export
shim and the stub. One service, one transport, every platform.

SDK floor corrected to Flutter 3.38 / Dart 3.10. The previous commit's >=3.32
was unsatisfiable: google_maps_apis 5.x needs meta ^1.17.0, and the Flutter SDK
pins meta *exactly* -- 1.16.0 on Flutter 3.32 through 3.37, 1.17.0 from 3.38.
Verified against the Flutter git tags. This was invisible when resolving on
3.47, which is why CI now pins the floor explicitly.

pointer_interceptor added and wired around every overlay. google_maps_flutter_web
renders into an HTML platform view that wins hit-testing, so anything stacked
over it is unclickable; flutter_typeahead 6 dropped the transitive dependency
that 5.x used to provide. Without this, web was pan-only -- no typing, no
Confirm.

New features:
- `PickerPinMode.centerPin`: a fixed pin with the map panning underneath,
  committing on camera idle. This is the interaction ride-hailing and delivery
  apps use, and it was the largest UX gap against place_picker_google. Idle is
  debounced (Android reports it several times as a fling settles, and each one
  would be a billed geocode) -- which finally gives `DeBouncer` a caller.
- `MapLocationPickerStrings`: all 17 user-visible strings, previously hardcoded
  English with no override. The map-type sheet was unreachable even through
  `bottomCardBuilder`.
- Nearby places via `PlacesAPINew.searchNearby` -- on the new API, not the
  legacy endpoint the reference package uses.
- `startWithCurrentLocation`, with a timeout and a last-known-position fallback,
  so a cold GPS fix cannot pin the spinner. The default position was previously
  a village near Rohtak, shown to every user on earth.
- `showBackButton` / `backButtonBuilder`: a full-bleed map with no leading
  control had no discoverable exit.
- `showMapLocationPicker()` returning `PickedPlace`, replacing the ~28 lines of
  Navigator + copyWith(onNext:) + pop that every consumer wrote by hand.
  `PickedPlace.latLng` is always populated, and `name` survives from Place
  Details so a POI keeps "Heathrow Terminal 5" instead of the street address
  reverse geocoding returns.
- `countries` / `placeTypes` as first-class search restrictions. The README has
  documented a `components` parameter for years; it never existed.
- `FloatingControlsPosition`, configurable hero tags (two pickers in one route
  used to throw on duplicate hero tags), `mapTypeButtonHeroTag`,
  `locationButtonHeroTag`, `tapToSelect`, `requireGeocodedAddress`.
- `MapPickerLogLevel` + `mapLogger.onLog`: the logger had no level control and
  no sink, which is why commit 1ca57f5 had to edit the library to reduce
  console spam.

Public API: the barrel used to re-export all of geolocator, google_maps_flutter
and both the legacy and new google_maps_apis libraries -- hundreds of symbols
and nine collisions, including two `LatLng` and two `AddressComponent`. That is
why the package fights itself with `hide LatLng` / `hide Circle` internally, and
why consumers could not name `PlacesAPINew` or `Suggestion` without adding
google_maps_apis, dio and flutter_typeahead to their own pubspec. Now narrowed
to exactly the types in our signatures, plus the generated copyWith extensions
(a `show` clause hides extensions unless named). `example/lib/api_surface_check.dart`
names every one of them and is analyzed in CI, so a missing export fails the
build rather than a user's app. It has already caught two: `NearbySearchFilter`
drags in a `Circle` that collides with google_maps_flutter's.

Also: the confirm button no longer renders filled-and-enabled with a no-op
`onPressed` when geocoding failed; `bottomCardTitle` is wired up and
`bottomCardType` deprecated (both were dead); the "N places found nearby" label
now says "N matching addresses", which is what the list actually contains; and
the `flutter.plugin.platforms` block is replaced by a top-level `platforms:` key
-- the old one was federated-plugin endorsement for a package with no native
code, and made the Flutter tool register a plugin in every consumer app.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
CI was running only `stable`, under a comment promising a `min` entry that did
not exist -- which is exactly how the unsatisfiable >=3.32 floor shipped. The
matrix now pins 3.38.0 alongside stable and runs `pub downgrade` on the pinned
job so the lower bounds are actually compiled, not just asserted.

The pana gate was `--exit-code-threshold 0`, which fails on every push forever:
the measured baseline is 120/160. Set to 10, which covers the platform section
alone -- google_maps_flutter has no macOS/Windows/Linux implementation, so 3 of
6 platforms is an upstream cap this package cannot fix. Everything else has to
be perfect.

Example:
- Removed `UIBackgroundModes: location` and the always-authorization strings
  from Info.plist. A location *picker* has no reason to request background
  location, and anyone who copied this block risked an App Store rejection.
- Added ACCESS_COARSE_LOCATION alongside ACCESS_FINE_LOCATION.
- Moved the Maps JS script after `<base href>` and added `loading=async`, and
  documented that it is needed only to *render* the map -- place search goes
  through the REST API, which supports CORS.
- Dropped the now-redundant direct google_maps_apis dependency; the barrel
  exports what the example needs.
- Rewrote the app to demonstrate what actually exists: centre-pin mode,
  start-at-current-location, country restriction, a fully translated UI, custom
  markers with left-aligned controls, nearby places, and an embedded
  MapLocationPickerView inside a SizedBox -- the case from issue #65.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
pub.dev score: 120/160 -> 160/160.

The 3.x README could not be followed. Its examples used `MapPickerConfig` and
`PlacesAutocompleteConfig`, renamed two releases earlier in 2.0.1; the
restricted-key snippet passed `geoCodingApiHeaders`, which has never been a
parameter; and it documented a `components` option for restricting results by
country that did not exist in the API at all. Rewritten around what the package
actually does, leading with the four-line `showMapLocationPicker` integration.

Every code block in the README is now compiled by
`example/lib/readme_samples.dart`, so `flutter analyze` fails if the docs drift
again. That guard immediately caught one: the README used `picked.city`, which
did not exist (only `locality`) -- so `city` was added as an alias, since that
is the word developers reach for.

MIGRATION_GUIDE.md gets a 3.x -> 4.0.0 section explaining the two changes most
apps need (the Flutter floor and the narrowed barrel), with the meta-pin table
showing why the floor is 3.38 rather than a preference. The stale 2.0 file
structure listing is dropped.

CHANGELOG.md gets the 4.0.0 entry, crediting @CaptainRiley, @vkourtis,
@jansvanda, @CarmeloBeeapp, @Brandon2255p, @Bylinkk, @demon36, @Elbarae1921 and
@dend456 by issue and PR number.

Adds CONTRIBUTING.md, which the README has linked to since 2.0 without it
existing. It covers the two rules that are not obvious: never hand-edit
generated files (the generators are pinned so CI can diff them), and add new
public types to `api_surface_check.dart`.

Also documents platform support honestly: Android, iOS and web (JS and Wasm) are
supported; desktop is not, because google_maps_flutter has no desktop
implementation. The pana gate is tightened from 10 to 0 to match the score.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
54 tests, up from zero before this branch. Each names the bug it pins down.

location_permission_test.dart fakes GeolocatorPlatform to cover the tautology
directly: the first grant now moves the pin (it used to return early and do
nothing), denied / deniedForever / services-disabled each surface a distinct
MapPickerErrorKind rather than returning silently, and isLoading is cleared on
every path. Also covers the startWithCurrentLocation fallbacks -- refusal falls
back to initialPosition without raising an error, and a slow fix falls back to
the last known position instead of pinning the spinner.

picker_features_test.dart covers PickedPlace (the coordinate survives a failed
geocode; a search result's name beats the reverse-geocoded street address),
centre-pin mode (the pin lifts and settles; a burst of five idle events costs
one geocode, not five), and the strings object.

controller_test.dart covers the two async defects that were previously
untestable because the state lived in useState hooks: a slow earlier lookup
cannot overwrite a newer one, and a response arriving after dispose is dropped
instead of writing disposed state.

card_test.dart covers all four inputs that used to throw from `addressTitle`'s
predecessor -- null components, empty list, empty longName, missing formatted
address -- plus the typed component accessors.

Adds geolocator_platform_interface and plugin_platform_interface as dev
dependencies for the fake.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Copilot AI lite review requested due to automatic review settings September 9, 2026 11:46
@kilo-code-bot

kilo-code-bot Bot commented Sep 9, 2026

Copy link
Copy Markdown

Kilo Code Review could not run — your account is out of credits.

Add credits or switch to a free model to enable reviews on this change.

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

🟡 Changes recommended

There are a few correctness/production-safety issues in the new code paths (notably potential double-pop in showMapLocationPicker and release logging callback behavior) that should be addressed before approval.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR delivers the 4.0.0 “full audit” release: it modernizes the dependency stack, fixes multiple long-standing functional issues (autocomplete/session tokens, permission handling, race conditions, crashes), introduces a controller + showMapLocationPicker() API, adds a comprehensive unit test suite, and rewrites documentation and examples to match the new public surface.

Changes:

  • Introduces new public APIs (PickedPlace, showMapLocationPicker, MapLocationPickerController, strings/errors/logging) and refactors internals around them.
  • Fixes correctness issues (permission guard logic, request sequencing, empty-string/list crashes, session token lifecycle, web overlay hit-testing).
  • Adds CI + tests, and rewrites README/migration docs with compiled samples and API surface guards.
File summaries
File Description
test/smoke_test.dart Adds smoke coverage for static map URL helper + config copyWith behavior.
test/picker_features_test.dart Pins behavior for PickedPlace, center-pin mode, strings, config defaults, marker-id contract.
test/location_permission_test.dart Verifies permission/service/timeout flows for current-location behavior.
test/controller_test.dart Tests controller state transitions, sequencing (stale response discard), and map-controller timeout behavior.
test/card_test.dart Regression tests for bottom card + nearby sheet crash fixes and GeocodingResultParts.
test/autocomplete_filter_test.dart Regression tests for issue #70 filter merge + session token rotation semantics.
README.md Full rewrite: new setup guidance, new APIs, troubleshooting, and corrected key-restriction headers.
pubspec.yaml Bumps to 4.0.0, raises SDK floor, updates deps, adds pointer_interceptor and test fakes.
MIGRATION_GUIDE.md Replaces prior content with focused 3.x→4.0.0 guidance + compatibility notes.
lib/src/show_picker.dart Adds showMapLocationPicker() route wrapper returning PickedPlace.
lib/src/picked_place.dart Adds PickedPlace value object with flattened address parts and stable display label.
lib/src/map_location_picker_controller.dart Introduces controller, sequencing, pin modes, nearby search, and robust error reporting.
lib/src/logger.dart Adds log levels + callback routing and filtering controls.
lib/src/geocoding_service.dart Refactors geocoding client for proper client ownership, typed errors, and helpers/extensions.
lib/src/exceptions.dart Adds typed exception model (MapPickerErrorKind, MapLocationPickerException).
lib/src/configs/strings.dart Adds fully overridable MapLocationPickerStrings for localization/customization.
lib/src/configs/search_config.dart Updates search config (hint localization behavior, deprecations, typeahead 6 fields).
lib/src/configs/map_config.dart Expands picker config: new options, deprecations, and richer callbacks.
lib/src/configs/enums.dart Adds enums for pin mode/state and floating control placement.
lib/src/card.dart Fixes crashy title derivation, improves confirm semantics, and corrects nearby-sheet behavior.
lib/src/autocomplete/autocomplete_service.dart Deletes orphaned conditional-export autocomplete abstraction.
lib/src/autocomplete/autocomplete_service_web.dart Deletes JS-interop web autocomplete implementation.
lib/src/autocomplete/autocomplete_service_stub.dart Deletes stub autocomplete implementation.
lib/src/autocomplete/autocomplete_service_io.dart Deletes IO autocomplete implementation.
lib/src/autocomplete_view.dart Improves autocomplete UX, session token stability, web hit-testing, and typed error reporting.
lib/src/autocomplete_service.dart Consolidates Places (New) REST autocomplete/details with error translation + filter merge.
lib/map_location_picker.dart Reduces barrel exports to signature-only types; exports new public APIs.
example/web/index.html Fixes script placement + documents purpose of JS API script for rendering only.
example/pubspec.yaml Updates example SDK/lints and removes redundant direct deps.
example/pubspec.lock Updates lockfile to match the new dependency set.
example/lib/readme_samples.dart Compiles README snippets to prevent doc/API drift.
example/lib/main.dart Rebuilds the demo around showMapLocationPicker, new config options, and embedded view.
example/lib/key.dart Commits placeholder key so the example builds from a clean clone.
example/lib/api_surface_check.dart Adds compile-time guard ensuring the barrel exports the full public surface.
example/lib/.pubignore Removes ignore so key placeholder is included for pub consumers of the example.
example/lib/.gitignore Removes ignore so key placeholder is included in repo builds.
example/ios/Runner/Info.plist Fixes location permission strings and removes inappropriate background-location entries.
example/android/app/src/main/AndroidManifest.xml Adds coarse location permission for parity.
example/analysis_options.yaml Excludes platform/build dirs from analysis for the example app.
example/.gitignore Removes generated_plugin_registrant ignore (web).
CONTRIBUTING.md Adds contributor workflow + emphasizes codegen and doc-sample guards.
CHANGELOG.md Adds detailed 4.0.0 changelog entry covering fixes/additions/behavior changes.
analysis_options.yaml Excludes build outputs at repo root.
.github/workflows/ci.yml Adds CI matrix (floor + stable), codegen drift check, tests, example builds, pana gate.
Review details
  • Files reviewed: 44/47 changed files
  • Comments generated: 4
  • Review effort level: Lite

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread lib/src/autocomplete_service.dart Outdated
Comment on lines +146 to +150
if (_reportIfError(response.error, response.statusCode)) return null;
// Caching the details also refreshes the session token, which is how the
// Places session is formally concluded.
sessionToken?.cachePlaceDetails(id: placeId, data: response.body);
return response.body;
Comment thread lib/src/logger.dart
Comment on lines +182 to +186
if (resolved.value < this.level.value) return;

onLog?.call(resolved, message, error, stackTrace);

if (kDebugMode || emitInRelease) {
Comment thread lib/src/show_picker.dart
Comment on lines +101 to +114
config: widget.config.copyWith(
onNext: (result) {
// Honour a caller-supplied onNext first, then pop with the result.
widget.config.onNext?.call(result);
if (!mounted) return;
Navigator.of(context).pop(
PickedPlace.from(
latLng: _controller.position,
result: result,
place: _controller.lastSelectedPlace,
),
);
},
),
Comment thread lib/src/autocomplete_view.dart Outdated
Comment on lines +277 to +281
/// Get the details of a place.
///
/// Routed through [AutoCompleteService] rather than calling [PlacesAPINew]
/// directly, so that the web implementation (Maps JavaScript API) is used on
/// web where the REST endpoint is blocked by CORS.
itsarvinddev and others added 2 commits September 9, 2026 17:27
…uild

The first CI run failed four ways. Three were real; all three were things the
workflow existed to catch.

1. `channel: 3.38.0` — subosito/flutter-action's `channel` input only accepts
   stable/beta/master, so pinning the floor failed with "Unable to determine
   Flutter version for channel: 3.38.0". An exact release goes in
   `flutter-version`. The matrix is now an explicit `include` with an `id`, both
   because it reads better in the checks list and because `matrix.flutter-version`
   would have been parsed as subtraction in an Actions expression.

2. pana scored 150/160 on CI against 160/160 locally, losing the whole
   "example and screenshots" section to twelve "No such file or directory"
   errors. The screenshots are committed and are not LFS pointers -- pana shells
   out to ImageMagick and cwebp to generate thumbnails, and neither is on the
   ubuntu runner. Installed in that job.

3. The example's Android build failed on "You are applying Flutter's
   app_plugin_loader Gradle plugin imperatively using the apply script method,
   which is not possible anymore." The scaffold dated from 2022: Gradle 7.4,
   AGP 7.1.2, Kotlin 1.6.10, Groovy DSL, imperative plugin loader. Regenerated
   from the current Flutter template -- Kotlin DSL, declarative `plugins` block,
   Gradle 9.3.1, Java 17 -- keeping the API key meta-data and location
   permissions. Also dropped the legacy `package=` attribute from all three
   manifests (the namespace now lives in build.gradle.kts) and adopted the
   PROCESS_TEXT `queries` block the current template ships.

   `flutter build apk --debug` now succeeds locally.

The fourth failure is the third-party Kilo Code Review bot, which also reports
failure on the pre-existing #71 and #72.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
An adversarial review of the branch's own diff raised 32 findings; 28 survived
independent verification. Most are in code this branch introduced, so they are
fixed here rather than being shipped and found by users. Tests: 55 -> 67.

Wrong data returned to the host:
- Centre-pin mode committed the new coordinate immediately but only *scheduled*
  the geocode, leaving the previous point's address and an enabled confirm
  button for the whole debounce window. Confirming inside it returned the new
  coordinates paired with the old street, postcode and country. The lookup is
  now marked in flight the moment it is scheduled.
- A superseded geocode still reported its failure, so an abandoned lookup could
  raise an error toast over a correct address. Failures now go through the same
  request-id sequencing the results do.
- goToCurrentLocation's `finally` cleared isLoading unconditionally, stopping
  the spinner and re-enabling confirm while a newer lookup was still running --
  the same coordinate/address mismatch, reachable in the default marker mode.
- In centre-pin mode the camera settling after a chosen suggestion looked like a
  user pan, so it cleared lastSelectedPlace (losing the POI name from
  PickedPlace) and paid for a second geocode of the point just geocoded.

Crashes and assertions:
- MapLocationPickerView threw "No Material widget found" when embedded exactly
  as its own docs and the README show -- ListTile needs a Material ancestor and
  the view has no Scaffold. It now supplies a transparent one.
- searchBarBuilder's result was wrapped in Positioned/SafeArea/Padding, so
  returning a Positioned -- the 3.x contract -- tripped a ParentDataWidget
  assertion. It goes straight into the Stack again.

Silent misbehaviour:
- The centre pin was drawn at the widget centre while the camera target sits
  inside the padded region, so the geocoded point was ~450 m north of the pin at
  zoom 14. The pin is now offset by the map padding (not on web, which ignores
  padding).
- hideBottomCardOnKeyboard never fired: Scaffold strips the bottom view inset
  from its body when it resizes. Resizing is now off, with the card padded up
  when a caller deliberately keeps it visible.
- initialise() re-ran on every mount, so a caller-supplied controller in a
  TabBarView paid an extra geocode per revisit -- and with
  startWithCurrentLocation it discarded the user's selection. Now idempotent,
  with initialise(force: true) as the escape hatch.
- The geocoding client cache key ignored geocodingHttpClient and
  geocodingApiHeaders, so rotating a restricted-key header or injecting a test
  client was a no-op. Superseded clients are now retired and disposed once
  nothing is in flight, rather than closed under a live request.
- The centre-pin debouncer froze the first pinIdleDebounce value forever.
- moveTo(animate: false) without a zoom never moved the camera at all.
- Nearby search shared the address request id, so a superseded response cleared
  the newer one's loading flag; stale chips stayed rendered and tappable after
  the pin moved.

Errors and lifecycle:
- Transport failures were classified `unknown`, so a host switching on
  MapPickerErrorKind.network never fired; cancellations reported nothing at all
  despite the documented contract. Both now classify correctly, without
  importing dart:io (the package supports wasm).
- _report had no disposed guard, and GeoCodingConfig.dispose() aborts its own
  in-flight request -- so closing the picker mid-lookup called the host's
  onError, typically after the widget that would show it was gone.
- A new PlacesAPINew (two Dio instances and an unclosed HttpClient) was built on
  every keystroke in the default configuration. One client per API key now,
  disposed with the widget.

Regressions against 3.x, now fixed or documented:
- onAddressSelected fired twice per pick in the matching-addresses sheet.
- onLocationError was never called in 4.0 -- an existing app upgrading silently
  lost all location error reporting. Restored and deprecated.
- MapLocationPickerStrings.noAddressFound was never read: the picker used
  config.noAddressFoundText, so the translation example in the README, the
  doc comment and the example app was a no-op for that one string.
  noAddressFoundText is now nullable and deprecated; an explicit value wins.
- A standalone PlacesAutocomplete rendered with a blank placeholder -- the exact
  snippet the README documents.
- The narrowed barrel dropped every geolocator exception, GeolocatorPlatform,
  the companions of the settings classes it does export (ActivityType,
  ForegroundNotificationConfig, AndroidResource), and Bounds/Component/
  ResponseStatus, which Geometry and GeocodingResponse name in their own
  members. All restored. LocationBias/LocationRestriction/ReferencePoint/
  QueryPrediction are added, and the three Places types whose names collide with
  google_maps_flutter are exposed as PlacesCircle, PlacesLatLng and
  PlacesAddressComponent, since Dart cannot rename on export.
- bottomCardTitle went from dead to rendered with a changed default; now in the
  CHANGELOG and the migration guide.
- Query-prediction suggestions rendered as a blank row that did nothing when
  tapped, if includeQueryPredictions was set.

example/lib/api_surface_check.dart is extended to cover the settings companions,
the geolocator exceptions, the geocoding companions and the filter types -- it
had passed while all of the above were missing, which is why they shipped.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
itsarvinddev and others added 4 commits September 12, 2026 13:48
The pinned-floor CI job earned its keep on its first real run. `flutter pub get`
on Flutter 3.38.0 fails with:

    The current Dart SDK version is 3.10.0-290.4.beta.
    Because map_location_picker requires SDK version >=3.10.0 <4.0.0,
    version solving failed.

Flutter 3.38.0 stable ships Dart `3.10.0-290.4.beta`, and pub ranks a
prerelease *below* the release it precedes, so `>=3.10.0` excludes it. 3.38.1 is
the first stable reporting a plain `3.10.0`.

The meta analysis behind the floor is unchanged and re-verified against the
Flutter tags: `meta` is pinned at 1.16.0 through 3.37 and 1.17.0 from 3.38.0, so
google_maps_apis 5.x (meta ^1.17.0) still cannot resolve below 3.38. Only the
patch level moves.

The alternative — writing `sdk: ">=3.10.0-0"` to admit prereleases — would let
every 3.10.0 dev and beta build in, which is not what the package supports.

Updated in pubspec.yaml, the CI matrix, README, MIGRATION_GUIDE and CHANGELOG.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The pinned-floor job's `pub downgrade` step failed with:

    Plugin pointer_interceptor:ios provides an implementation for
    pointer_interceptor_platform_interface and also references a default
    implementation for pointer_interceptor_ios, which is currently not
    supported.
    Please resolve the plugin pubspec errors

pointer_interceptor 0.10.0 and 0.10.1 declare both
`implements: pointer_interceptor_platform_interface` and an ios
`default_package:`, a combination the Flutter tool rejects. 0.10.1+1 dropped the
`implements` key. Verified by reading every published pubspec:

    0.10.0    implements + default_package   broken
    0.10.1    implements + default_package   broken
    0.10.1+1  default_package only           ok
    0.10.1+2  default_package only           ok

So `>=0.10.1` was a bound that resolves but does not build -- exactly what the
"Resolve lower bounds" step exists to find. It only surfaces under
`pub downgrade`, because a normal solve picks the newest version.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…cally

The 3.38.1 analyzer reports `deprecated_member_use_from_same_package` in two
places the 3.47 analyzer does not:

- doc-comment references, e.g. `[MapLocationPickerConfig.noAddressFoundText]`,
  which are now plain code spans or point at the replacement instead;
- the test call sites that exercise the deprecated fields deliberately, whose
  ignore comments named `deprecated_member_use` -- the wrong diagnostic inside
  the defining package.

So `flutter analyze --fatal-infos` was clean here and failed there.

Rather than keep fixing one thing per CI round, Flutter 3.38.1 is now installed
locally and the whole `min` job was run against it end to end: pub get, pub
downgrade, analyze --no-fatal-infos, pub get, analyze --fatal-infos, test.
All green, and 67/67 tests pass on the floor SDK as well as on stable.

CONTRIBUTING.md documents how to install the floor SDK and run that sequence,
because the job has now caught four defects that were each invisible on a recent
Flutter: an unsatisfiable >=3.32 floor, 3.38.0's prerelease Dart, a
pointer_interceptor bound that resolved but did not build, and this.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
…ssed, two docs

Copilot left four inline comments on #73. Each was verified with a running test
rather than taken at face value; two held, one was wrong about its own mechanism
but pointed at a real defect nearby, and one was a documentation fix.

1. show_picker.dart — VALID, and the most serious. `showMapLocationPicker` called
   the caller's `onNext` and then popped unconditionally. MIGRATION_GUIDE.md
   tells 3.x users to adopt this function while keeping a config that almost
   certainly carries `onNext: (r) => Navigator.pop(context, r)` -- so the first
   Confirm tap closed the picker AND the screen that opened it. The `!mounted`
   guard did not help: a route stays mounted for its whole exit transition.
   Now captures the route first and pops only if it is still current.
   `isActive` and `Navigator.canPop` were both tried and are not sufficient --
   `isActive` still pops a dialog the caller opened from `onNext`, and `canPop`
   stays true whenever anything sits below the picker.
   test/show_picker_double_pop_test.dart: 3 of its 5 tests fail without the fix.

2. autocomplete_service.dart — Copilot's stated reasoning was wrong; the defect
   underneath it was real. It claimed caching a null `response.body` "can throw"
   and "won't reliably rotate the session token". Neither is true of
   google_maps_apis 5.1.0: `cachePlaceDetails` guards the map write with
   `if (id != null && data != null)` and calls `refresh()` unconditionally,
   outside that guard. Applying the suggested early-return would have *caused*
   the bug it warned about, leaving a concluded token in place after a billed
   2xx Details request. Verified by applying it and watching the rotation tests
   go red.
   But the symptom is real one branch over: the cache short-circuit returned
   early without rotating, so picking the same place twice in a long-lived
   picker sent an already-used token into the next session -- and Google bills a
   reused token as if none had been sent. Fixed by refreshing on the cache hit.

3. logger.dart — behaviour correct, docstring misleading. `onLog` fires in every
   build mode by design: it is the crash-reporting sink, and gating it behind
   `emitInRelease` as suggested would make it useless in production unless you
   also switched on console spam. The docs now state the two sinks separately --
   `level` gates everything, `emitInRelease` gates only the console copy.

4. autocomplete_view.dart — genuine doc drift of my own making. `_getPlaceDetails`
   still claimed the REST endpoint "is blocked by CORS" on web and that a "Maps
   JavaScript API" implementation handled it there. Both became false in dc258e5,
   which deleted that implementation after the CORS premise was disproved.
   Rewritten to say what the indirection is actually for: the shared session
   token, the per-key client, the details cache and typed errors.
   test/doc_accuracy_test.dart now greps lib/ and the docs for the stale claims
   so this cannot drift back.

83 tests (up from 67), green on both Flutter 3.38.1 (the declared floor) and
3.47.2, analyze --fatal-infos clean on both.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
@itsarvinddev
itsarvinddev merged commit 71592a6 into master Sep 12, 2026
8 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

2 participants