onury / geolocator

A utility for getting geo-location information via HTML5 and IP look-ups, geocoding, address look-ups, distance and durations, timezone information and more...
https://onury.io/geolocator
MIT License
642 stars 107 forks source link

Server-side support #55

Open paulogr opened 6 years ago

paulogr commented 6 years ago

Hello!

This function brakes on server side because obviously navigator api isn't part of the NodeJS, but given the name of the function isGeolocationSupported isn't supposed to verify exactly this ?

https://github.com/onury/geolocator/blob/728785d619398f78eb9be5609cc722dcbfa7dffb/src/core/geolocator.js#L1783

My proposal is to check the type of navigator keyworkd before use it like bellow:

return typeof navigator != 'undefined' && ('geolocation' in navigator);

That way the function will return false and won't break anymore :)

What do you think about?

Thanks

onury commented 6 years ago

Hi. It's technically impossible to get the precise location of a user from server-side via GPS. On server-side, you can only guess the location from remote address/IP.

This library is only for browser. You cannot/should not use it on server-side.

paulogr commented 6 years ago

I’m sorry but I think you’re missing my point.

I do not want navigator api to work on server, I know it’s impossible. I just want your function do not throw an error but fail gracefully.

I can give you some use cases if you still don't see the problem.

@onury Could you check it again, plz?

Thank you!

onury commented 6 years ago

Some methods of Geolocator would work on server-side. But it's not meant for it. Most of the API make use of Google JS APIs, HTML5 APIs and others that only work on browsers. Why do you want to use this on server?

paulogr commented 6 years ago

In case of use Single Page Application like Angular, React and others there's a feature called Server Side Rendering, that executes the client code on server side to improve speed and features like SEO (Search Engine Optimization).

What's happened with your code, is when I enable the SSR and the code is executed on server side it throw an error to check if geolocation is supported.

My proposal is only to improve how the support of geolocation is checked to not broke the application when it doesn't.

Being aware that your code could be executed on server side even if the feature is not supported makes you lib isomorphic and helps to improve use cases like ssr.

Make sense for you?

onury commented 6 years ago

I didn't do too many SSR apps. I'm assuming you will still make use of geolocator on client-side when page is rendered and passed back to browser. Still, I'm curious why that line got invoked on server.

Rendering your app with SSR, is that the only place Geolocator throws? Even if we change that I'm sure there will be others. Can you change that line and see if it throws in other places? Does it still work on client-side after render?

paulogr commented 6 years ago

The line is invoked on the servers, because the same component/page is executed on both client and server sides. Yes it still works on client even broken on server but something broken on the server side means that the app couldn't generate the server side rendering and so the benefits of performance and seo were broken too.

Yes on SSR is the only place where Geolocator throws an error.

Maybe you're right and another pieces of code could break on server too and yes I can take a look if just changing this line solves the issue or have to have more changes.

If it works, I will send a PR for analyse.

Happy you get the point :)

Thanks

davidfurlong commented 6 years ago

+1 I'm trying to use geocode on the server (Which I don't expect to be specific to browser Web APIs, as calling an external API?) but its giving a ReferenceError: window is not defined. With all the bells and whistles of this library I expected it to work. I don't expect the library authors to have done this, its more of an expectations / documentation thing. I'd appreciate allowing a small disclaimer at the end of the README regarding support for Isomorphic applications / Server side rendering, as this use case has become fairly common nowadays.

Additionally, the solution to my particular issue could be resolved by using an isomorphic fetch library or letting the implementers polyfill it here: https://github.com/onury/geolocator/blob/728785d619398f78eb9be5609cc722dcbfa7dffb/src/lib/fetch.js#L295

onury commented 6 years ago

@davidfurlong you're right. Pls see updated readme, Caveats section.

I'm reopening this. Let's see some further discussion and if we can get help on this.

Gonzo2O28 commented 5 years ago

great lib, using it with a lot of fun here. Is it possible to use this to get an dress from coords to reverse lookup? I have multiple abilities to select the location, one of them is to pick the center of the map and start from there and the google reverse lookup only gives me that strange formatted_adrressen instead of easy accessible variables like i got with the geolocator.

regards