Open chrysn opened 3 years ago
Thanks for the report! I think it should be fine for us to change the WebIDL to accomodate this perhaps?
Reading further into WebIDL, that might easily make sense, eg. as
- readonly attribute Bluetooth bluetooth;
+ readonly attribute Bluetooth? bluetooth;
and
- Promise<BluetoothDevice> requestDevice(optional RequestDeviceOptions options = {});
+ Promise<BluetoothDevice> requestDevice(RequestDeviceOptions options = {});
. Is there any general guidance on which deviations from the upstream WebIDL are OK? (The Guide is silent on that.). Or would those be patches for upstream? On RequestDeviceOptions I'm a bit more clueless -- however, given it's a -sys, it may be up to other crates to create something builder-style that implements Into<RequestDeviceOptions>
-- or where would something be done here?
There isn't necessarily a hard-and-fast guideline here but I think it's fine in this regard since it's unstable. There's not necessarily a canonical source-of-truth of WebIDL from what I've seen since browsers often have their own tweaks or perhaps a different snapshot than us.
In any case the unstable part of this is what matters and that gives us the freedom to change this at any time, so whatever works I think should be fine!
On the proposed patches I now have a PR open (#2385).
On the topic of how to get by the filters, I've found BluetoothLeScanFilterInit which helps over what I had to do with serde before -- but after the conversion, the information that filters is a sequence<BluetoothLEScanFilterInit>
is lost, and only shows shows as JsValue. The two ways forward I see (but would need a great larger understanding of this system) are, in order of decreasing preference:
Express sequences in the type system. A sequence<BluetoothLEScanFilterInit>
would become some type (possibly a generic Iterator<BluetoothLEScanFilterInit>
, as that's what I'm using now with &[...].iter().collect::<js_sys::Array>()
). That would hint the user at how to create them. It also makes the types stricter (you can't do .filter(some_arbitrary_jsvalue)
any more -- that's kind of good but also very breaking.
For all cases where such information is lost, auto-generate documentation there; in this particular case it could be:
The
val
argument should be a sequence of BluetoothLeScanFilterInit; even though that is not enforced at the Rust type level, it is expected on the JavaScript side. One way to create this argument is as&[val1, val2].iter().collect::<
Array
>()
.
If there is a way to get manually added comments from WebIDL to doc comments, equivalent (but possibly more precise) statements or examples could be added via there.
[edit: s/into_iter/iter/ due to https://github.com/rust-lang/rust/issues/66145]
Ah yeah carrying over type parameters would be really nice to do and is an open issue with web-sys and its bindings. At the very least appending to the generated documentation would be great!
I would prefer a Rust-friendly filters type, instead of having to to create JavaScript objects manually with Rust.
Motivation
I'm porting Bluetooth using code over from Javascript, and am running into a few warts around how web-sys exposes them.
As I'm relatively new to web-sys usage, I don't have a clear plan forward with them, and they may or may not be related; in this issue I'd like to hash out how they are related, and what can reasonably be done within being a -sys crate.
Issues
navigator().bluetooth() may fail: Not all browsers have Bluetooth support, and Mozilla probably won't until it has vastly changed.
Currently this causes a panic at with
wasm-bindgen: imported JS function that was not marked as
catchthrew an error: getObject(...) is undefined
; making.bluetooth()
fallible may help here.The request_device method is unusable as the default empty argument for options is not accepted, at least by Chrome. It either needs a concrete filter set, or acceptAllDevices set to true, in the options object.
Building the actual options object with a filter as a JsObject is comparatively verbose. To express
I had to go through a serde object and wrote
Making what MDN calls BluetoothScanFilters available in Rust might help, but so would an example of how to do this more in-line in the JsValue documentation. (I'm pretty sure there's a way to write which RequestDeviceOptions could then implement for compatibility, but a) this is all unstable anyway, and b) given there's codegen involved, such stunts may be hard).
RequestDeviceOptions::new().filters(js! [ {"services": [UUID_US]} ])
, I just didn't find it). If that's accessible easily, it may even be worth considering to drop RequestDeviceOptions in favor of going all JavaScript object (given most of the possible options incur going to a JsValue right again), possibly through a TryInto (but that wouldn't be zero-cost; in a regular API one could probably go from accepting RequestDeviceOptions to accepting any Into