Skip to content

refactor(web-client): improve Extension API#761

Closed
Alex Yusiuk (RRRadicalEdward) wants to merge 1 commit into
masterfrom
refactor-extensions-api
Closed

refactor(web-client): improve Extension API#761
Alex Yusiuk (RRRadicalEdward) wants to merge 1 commit into
masterfrom
refactor-extensions-api

Conversation

@RRRadicalEdward
Copy link
Copy Markdown
Collaborator

@RRRadicalEdward Alex Yusiuk (RRRadicalEdward) commented Apr 18, 2025

I improved the API based on the feedback in #749

# WASM
wasm-bindgen = "0.2"
wasm-bindgen-futures = "0.4"
wasm-bindgen-derive = "0.3"
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have tried to use wasm-bindgen convert module to be able to downcast from JsValue to an extension, but I have found that this module is unstable and gives different results each call (first call - fine, everything else - errors). This is a known issue: wasm-bindgen/wasm-bindgen#2231.
This crate fixes the issue.


impl Extension {
pub(crate) fn try_from_js_value(value: JsValue) -> Result<Self, anyhow::Error> {
if let Ok(Some(display_control)) = try_from_js_option::<DisplayControl>(value.clone()) {
Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We have to clone every time since wasm_bindgen_derive doesn't have an API to take JsValue by reference

Copy link
Copy Markdown
Member

@CBenoit Benoît Cortier (CBenoit) left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is looking much more complex than necessary to me. Here is what I had in mind: #762

Can you check this PR instead?

@RRRadicalEdward
Copy link
Copy Markdown
Collaborator Author

Done in #762

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Development

Successfully merging this pull request may close these issues.

3 participants