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

parseDate is incorrect for these inputs #552

Open
ryancwalsh opened this issue May 11, 2024 · 5 comments
Open

parseDate is incorrect for these inputs #552

ryancwalsh opened this issue May 11, 2024 · 5 comments

Comments

@ryancwalsh
Copy link

ryancwalsh commented May 11, 2024

Thank you so much for this library!

I've spent hours looking for a solution, and this is the best I've found. 🎉

It handles most of my use cases.

However, here are some that it didn't handle as I'd hope:

  • 'Fri 11am EDT' (when called on a Saturday, it's showing yesterday, when the principle of least surprise would be for it to show the upcoming Friday)
  • 'tomorrow 8pm America/New_York'
  • '7:30pm Dec 10 Europe/London'

https://stackoverflow.com/questions/78464929/js-version-of-phps-strtotime-to-parse-string-inputs-of-unknown-formats-and-conv

@wanasit
Copy link
Owner

wanasit commented Jun 1, 2024

Hello. I'm sorry for not getting back to you sooner.

'Fri 11am EDT' (when called on a Saturday, it's showing yesterday,

Without the context, the default behavior is guessing the Friday closest to the reference date. Would specifying forwardDate in the parsing options work for you?

'tomorrow 8pm America/New_York'

Do you mean, in addition to the timezone abbreviations, the library use also recognizes the timezone IDs (e.g. "America/New_York", "Europe/London")?

It is possible to add that. I'll add that to the todo list.

@ryancwalsh
Copy link
Author

Yes, if you could rely on a library that always has the updated IANA time zone database, that would be great. https://en.wikipedia.org/wiki/List_of_tz_database_time_zones

Location-based time zones are helpful especially because Daylight Saving is confusing and changes multiple times per year and then even gets redefined occasionally. But if you know where someone is, then you should always be able to convert to their datetime without needing to ask what season it is and whether laws have changed.

As for forwardDate, I see that I wrote a comment for myself:

Consider using the forwardDate option, depending on how it works? https://github.com/wanasit/chrono?tab=readme-ov-file#parsing-options

I hadn't looked into the implementation yet and couldn't tell from the docs whether this is what I want.

I now see it at https://github.com/wanasit/chrono/blob/22b831cd87fe625b0a392fd94848e2a0050d08a0/src/common/refiners/ForwardDateRefiner.ts

So far, my app has always been calling chrono.parseDate(input).

But since I always want "Fri 11am" to mean "Fri in the future 11am" and always want "March 12" to mean "March 12 in the future", it sounds like I should always use { forwardDate: true }.

And that requires that I pass a reference date? So I should always pass the current moment as the reference date?

Thanks.

@wanasit
Copy link
Owner

wanasit commented Aug 10, 2024

Sorry for my slow reply.

And that requires that I pass a reference date? So I should always pass the current moment as the reference date?

Yes. You can just pass the current time or new Date(); but in your unit test or example, you may want to specify a specific reference date to make the output reproducible.

--

Yes, if you could rely on a library that always has the updated IANA time zone database, that would be great. https://en.wikipedia.org/wiki/List_of_tz_database_time_zones

Thanks for sharing.
I will try to add the support for timzone ID (not just abbreviation) when I have time.

@ryancwalsh
Copy link
Author

Thanks!

@olivr70
Copy link

olivr70 commented Aug 17, 2024

Hello,

that would be extremely useful.
I think being able to provide a IANA Timezone in the reference argument of parse() would be great

As possible hints, Intl.supportedValuesOf("timeZone") is a standard way to get an updated list of IANA Timezones. A few zones have 3 segments (like 'America/Argentina/Tucuman' or 'America/Indiana/Marengo'). At the time of writing, there are no duplicates of location names, which could allow accepting only location names only when parsing.

dayjs with the plugins utc and timezone offers great support for timezone operations, including complex cases where reference Date is in standard time, but the result would be in Daylight Saving Time (like adding 2 weeks from a winter time ). dayjs.tz.guess() could also be useful.

My perception is that the ReferenceWithTimezone class could be removed and replaced by a dayjs instance, using tz() when user provides a timezone name and utcOffset() when caller provides a offset to UTC or a timezone abbreviation like "PST" (using numbers from the table in timezone.ts). This table could probably be simplified by removing DST support, which is better provided and updated in dayjs.

This is very probably way too simplifying. I am ready to help on this, but looked at the code and I feel it is too far reaching for me as it may imply modifiying lots of files

Best regards

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

No branches or pull requests

3 participants