Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

JSON.stringify with optional arguments #1186

Closed
liamcurry opened this issue Jan 16, 2019 · 4 comments
Closed

JSON.stringify with optional arguments #1186

liamcurry opened this issue Jan 16, 2019 · 4 comments

Comments

@liamcurry
Copy link
Contributor

JSON.stringify has two optional arguments that are unsupported in js-sys.

The two arguments are:

  1. replacer, is nullable and can be a function or an array of JsValues.
  2. space, is nullable and can be a str or a number.

I imagine two new functions could be exported to support this (untested):

#[wasm_bindgen]
extern "C" {
    #[wasm_bindgen(catch, static_method_of = JSON, js_name = stringify)]
    pub fn stringify_with_replacer(obj: &JsValue, replacer: Option<&JsValue>) -> Result<JsString, JsValue>;

    #[wasm_bindgen(catch, static_method_of = JSON, js_name = stringify)]
    pub fn stringify_with_replacer_and_space(obj: &JsValue, replacer: Option<&JsValue>, space: Option<&JsValue>) -> Result<JsString, JsValue>;
}
@alexcrichton
Copy link
Contributor

Sounds like great APIs to add to me!

@fitzgen
Copy link
Member

fitzgen commented Jan 16, 2019

Overall: yes we should totally add support for these optional parameters!


Nitpicking: Does the stringify_with_replacer variant need to take an Option<&JsValue>? presumably if you weren't using a replacer you would use normal stringify, so it seems we don't need an Option here.

@liamcurry
Copy link
Contributor Author

Makes sense to me. I assume then stringify_with_replacer_and_space wouldn't take an Option<&JsValue> in the last argument either.

liamcurry added a commit to sagan-software/wasm-bindgen that referenced this issue Jan 16, 2019
This commit adds two new externs for `JSON.stringify`:
`JSON::stringify_with_replacer` and
`JSON::stringify_with_replacer_and_space`.

Fixes rustwasm#1186
@liamcurry
Copy link
Contributor Author

I created PR #1190 to resolve this issue. Apparently Option<&JsValue> and Option<JsValue> are both unsupported:

error[E0277]: the trait bound `&wasm_bindgen::JsValue: wasm_bindgen::convert::OptionIntoWasmAbi` is not satisfied
    --> crates\js-sys\src\lib.rs:3769:1
     |
3769 | #[wasm_bindgen]
     | ^^^^^^^^^^^^^^^ the trait `wasm_bindgen::convert::OptionIntoWasmAbi` is not implemented for `&wasm_bindgen::JsValue`
     |
     = note: required because of the requirements on the impl of `wasm_bindgen::convert::IntoWasmAbi` for `std::option::Option<&wasm_bindgen::JsValue>`

So all the arguments are &JsValue, which is fine, people can just use JsValue::NULL instead of None.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants