Types of js_sys::Date functions seem wrong

[flumm]

Summary

Types of js_sys::Date functions seem wrong

Additional Details

When i have a Date object in the browser (or nodejs). E.g., i can do

let foo = new Date(); foo.setMonth(-1);

to set the date to the last month of the previous year which is super handy when doing calculations on dates (This works for all fields). With this crate this does not work, since most of the functions only take 'u32' as parameter. MDN does not really specify the exact type, but it says 'integer' so unsigned values seem wrong. ECMAScript standard just says it has to be convertable to the number type:
https://262.ecma-international.org/14.0/#sec-date.prototype.setmonth
https://262.ecma-international.org/14.0/#sec-tonumber

Curiously, the function set_full_year_with_month (and similar ones) do accept i32, but for the month part only.
(This also applies for the new_with_year_month_.. etc functions, but only for month,day,etc. never for the year)

I'd propose that all the functions should take a signed integer (at least i32, probably even i64/f64?) so that one can use that functionality described above, or is there any reason why it is implemented in this way? If there is a reason, this should at least be documented properly in the crate.

[daxpedda]

Yeah, that's a bug.
Unfortunately we can't fix this right now as it would be a breaking change.

[flumm]

makes sense. when do you do breaking changes? should i write a pull request? if yes, which types would be preferable?

[daxpedda]

when do you do breaking changes?

There is currently no timeline for a breaking change. Unfortunately wasm-bindgen is practically not in active development anymore, but in maintenance mode.

should i write a pull request?

No, but I will keep it in mind and ping you when we get there.

if yes, which types would be preferable?

I think i32 sounds like the right choice, but let us know if you think otherwise!

[flumm]

There is currently no timeline for a breaking change. Unfortunately wasm-bindgen is practically not in active development anymore, but in maintenance mode.

that does not sound good, what's the reason? is there a way i can help?

I think i32 sounds like the right choice, but let us know if you think otherwise!

i think for all (realistic) scenarios this should be enough, though there might be a case for f64 since that is more or less what you can give in js ? (though i doubt setting one of the fields to more than 2^31 is necessary or sensible?)

[daxpedda]

There is currently no timeline for a breaking change. Unfortunately wasm-bindgen is practically not in active development anymore, but in maintenance mode.

that does not sound good, what's the reason? is there a way i can help?

There is no specific reason, I guess nobody is getting paid to do it and there is nobody currently available with enough spare time to take it on either. It's the nature of open source!

We are always happy to welcome new contributors, so if you want to help there is always stuff to do. Resolving issues, reviewing PRs and making PRs. You can find us on Discord if you want some pointers on where to start or to just discuss things with us in real time.

I think i32 sounds like the right choice, but let us know if you think otherwise!

i think for all (realistic) scenarios this should be enough, though there might be a case for f64 since that is more or less what you can give in js ? (though i doubt setting one of the fields to more than 2^31 is necessary or sensible?)

Ideally I think we need a more neutral API that only does what bindings are supposed to do and nothing else. E.g. we could just use js_sys::Number here, which would be true to the spec. But until that happens, I think i32 should be fine because I assume any number outside that range wouldn't have any further effect.

[ColonelThirtyTwo]

For completeness, there's other issues with the Date API as well. For example, the arguments to toLocaleString are optional, defaulting to the user's current locale (a useful behavior), but the only binding available, to_locale_string, forces you to pass them.