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

Allow for js property inspection #1876

Merged
merged 4 commits into from
Nov 26, 2019
Merged

Allow for js property inspection #1876

merged 4 commits into from
Nov 26, 2019

Conversation

codehearts
Copy link
Contributor

This resolves #1857 by introducing #[wasm_bindgen(inspectable)]. This annotation:

  • Generates toJSON, which can be overridden if the user defines their own toJSON
  • Generates toString, which can be overridden if the user defines their own toString
  • Generates [util.inspect.custom] for the Node.js target only, which applies to console.log and friends

Its allows all public fields/getters of the Rust struct to be shown when logging or serializing the generated class in JavaScript (console.log, JSON.stringify, etc.)

Let me know if this needs any additional work! I thought about using "display," "debug," or "serialize" to mimic the similar derivations in Rust, but I decided "inspectable" was more clear

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
`#[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
Copy link
Contributor

@alexcrichton alexcrichton left a comment

Choose a reason for hiding this comment

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

Looking great, thanks for this!

}}

toString() {{
return JSON.stringify({readable_properties});
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this perhaps be JSON.stringify(this.toJSON()) to avoid duplication?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I implemented it that way originally, but I thought it may be confusing if a struct providing its own toJSON had the side effect of changing toString and [util.custom.inspect]. If it’s documented then it shouldn’t be an issue though. I’ll take your advice and mention it in the guide!

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah sounds good to me!

name: "inspect".to_string(),
},
fields: Vec::new(),
})?;
Copy link
Contributor

Choose a reason for hiding this comment

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

The return value of this is a String which I think should be used instead of import.custom below

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not sure if I can, this import gives me "inspect" back but the symbol name has to be "inspect.custom"… Do you know if there's another method I should try to get the symbol name in a variable?

Copy link
Contributor

Choose a reason for hiding this comment

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

Oh sorry what I mean is that below instead of [inspect.custom] you'll want to do [{}.custom] where {} is the return value of this function. That way if the identifier inspect is imported from elsewhere it'll still work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That makes sense! I've updated it

dst.push_str(
&format!("
[inspect.custom]() {{
return Object.assign(Object.create({{constructor: this.constructor}}), {readable_properties});
Copy link
Contributor

Choose a reason for hiding this comment

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

Could this use this.toJSON() instead of duplicating the readable properties?

Generated `toString` and `[util.inspect.custom]` methods now call
`toJSON` to reduce duplication
@alexcrichton alexcrichton merged commit df34cf8 into rustwasm:master Nov 26, 2019
@alexcrichton
Copy link
Contributor

Thanks again for this!

@codehearts codehearts deleted the allow-for-js-property-inspection branch November 26, 2019 18:50
alexcrichton pushed a commit that referenced this pull request Jan 6, 2020
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

Successfully merging this pull request may close these issues.

Unable to view properties of exported Rust structs
2 participants