WebBluetoothCG / web-bluetooth

Bluetooth support for the Web.
http://www.w3.org/community/web-bluetooth/
Other
1.38k stars 187 forks source link

Improve the getAvailability() design #307

Open jyasskin opened 8 years ago

jyasskin commented 8 years ago

@slightlyoff suggested that because getAvailability() works slightly differently from PresentationRequest.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>) and navigator.bluetooth.addEventListener('availabilitychanged', <here>), it might make more sense to use a unified watchAvailability(...) 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

domenic commented 8 years ago

use a unified watchAvailability(...) 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.

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.

jyasskin commented 8 years ago
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 watchAvailability() fires the event even if the availability hasn't changed, but doing the second would cause:

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.

domenic commented 8 years ago

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.

beaufortfrancois commented 7 years ago

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.

tabatkins commented 7 years ago

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.