wasm-bindgen icon indicating copy to clipboard operation
wasm-bindgen copied to clipboard

Include Record Type

Open OptimisticPeach opened this issue 1 year ago • 2 comments

Motivation

In the GpuDeviceDescriptor generated struct in web_sys, we don't get access to the required_limits api since it has a WebIDL type of record<DOMString, GPUSize64> requiredLimits;. The relevant piece of code which omits the record type is here.

WGPU needs access to this particular field to be able to request a device with limits other than the bare minimum.

Proposed Solution

A kind of Record type added into js_sys, since it seems that the blocker (reasonably) is the lack of a Record type in js_sys. Exactly how this would look is something I'm not sure, hence why this is an issue and not a PR, and am welcoming discussion.

Alternatives

We could also simply ignore the type constraints we are supposed to follow for record types and allow any keys. However, this would be unwanted since you could get silent or cryptic errors since the api you are communicating with may recieve unexpected types as the keys.

OptimisticPeach avatar Aug 31 '23 07:08 OptimisticPeach

It seems like records are only ever used as inputs in WebIDL, so it could also make sense to have a builder-style API for them similar to what's done for dictionaries. Something like:

#[wasm_bindgen]
extern "C" {
    #[wasm_bindgen(extends = Object)]
    pub type GpuDeviceDescriptorRequiredLimits;

    #[wasm_bindgen(method, indexing_setter)]
    fn add_entry(this: &GpuDeviceDescriptorRequiredLimits, key: &str, value: f64);
}

impl GpuDeviceDescriptorRequiredLimits {
    pub fn new() -> Self {
        Object::new().unchecked_into()
    }

    pub fn entry(&mut self, key: &str, value: f64) -> &mut Self {
        // alternatively this could use `Reflect::set`
        self.add_entry(key, value);
        self
    }
}

Liamolucko avatar Sep 02 '23 10:09 Liamolucko

It seems like records are only ever used as inputs in WebIDL, so it could also make sense to have a builder-style API for them similar to what's done for dictionaries.

Considering how using a builder-style API for dictionaries didn't turn out so well, I would prefer not to repeat this mistake. Especially the whole &mut self and using Reflect. I think we should just supply getters and setters as with any other normal type.


Happy to review a PR adding this!

daxpedda avatar Sep 02 '23 12:09 daxpedda