search

smeijer / leaflet-geosearch

A geocoding/address-lookup library supporting various api providers.
https://smeijer.github.io/leaflet-geosearch/
MIT License
1.02k stars 270 forks source link

Still getting Uncaught TypeError: t.trim is not a function in v3.10.1 #384

Closed gjvoosten closed 8 months ago

gjvoosten commented 8 months ago

382 isn't completely solved (by #383) in v3.10.1; there is a difference between how validateCoords() is called while typing and after pressing Enter (or selecting a suggestion from the list). While typing, it is called with the string that was typed in, and after pressing Enter, it is called with an object { query: this.searchElement.input.value, data: item } (see https://github.com/smeijer/leaflet-geosearch/blob/develop/src/SearchControl.ts#L332-L336 ).

gjvoosten commented 8 months ago

I had some time to look a bit deeper into this, and I must say I'm quite surprised #368 ever passed the review stage… E.g. it adds coords.ts which starts with this line:

// @ts-nocheck

which frankly I would simply reject. (Remove that directive and npm run lint will tell you some useful things about that file.)

Then the file also has this check:

    if (-90 < lat < 90 && -180 < lng < 180) {

which is interesting (to put it mildly) but not a correct condition to check for valid coordinates (and if we take what it intended to accomplish, it also rejects 90, -90, 180 and -180 as invalid, which is wrong).

I can provide a branch/PR that addresses my gripes (properly TypeScript'ed) if needed, but: Some search providers already support coordinate searches, e.g the EsriProvider (which even suggests alternatives with X and Y coordinates swapped, so you don't really have to worry whether lat or lon come first) and the OpenStreetMapProvider (which does a reverse lookup and presents you with a location if its gazetteer can find anything). So this should at least have been a configurable option for those who don't want or need it.

I would propose to simply revert #368 and re-think what is required, @smeijer.

smeijer commented 8 months ago

Fair enough. Can you open a pull request that reverts both that pull and the later applied fix?

pmaga commented 8 months ago

The problem is not only with coordinate searches; it breaks all custom providers.

Described the problem here: https://github.com/smeijer/leaflet-geosearch/pull/368

gjvoosten commented 8 months ago

PR that reverts this 'feature' is here: #385

gjvoosten commented 8 months ago

Closed by #385

alexandervlpl commented 2 months ago

I've apologized for the PR that caused this. But the fact that query is sometimes a string, and sometimes an object throughout the code seems to be against the philosophy of TypeScript, no?