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

Promisify getCurrentPosition() #174

Open
lukewarlow opened this issue Aug 21, 2024 · 5 comments
Open

Promisify getCurrentPosition() #174

lukewarlow opened this issue Aug 21, 2024 · 5 comments

Comments

@lukewarlow
Copy link
Member

lukewarlow commented Aug 21, 2024

The geolocation API requires use of callbacks rather than being promise based like modern APIs. However, there is room to offer a promise alternative. If we make the callbacks optional, in the case where they're undefined we could return a promise. This was previously done for Notification.requestPermission. This feels like it should be a relatively simple change to make to the API that little bit more intuitive for new developers.

It'll probably never be possible to deprecate the old syntax but at least it would improve newer code.

The only slightly unfortunate bit would be that setting the options param might need to be done like await navigator.geolocation.getCurrentPosition(undefined, undefined, {...}), this also wouldn't be the first time a parameter needs to be set to an empty value, the same is common for history.pushState()

It feels like watchPosition should/could be updated too but I can't think what that would look like given it fires multiple times?

@lukewarlow
Copy link
Member Author

As it turns out I'm not the first to think of this, see #48 (comment) which came to exactly the same idea as I did.

@lukewarlow
Copy link
Member Author

The only slightly unfortunate bit would be that setting the options param might need to be done like await navigator.geolocation.getCurrentPosition(undefined, undefined, {...})

I guess you could work around this by making the IDL have the first paramter be a PositionSuccessCallback or PositionOptions but that probably gets messier from an implementation perspective?

@reillyeon
Copy link
Member

While I love promises as well, my main concern is that the new behavior would be hard to feature detect for, so it is only practical for sites targeting only evergreen browsers (once they all implement it). In contrast, the promise-returning version is extremely easy to polyfill. Basically, I expect that even if we did spec and implement this it would be years before anyone could reliably use it. Maybe that's still worth it?

@lukewarlow
Copy link
Member Author

The first parameter is required at the minute so I guess you can try the new approach and if it throws fallback to the old one.

Wouldn't provide any immediate benefit if you needed to handle both.

But regarding it taking years before it's useful, that's true but that's always going to be the case so if we think it's a good idea to change the API shape the sooner it happens and is implemented the sooner it will be usable?

Though as you say it's not super difficult to do in JS so maybe it's not worth it?

@mkruisselbrink
Copy link
Contributor

There is also always the slight concern that changing the return type of a method that previously didn't return a promise to now return a promise also means that any exceptions thrown by the method will immediately change into promise rejections instead (i.e. webidl does not support methods that only return a promise for some of its overloads). Although in this case that's probably not much of a problem either since getCurrentPosition doesn't really ever throw an exception anyway (other than in the case of arguments being passed that don't match its signature)

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

3 participants