Skip to content
This repository was archived by the owner on Jan 11, 2024. It is now read-only.

Conversation

@melatron
Copy link
Contributor

@melatron melatron commented Apr 7, 2022

No description provided.

@melatron melatron force-pushed the melatron/sier-to-json branch from 9dc607b to 0c70ce1 Compare April 7, 2022 15:50
@melatron melatron force-pushed the melatron/sier-wasm-binding branch from 0dafc81 to d731d3d Compare April 7, 2022 15:53
@melatron melatron marked this pull request as ready for review April 7, 2022 15:55
@melatron melatron requested a review from shelbyd April 7, 2022 15:55
Base automatically changed from melatron/sier-to-json to master May 2, 2022 12:08
@melatron melatron requested a review from shelbyd May 2, 2022 12:16

let mut parser = Parser::default();
parser.add_file_defs(file_defs).unwrap_or_else(|e| {
wasm_bindgen::throw_str(e.to_string().as_str());
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead of throw_str(s.as_str()) you should be able to throw_str(&s). Same elsewhere.

#[wasm_bindgen]
pub fn serialize(js_object: JsValue, file_defs: &str, struct_def: &str) -> js_sys::Uint8Array {
if !js_object.is_object() {
wasm_bindgen::throw_str("Provided argument is not a JS Object.");
Copy link
Contributor

Choose a reason for hiding this comment

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

You might be able to have these functions return Result<T, JsValue> instead of explicitly throwing everywhere. More idiomatic and probably easier to read/write. Might even be able to use the ? operator.

Comment on lines +11 to +13
let json = js_sys::JSON::stringify(&js_object).unwrap_or_else(|e| {
wasm_bindgen::throw_val(e);
});
Copy link
Contributor

Choose a reason for hiding this comment

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

This should be doable as:

let json = js_sys::JSON::stringify(&js_object)?;

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

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants