biscuit-auth / biscuit-wasm

WebAssembly wrapper for Biscuit authorization tokens
Apache License 2.0
24 stars 10 forks source link

implement set_root_key_id #44

Closed louisjmorgan closed 10 months ago

louisjmorgan commented 10 months ago

I've added the wasm bindings for set_root_key_id as well as a test to ensure it works.

Currently the errors aren't being handled as far as I can tell. I assume if you pass the native function a string instead of an int you get a type error, but in the current implementation it seems to set the root key id to 0.

I was unable to chain the error mapping method that most of the other bindings seem to be using:

#[wasm_bindgen(js_name = setRootKeyId)]
pub fn set_root_key_id(&mut self, root_key_id: u32) -> Result<(), JsValue> {
    self.0
        .set_root_key_id(root_key_id)
        .map_err(|e| serde_wasm_bindgen::to_value(&e).unwrap())
}

Would give the following compiler/rust analyzer error:

no method named `map_err` found for unit type `()` in the current scope
   method not found in `()`

I did make it work the following way:

#[wasm_bindgen(js_name = setRootKeyId)]
pub fn set_root_key_id(&mut self, root_key_id: u32) -> Result<(), JsValue> {
    Ok(self.0.set_root_key_id(root_key_id))
}

But I wasn't sure if this is redundant without an accompanying Err statement. I certainly couldn't see any other methods constructed this way. I assume the JsValue refers to the way that wasm-bindgen returns the rust errors to javascript, but I'm not sure exactly how to make that happen without the map_err method chain.

Having said that I'm probably just missing something very obvious, so let me know and I'll make the relevant change.

I also notice my javascript formatter config disagrees with yours, if you let me know exactly what you're using (some kind of global prettier config?) I will try to match it and avoid inconsistencies.

divarvel commented 10 months ago

I was unable to chain the error mapping method that most of the other bindings seem to be using

set_root_key_id() cannot fail, so you don't need to return a Result, and it does not return any value, it just modifies the object on which it is called, so returning () is fine. The merge method on BlockBuilder is like this.