Aleph-Alpha / ts-rs

Generate TypeScript bindings from Rust types
MIT License
1.08k stars 107 forks source link

`usize` and `isize` should be typed as `bigint` on 64-bit architectures #311

Open lucasvanmol opened 5 months ago

lucasvanmol commented 5 months ago

This would align with how, for example, u32 and u64 are currently typed.

escritorio-gustavo commented 5 months ago

This can be implemented with the following code

#[cfg(target_pointer_width = "16")]
impl_primitives! {
    usize, isize, NonZeroUsize, NonZeroIsize => "number"
}

#[cfg(target_pointer_width = "32")]
impl_primitives! {
    usize, isize, NonZeroUsize, NonZeroIsize => "number"
}

#[cfg(target_pointer_width = "64")]
impl_primitives! {
    usize, isize, NonZeroUsize, NonZeroIsize => "bigint"
}

But people seem to dislike the use of bigint in the first place (#94)

What do you think @NyxCode?

escritorio-gustavo commented 5 months ago

Alternatively, we could make a manual impl that returns

// 6 bytes is the most that can be guaranteed to fit a JS Number
if std::mem::size_of::<usize>() <= 6 {
    "number"
} else {
    "bigint"
}.to_owned()

This would handle any weird architechture (I'm decently sure there's nothing with 5 byte pointers, but who knows)

escritorio-gustavo commented 5 months ago

people seem to dislike the use of bigint in the first place

I do agree that this should be the default though. As https://github.com/Aleph-Alpha/ts-rs/issues/94#issuecomment-1236198583 states, "soundness should not be an opt-in", users coercing i/u64, i/u128, i/usize to number should do so on purpose and at their own risk

lucasvanmol commented 5 months ago

Agreed. The reason I had this issue come was that my rust backend was sending random usizes to the frontend, and they weren't getting parsed correctly because of the limitations with JS numbers.

escritorio-gustavo commented 5 months ago

I see, but beware that even if this gets implemented, you still need to customize your (de)serialization logic to produce bigints for JS, as just changing the type definition in TS doesn't actually fix the issue. In the meantime, you can use #[ts(type = "bigint")] or #[ts(as = "u64")] (I'd prefer as since it prevents typos by actually parsing and using the type you give it, while type just takes in a string and doesn't check it) as a temporary fix

escritorio-gustavo commented 5 months ago

In particular, if you send data to your frontend as JSON, which doesn't support bigint, you'd need to encode your numbers as strings or arrays of bytes, which you'd then need to deserialize into a bigint in your frontend code