Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Closed
gjvoosten opened this issue Oct 18, 2023 · 6 comments
Closed

Comments

@gjvoosten
Copy link
Contributor

gjvoosten commented Oct 18, 2023

#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
Copy link
Contributor Author

gjvoosten commented Oct 18, 2023

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
Copy link
Owner

smeijer commented Oct 18, 2023

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

@pmaga
Copy link

pmaga commented Oct 18, 2023

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

Described the problem here: #368

@gjvoosten
Copy link
Contributor Author

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

@gjvoosten
Copy link
Contributor Author

Closed by #385

@alexandervlpl
Copy link
Contributor

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?

alexandervlpl added a commit to alexandervlpl/leaflet-geosearch that referenced this issue May 3, 2024
alexandervlpl added a commit to alexandervlpl/leaflet-geosearch that referenced this issue May 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants