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

Add string as possible input date arguments. #1836

Open
infacto opened this issue Jun 10, 2020 · 3 comments
Open

Add string as possible input date arguments. #1836

infacto opened this issue Jun 10, 2020 · 3 comments

Comments

@infacto
Copy link

infacto commented Jun 10, 2020

The current type of the date argument of all or the most functions is date: number | Date.
My suggestion: date: number | Date | string. (mostly ISO string or other valid strings for the Date constructor.)

date-fns 2.13.0

@js2me
Copy link

js2me commented Aug 6, 2020

Unfortunately, string type from most functions was removed in v2.0.0-beta+ :(

@ericbiewener
Copy link

ericbiewener commented Feb 3, 2021

Just in case the maintainers are still second guessing their decision, I wanted to chime in here in favor of supporting fully-formed ISO strings. The reason to remove the parseISO function because of file size makes sense, but that problem is solved if only fully formed ISO string parsing is supported, as this comment notes (although unlike that commenter I'd suggest just doing new Date(isoString) rather than any custom parsing at all).

The maintainer's response is essentially that JS will not throw if you give it an invalid string, i.e. new Date('foo') but it will return an Invalid Date that will return NaN on all of its methods. Furthermore, there is no protection against providing the wrong number value as an argument to any date-fn function, i.e. new Date(1) is totally valid but likely not what the developer actually intended. In fact, I'd argue that the invalid ISO string in this case provides a much better indication to the developer that something has gone wrong.

Storing dates as ISO strings is very common, and converting them to numeric timestamps (either at the db level or in memory) just to use with this library is a disappointing requirement.

@fadi-george
Copy link

I agree, allowing for full ISOStrings would be great.

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

No branches or pull requests

4 participants