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

Unable to view properties of exported Rust structs #1857

Closed
codehearts opened this issue Nov 9, 2019 · 6 comments · Fixed by #1876
Closed

Unable to view properties of exported Rust structs #1857

codehearts opened this issue Nov 9, 2019 · 6 comments · Fixed by #1876
Labels

Comments

@codehearts
Copy link
Contributor

Describe the Bug

When exporting a struct from Rust to JavaScript, calling console.log or JSON.stringify on an object of the class shows only a ptr property and none of the exported struct's properties

Steps to Reproduce

I've created an example project at codehearts/whoops to reproduce the issue

Consider the following exported Rust struct:

#[wasm_bindgen]
pub struct Series {
    name: String
}

#[wasm_bindgen]
impl Series {
    #[wasm_bindgen(js_name=okKo)]
    pub fn ok_ko() -> Self {
        Self {
            name: "OK K.O.! Let's Be Heroes".to_string(),
        }
    }

    #[wasm_bindgen(getter)]
    pub fn name(&self) -> String {
        self.name.clone()
    }
}

In the output from wasm-pack build, the Series object has the following properties:

const series = Series.okKo();
console.log(series.name); // OK K.O.! Let's Be Heroes

However, when logging the object we see neither of these properties:

console.log(series); // Series { ptr: 1114136 }
console.log(JSON.parse(JSON.stringify(series))); // { ptr: 1114136 }

Expected Behavior

I would expect to see the name property exported from Rust, like how a native JavaScript class would look

Actual Behavior

The object shows only a ptr property, which is not useful for debugging or logging state

Additional Context

I found a solution that works for Node.js at least. The output of console.log can be customized by defining a [util.inspect.custom] method within the class, and the JSON.stringify result is based on an existing toJSON method if one is defined

wasm-bindgen would have to define the [util.inspect.custom] method because util is not imported by default and the brackets create an invalid symbol during compilation, but toJSON can be exported from the Rust

I'm wondering if wasm-bindgen should provide a default [util.inspect.custom], toJSON, and toString which provides the exported properties. Would these be appropriate methods for wasm-bindgen to include when targeting Node.js (and possible just JS for the JSON and string representations?)

@codehearts codehearts added the bug label Nov 9, 2019
@alexcrichton
Copy link
Contributor

Thanks for the report! I'd be somewhat wary to develop a node.js-specific solution here rather than a JS-general solution. Do you know if it's possible to fix this on the web as well? If not then this may fall within the category of "not fixable at this time" for now

@Pauan
Copy link
Contributor

Pauan commented Nov 11, 2019

@alexcrichton toJSON and toString are standard and work everywhere, so we can at least support those.

I do like this idea, but I'm a little concerned about code size: this will add a significant amount of code to every single class, and that code cannot be dead code eliminated.

So I'd prefer if this was behind an attribute or a feature or something like that. Or maybe it could be only enabled in debug mode, but that has some compat risks.

@codehearts
Copy link
Contributor Author

I do like the idea of placing these behind an attribute, my only reasoning to include them by default is that the JS objects would match the Rust structs when you log and inspect them. If the attribute is documented, thought I think it's fair that anyone picking up wasm-bindgen would see that caveat and be able to decide if they need it in their case

I'll try and make time to implement this behind a wasm_bindgen(inspect=true) attribute, generating the console inspection code only for the Node.js target (unfortunately, web doesn't seem to have anything equivalent to override output from console.log)

@alexcrichton
Copy link
Contributor

Instrumenting this as part of #[wasm_bindgen] sounds great to me!

@codehearts
Copy link
Contributor Author

I've been able to get this working, but I'm not happy with the implementation. The #[wasm_bindgen(inspectable)] attribute goes on the struct, so the macro on the impl can't know about it as far as I'm aware. I wanted to panic if the user defined toJSON or toString in that case, but it seems like this has to be done on the cli side (which I think would be an error that rsl/analyze couldn't report)

I also had to add some bookkeeping I'm not happy about. The cli code has to keep a vector of methods on the struct to check for toJSON and toString being defined by the user, and then another vector for getter names to generate exposed fields in the toJSON body. I tried generating toJSON as part of the macro processing, but it seemed difficult to generate a function that returns a js_sys::Object or which builds an object JsValue without serde or js_sys

@alexcrichton
Copy link
Contributor

I suspect it's probably fine to ignore the case where toJSON and/or toString is custom defined, and a struct annotation should likely be good enough for now? Adding more error handling and/or better diagnostics is fine to happen at a future date.

alexcrichton pushed a commit that referenced this issue Nov 26, 2019
* Add support for #[wasm_bindgen(inspectable)]

This annotation generates a `toJSON` and `toString` implementation for
generated JavaScript classes which display all readable properties
available via the class or its getters

This is useful because wasm-bindgen classes currently serialize to
display one value named `ptr`, which does not model the properties of
the struct in Rust

This annotation addresses #1857

* Support console.log for inspectable attr in Nodejs

`#[wasm_bindgen(inspectable)]` now generates an implementation of
`[util.inspect.custom]` for the Node.js target only. This implementation
causes `console.log` and friends to yield the same class-style output,
but with all readable fields of the Rust struct displayed

* Reduce duplication in generated methods

Generated `toString` and `[util.inspect.custom]` methods now call
`toJSON` to reduce duplication

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

Successfully merging a pull request may close this issue.

3 participants