fix: Remove try_array_to_vec and downcast
#82
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
richvdh
reviewed
Jan 8, 2024
richvdh
requested changes
Jan 8, 2024
There was a problem hiding this comment.
lgtm generally, though please could the doc be updated as previously noted.
I don't suppose that wasm-bindgen supports Vec<&identifiers::UserId>, thus avoiding the drop problem?
Particularly given this is a not-very-obvious breaking change, could you add a note to the the CHANGELOG warning applications about this?
It's not supported, indeed. I've updated the docs. |
richvdh
requested changes
Jan 10, 2024
Now that `wasm-bindgen` supports `Vec<T>` and `Option<Vec<T>>`, we no longer need `try_array_to_vec`. This patch removes this helper function.
The `downcast` function is removed in this patch because it was incompatible with JS code minifier. Instead, we now rely on the new `wasm_bindgen::convert::TryFromJsValue` API. This patch also replaces `&Array` by `Vec<T>`. It's fine, except that JS arrays that are sent to Rust are going to be dropped after the functions return. It's quite unsual for JS, but documentations have been updated accordingly.
This can be helpful when the `user_id` is dropped on the Rust side.
This patch moves the `OlmMachine::import_exported_room_keys_helper` to a `impl` that is not annotated by `#[wasm_bindgen]` because it doesn't have to live in the JS binding.
This patch replaces a few `js_sys::Array` by `std::vec::Vec` when possible. It makes the code simpler, and `wasm_bindgen` now handles `Vec` well for us.
|
I've rebased the patch. I've updated the documentation. I've also replaced a couple more |
richvdh
approved these changes
Jan 19, 2024
9 tasks
richvdh
added a commit
that referenced
this pull request
Jan 22, 2024
richvdh
added a commit
that referenced
this pull request
Jan 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This patch updates
wasm-bindgento 0.2.89. It allows to removetry_array_to_vec(which was fine, but no more necessary), and to removedowncast(which was a problem, see #51).This patch should ideally be reviewed commit by commit.