Skip to content
This repository has been archived by the owner on May 1, 2024. It is now read-only.

Fix "supermarket" missing from PlaceTypes #402

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

breyed
Copy link

@breyed breyed commented Apr 30, 2020

Fixes #401. This is a breaking change: the type of Google.Places.Types is changed from PlaceType[] to string[].

Advantages:

  • Works around a bug in Google's SDK, which includes supermarket in the supported types, but does not include it in GMSPlaceType.
  • Adds resilience against future change to the Google SDK. If a type is added that is not known to the Xamarin wrapper, the wrapper's code will get an unexpected string, which is much easier to deal with than the getter for Places.Types throwing NotSupportedException.

Disadvantages:

  • When developers update to the new NuGet package, they will get a compile error and have to change their code. This is preferable, however, to their code compiling and the system appearing to work, only to have apps mysteriously crash in the field as users search happen to search for supermarkets.

If the PR is implemented, PlaceType should be removed, as it is no longer uses, and the documentation should be updated to link to the supported types.

Fixes xamarin#401. This is a breaking change: the type of `Google.Places.Types` is changed from `PlaceType[]` to `string[]`.
@breyed
Copy link
Author

breyed commented Apr 30, 2020

Report for bug in Google SDK: No constant for "supermarket" place type

@breyed
Copy link
Author

breyed commented Apr 30, 2020

For consistency, if this PR is implemented, In Google.Places.AddressComponent, the property Types should be changed to type string[]. The property Type should be removed: it is already obsolete; a breaking-change transition is a good time to remove it.

@SotoiGhost SotoiGhost self-assigned this Apr 30, 2020
@SotoiGhost
Copy link
Contributor

@breyed Thanks for your contribution and thanks for linking to the right docs!

Breaking changes are bad for everyone. Let me check the code to see if there's a way to avoid the pain.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

"supermarket" missing from PlaceTypes
2 participants