-
Notifications
You must be signed in to change notification settings - Fork 189
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
Improve the getAvailability() design #307
Comments
This sounds scary to me; it would be better to separate out the side effects into one method call, and continue using event listeners for the event. |
navigator.bluetooth.getAvailability().then(isAvailable => {
bluetoothUI.hidden = !isAvailable;
});
navigator.bluetooth.addEventListener('availabilitychanged', e => {
bluetoothUI.hidden = !e.value;
}); currently duplicates the .hidden logic. It's not straightforward to remove the duplication because the two functions take different kinds of arguments. navigator.bluetooth.watchAvailability(isAvailable => {
bluetoothUI.hidden = !isAvailable;
}); would work, as would navigator.bluetooth.addEventListener('availabilitychanged', e => {
bluetoothUI.hidden = !e.value;
});
navigator.bluetooth.watchAvailability(); if we said that navigator.bluetooth.addEventListener('availabilitychanged', e => console.log(e))
navigator.bluetooth.watchAvailability();
// Elsewhere:
navigator.bluetooth.addEventListener('availabilitychanged', e => console.log(e))
navigator.bluetooth.watchAvailability(); to log 3 times instead of 2. |
Sure, that all makes sense. I still don't think it's important enough of a problem to invent a new kind of primitive on the platform for. There are well-known techniques for dealing with code duplication that developers using the web bluetooth API should be able to apply on their own. |
I disagree. If we have the ability to make the platform better now, we should do it. One API at a time. Even if that means some inconsistency for now. I'm thinking about Promises that are not everywhere yet for instance. |
The problem is that it's not "some inconsistency now", it's guaranteed inconsistency until we actually get the new primitive this wants to rely on. This sort of "eh, go ahead and do something that kinda works" is why we have several APIs from the brief period between "promises are cool" and "here's promises" which have weird, inconsistent callback APIs. The convenience gain from switching to bespoke callbacks was very small, compared to the gain from actual promises; it would have been better for those to have stuck with events, which can be upgraded into promises in a consistent way. Same here. This wants to send the current value when you subscribe (like a fulfilled promise does) but also wants to send updates (which events do). The correct primitive is an Observable, but we haven't gotten that finished up for the ES spec yet. It's better, as Domenic says, to just apply existing platform patterns to this API for now, rather than inventing something bespoke (that's still inadequate, when compared to the "correct" solution) which'll just look like an awkward one-off in a few years. |
@slightlyoff suggested that because
getAvailability()
works slightly differently fromPresentationRequest.getAvailability()
, we should consider naming them differently. I believe he's going to take the general question of availability queries to the TAG, so we should wait to commit to something here until they take a look.Because we tend to want the same code to run in both
getAvailability().then(<here>)
andnavigator.bluetooth.addEventListener('availabilitychanged', <here>)
, it might make more sense to use a unifiedwatchAvailability(...)
function that works like an event listener but 1) is guaranteed to be called at least once, and 2) can specify registration to have side-effects, which event listeners can't.@mfoltzgoogle @domenic @tabatkins
The text was updated successfully, but these errors were encountered: