Fix MaplibreGeocoderPlaceResults type definition
#163
Conversation
MaplibreGeocoderPlaceResults should return a single place feature.
Moved GeoAPI example of getSuggestions and getPlaceById from 'getItemValue' attribute documentation to geocoder's addTo documentation with other GeoAPI functions.
| @@ -166,16 +166,6 @@ export type MaplibreGeocoderOptions = { | |||
| proximityMinZoom?: number; | |||
| /** | |||
| * A function that specifies how the selected result should be rendered in the search bar. HTML tags in the output string will not be rendered. Defaults to `(item) => item.place_name`. | |||
| * @example | |||
There was a problem hiding this comment.
This was probably incorrectly placed here, I would suggest to move it above the maplibregeocoder class, where there is a short comment.
There was a problem hiding this comment.
I think it makes sense to leave this removed, and perhaps there is no need to show getSuggestions and getPlaceById in examples as they are well defined in the types?
There was a problem hiding this comment.
Type definition and examples are different things... I would prefer to have even more than one example to indicate how to use something. Type definition are important but less helpful in my experience...
lib/index.ts
Outdated
| @@ -237,7 +227,7 @@ export type MaplibreGeocoderApiConfig = { | |||
|
|
|||
| export type MaplibreGeocoderFeatureResults = { type: "FeatureCollection", features: CarmenGeojsonFeature[]}; | |||
| export type MaplibreGeocoderSuggestionResults = { suggestions: { text: string, placeId?: string }[] }; | |||
| export type MaplibreGeocoderPlaceResults = { place: CarmenGeojsonFeature[] }; | |||
| export type MaplibreGeocoderPlaceResults = { place: CarmenGeojsonFeature }; | |||
There was a problem hiding this comment.
Shouldn't this improve the types in the code to avoid "as any" somewhere? Why doesn't this solve or break anything?
There was a problem hiding this comment.
Yes, I see that this broke the MaplibreGeocoderResults used by _handleGeocodeResponse which is expecting all of the geocoder function results to return arrays either directly or as the value of a property of the returned object.
I've reverted that and fixed the original issue by removing the array that was wrapped around getPlacebyId's place array on line . The extra array caused _handleGeocodeResponse to throw an error and the map to never zoom on the selected place.
lib/index.ts
Outdated
| @@ -384,6 +375,8 @@ export default class MaplibreGeocoder { | |||
| * const GeoApi = { | |||
| * forwardGeocode: (config) => { return { features: [] } }, | |||
| * reverseGeocode: (config) => { return { features: [] } } | |||
| * getSuggestions: (config) => { return { suggestions: {text: string, placeId?: string}[] }} | |||
There was a problem hiding this comment.
The example here is an "actual" example, there should be something like place: "my-place",... I think.
There was a problem hiding this comment.
I removed this as I think including these optional functions does not help the goal of showing how to attach the geocoder to a container element.
There was a problem hiding this comment.
I would recommend adding a "simple" example and "complicated" example.
|
Thanks for taking the time to solve this issue!! |
|
Any updates on this? |
Fix
MaplibreGeocoderPlaceResultstype definition and update documentation in code to reflect expected return ofsearchByPlaceIdfunction. Fixes #162