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

Add cargo feature: serde-serialize compatibility #70

Open
therocode opened this issue Nov 28, 2024 · 0 comments
Open

Add cargo feature: serde-serialize compatibility #70

therocode opened this issue Nov 28, 2024 · 0 comments

Comments

@therocode
Copy link

Some time ago the JsValue::from_serde(&to_serialize) in the "serde-serialize" feature of wasm-bindgen was deprecated, which prints the warning:

warning: use of deprecated associated function `wasm_bindgen::JsValue::from_serde`: causes dependency cycles, use `serde-wasm-bindgen` or `gloo_utils::format::JsValueSerdeExt` instead

I.e. it is pointing at this crate as an alternative which is great.

However, there is a compatibility problem here. You'd think that given the above warning, replacing the above line with the serde-wasm-bindgen equivalent of serde_wasm_bindgen::to_value(&to_serialize) would be a suitable fix but it's not. There are subtle differences in the output, including (but possibly not limited to):

  • None becomes undefined instead of null
  • HashMap becomes Map instead of an Object

This causes subtle breakage. When looking into the options to resolve this I found similar issues like 68 and 60 so it's not exactly a unique request. These requests were rejected, referring to the ability to configure the output from Serializer::new().

I'd like to present the perspective that this is not a sufficient solution, especially when people are migrating from codebases that use the now deprecated "serde-serialize" functionality. We have a sizeable application and our Rust<->JS layer is many thousands of lines long. We saw subtle breakages by changing over to this library, and while we should have known that there could be differences, the fact that it's suggested as a suggestion which has a very similar function to call makes this an easy mistake to make.

Since there is a suggestion in that warning to transfer to this library, it would have been amazing if this library had a cargo feature for "serde-serialize-defaults" or something that makes the default Serialize::new() instance be configured in a matching way. This could then be suggested in the wasm-bindgen warning.

By NOT providing this feature, every single person who's previously used serde-serialize who wants to upgrade to this library will need to:

  • Realise that the equivalent function is subtly different
  • Discover the intricacies of those differences
  • Create a custom to_value wrapping function that initializes the serializer with the correct options.
  • Make sure that everyone they are working with are NOT using the default provided, to_value.
  • All this, with no indication or help mentioned in the above deprecation warning.

To me, this just risks subtle bugs in many web applications which isn't a good thing. So, I hope you can be open to the suggestion of considering such a compatibility mode since the wasm-bindgen serialize feature has been such a corner stone in the Rust web app story.

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

1 participant