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

toISOString should not be effectful #38

Open
flip111 opened this issue Feb 8, 2024 · 4 comments
Open

toISOString should not be effectful #38

flip111 opened this issue Feb 8, 2024 · 4 comments

Comments

@flip111
Copy link

flip111 commented Feb 8, 2024

There are a bunch of effectful methods with in the description according to the current machine's date/time. I guess when the machine switches to another timezone or DST the time will be updated.

There are other methods that are not effectful with in description according to UTC.

Description of toISOString https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toISOString

The timezone is always UTC, as denoted by the suffix Z.

IMO this method should be non-effectful like the other UTC methods

@jamesdbrock
Copy link
Member

I took a look at the code and I agree with you. toISOString is a pure function which does not depend on timezone or locale or any other context. Do you want to make a PR?

@jamesdbrock
Copy link
Member

Though, from the git blame I see that @garyb wrote this originally. He's usually pretty thoughtful and deliberate.

https://github.com/purescript-contrib/purescript-js-date/blame/v8.0.0/src/Data/JSDate.purs#L256-L256

Maybe we should wait to see if he wants to explain this.

@garyb
Copy link
Member

garyb commented Feb 9, 2024

It's because it can throw an exception unfortunately, if the date is invalid. I think that's only when the underlying value is like NaN or something though.

@flip111
Copy link
Author

flip111 commented Feb 12, 2024

I assumed this library would provide safe abstraction. I see that it passes the invalid values from JS

(new Date(NaN)).toISOString()
Uncaught RangeError: invalid date
    <anonymous> debugger eval code:1
debugger eval code:1:17
(new Date(NaN)).toString()
"Invalid Date"
(new Date(NaN)).getUTCFullYear()
NaN 

Would be good to add documentation to the library or to relevant functions that it can return invalid outputs. Bit of a missed opportunity to turns those invalid outputs to Nothing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

No branches or pull requests

3 participants