Downcast doesn't play nicely with minification

[kegsay]

This function looks at constructor names to try to determine the right types. Specifically:

let constructor_name = Object::get_prototype_of(value).constructor().name();

This may return an unexpected constructor name when minification has been used, as classes are renamed to shorter strings e.g UserId becomes NG. This can then cause confusing errors:

Error: Expect an `UserId` instance, received `Ng` instead

[Hywan]

With https://github.com/rustwasm/wasm-bindgen/pull/3554, it might no longer be necessary to play with downcast here, as it's primarily used for supporting Vec<T> as far as I know.

[Hywan]

A new release of wasm-bindgen has been made yesterday, perfect (0.2.88).

[richvdh]

A new release of wasm-bindgen has been made yesterday, perfect (0.2.88).

Yes, though given we have custom impl From<X> for JsValue implementations for various of our types, it seems to cause damage for us in much the same way as https://github.com/rustwasm/wasm-bindgen/issues/3685.

[richvdh]

This is going to be a blocker for full release of Element R, because normal builds use minified source.

[Hywan]

Do you need my help here?

[richvdh]

Do you need my help here?

Any help would be much appreciated.

At the moment, my ideas are:

• Get rid of the check on the constructor name altogether, and just assume that the application side has passed an object of the right type. Obviously, this will break horribly if they do not.
• If we could somehow get hold of a JsValue holding an instance of an object known to be of the right type, then rather than hard-coding the constructor name, we could compare the two constructors. But we wouldn't want to create an object on the JS side for every call, so I don't think this works.

[richvdh]

Just for the record: We could have replaced downcast with wasm_bindgen::convert::TryFromJsValue (as introduced in https://github.com/rustwasm/wasm-bindgen/pull/3709), but:

(a) that's supposedly internal;

(b) we don't need downcast at all since wasm-bindgen now supports properly-typed Vec<...> parameters that will check the values in the array at runtime (added in https://github.com/rustwasm/wasm-bindgen/pull/3554, as modified by https://github.com/rustwasm/wasm-bindgen/pull/3709)

[richvdh]

Just for the record: We could have replaced downcast with wasm_bindgen::convert::TryFromJsValue (as introduced in rustwasm/wasm-bindgen#3709)

oh, in fact @Hywan's PR does exactly that in this instance: https://github.com/matrix-org/matrix-rust-sdk-crypto-wasm/pull/82/commits/c556316057b346ad7b6a9d6b1303738a55f67c49#diff-dd97281cbaa3b13ed5a8dc4417797959632eed864b774977e013fc307dcc9f01L1002-R1012

[MTRNord]

There is also https://github.com/matrix-org/matrix-rust-sdk/compare/main...MTRNord:matrix-rust-sdk:MTRNord/fix-binding-reflection which is based on wasmer code. However, afaik the repo for wasm-bindgen-downcast is still not public but the source on crates has a FOSS license as seen here: https://docs.rs/crate/wasm-bindgen-downcast/0.1.1/source/README.md

The missing repo is at https://github.com/wasmerio/wasm-bindgen-downcast

Thats how https://github.com/MTRNord/cetirizine is deployed

(Also yes my fork is dated ^^ but I hope it gets the point across still)

[Hywan]

Thank you but we don't need an extra dependency. wasm-bindgen provides all the features we need.