Properly implement From<usize/isize> and PartialEq<usize/isize> for JsValue
#2978
Conversation
…or `JsValue` Fixes rustwasm#2977 Previously, `usize` and `isize` were converted to bigints when converting to JS values, when they should be converted to regular JS numbers due to being 32-bit. `PartialEq` was then implemented on top of that, checking whether `self == JsValue::from(n)`. It could be argued that they should be bigints to try and be forwards-compatible with 64-bit WebAssembly; however, they're already represented as regular numbers everywhere else. One major issue here is that this is a breaking change. You can't just use a number where a bigint is expected (or vice versa), so this will break any code that used `From` to convert from `usize`/`isize` to `JsValue`. That happens most commonly indirectly, through the return type of async functions, which are internally converted using `Into<JsValue>`.
|
Just in case I wasn't clear enough (it was sorta buried at the bottom of the PR description), this will almost certainly break anything which relies on the previous behaviour. I doubt that there are many things that do, and this isn't actually a breaking change to the API (so the code will still compile), but I want to double-check that you're okay with that. |
|
It should be fine. I assume the equality works as it's supposed to, right? let one: JsValue = JsValue::from(10usize);
assert!(one == 10usize)How the comparison happens was (and still is) an implementation detail so I don't see how this breaking change effects much. |
Yes, that works; it's this kind of thing that breaks: // previously returned true, now returns false:
assert_eq!(JsValue::bigint_from_str("10"), 10usize);
// previously returned false, now returns true:
assert_ne!(JsValue::from_f64(10.0), 10usize); |
|
I'd argue that the previous behavior was incorrect and has been fixed now
|
|
This definitely broke serde-wasm-bindgen tests and took a bit to narrow down, but, luckily, it's only the tests that got broken, because Serde itself always serializes usize/isize as u64/i64, regardless of platform size, so our runtime behaviour didn't change. |
Update deps and fix tests after rustwasm/wasm-bindgen#2978.
Fixes rustwasm#3055 This changes `isize` and `usize` to be converted to `BigInt` in the same way as `i32`/`u32`, `BigInt(JsValue::from(n))`, rather than `JsValue::from(n).unchecked_into()`. The latter is now wrong since as of rustwasm#2978 that `JsValue::from` returns a `Number`, not a `BigInt`.
* js-sys: Fix `BigInt::from(usize)` and `BigInt::from(isize)` Fixes #3055 This changes `isize` and `usize` to be converted to `BigInt` in the same way as `i32`/`u32`, `BigInt(JsValue::from(n))`, rather than `JsValue::from(n).unchecked_into()`. The latter is now wrong since as of #2978 that `JsValue::from` returns a `Number`, not a `BigInt`. * Add a regression test * fmt
Fixes #2977
Previously,
usizeandisizewere converted to bigints when converting to JS values, when they should be converted to regular JS numbers due to being 32-bit.PartialEqwas then implemented on top of that, checking whetherself == JsValue::from(n).It could be argued that they should be bigints to try and be forwards-compatible with 64-bit WebAssembly; however, they're already represented as regular numbers everywhere else.
One major issue here is that this is a breaking change. You can't just use a number where a bigint is expected (or vice versa), so this will break any code that used
Fromto convert from ausize/isizetoJsValue. That happens most commonly indirectly, through the return type of async functions, which are internally converted usingInto<JsValue>.