2.7.0: Invalid date output when passing a tzid

[fknop]

Reporting an issue

Thank you for taking an interest in rrule! Please include the following in
your report:

• Verify that you've looked through existing issues for duplicates before
  creating a new one
• Code sample reproducing the issue. Be sure to include all input values you
  are using such as the exact RRule string and dates.
• Expected output
• Actual output
• The version of rrule you are using: 2.7.0
• Your operating system: osx
• Your local timezone (run $ date from the command line
  of the machine showing the bug)

Hello,

There seem to be an issue in the browser version of RRule when tzid is passed.
I managed to pinpoint the issue to this line of code https://github.com/jakubroztocil/rrule/blob/master/src/datewithzone.ts#L39
This returns Invalid Date in a browser but works in NodeJS

Example in a browser (latest firefox, I tried latest Chrome as well):

Example in Node:

[db82407]

Hi,

I'm seeing this issue using rrule.between() with a tzid.

Here's a test-case that shows the problem (I added it to test/rrule.test.ts):

  testRecurring(
    'testBetweenWithTZ',
    {
      rrule: new RRule({
        freq: RRule.WEEKLY,
        dtstart: parse('20220613T090000'),
        byweekday: [RRule.TU],
        tzid: 'Europe/London'
      }),
      method: 'between',
      args: [parse('20220613T093000'), parse('20220716T083000')],
    },
    [
      datetime(2022, 6, 14, 9, 0),
      datetime(2022, 6, 21, 9, 0),
      datetime(2022, 6, 28, 9, 0),
      datetime(2022, 7, 5, 9, 0),
      datetime(2022, 7, 12, 9, 0),
    ]
  )

yarn test fails:

  379 passing (1s)
  9 pending
  1 failing

  1) RRule
       testBetweenWithTZ [every week on Tuesday] [DTSTART;TZID=Europe/London:20220613T090000
RRULE:FREQ=WEEKLY;BYDAY=TU]:

      number of recurrences
      + expected - actual

      -7
      +5

      at assertDatesEqual (test/lib/utils.ts:12:39)
      at Context.<anonymous> (test/lib/utils.ts:84:9)
      at processImmediate (node:internal/timers:466:21)

The actual output of rrule.between() is shown below:

    const myrule = new RRule({
        freq: RRule.WEEKLY,
        dtstart: parse('20220613T090000'),
        byweekday: [RRule.TU],
        tzid: 'Europe/London'
      });

  console.log('myrule', myrule.between(parse('20220613T093000'), parse('20220716T083000')))

myrule [
  Invalid Date,
  Invalid Date,
  Invalid Date,
  2022-07-05T09:00:00.000Z,
  2022-07-12T09:00:00.000Z,
  Invalid Date,
  Invalid Date
]

This issue was introduced by this commit: cf76af3 "implement rezonedDate without luxon"

It appears to be caused when the output of date.toLocaleString() is day/month/year rather than month/day/year.
This can be fixed by specifing 'en-US' as the locale rather than undefined, as shown in this diff:

diff --git a/src/datewithzone.ts b/src/datewithzone.ts
index 74e65c5..8dd4149 100644
--- a/src/datewithzone.ts
+++ b/src/datewithzone.ts
@@ -32,8 +32,8 @@ export class DateWithZone {
     }

     const localTimeZone = Intl.DateTimeFormat().resolvedOptions().timeZone
-    const dateInLocalTZ = new Date(this.date.toLocaleString(undefined, { timeZone: localTimeZone }))
-    const dateInTargetTZ = new Date(this.date.toLocaleString(undefined, { timeZone: this.tzid ?? 'UTC' }))
+    const dateInLocalTZ = new Date(this.date.toLocaleString('en-US', { timeZone: localTimeZone }))
+    const dateInTargetTZ = new Date(this.date.toLocaleString('en-US', { timeZone: this.tzid ?? 'UTC' }))
     const tzOffset = dateInTargetTZ.getTime() - dateInLocalTZ.getTime()

     return new Date(this.date.getTime() - tzOffset)

I hope that helps,

--
Derek

[Rhysjc]

Seeing this too in node 14.19.1

[dbrody2004]

Seeing this in node 14.18.0

[zeluisping]

Getting Invalid Date with node v14.16.0 and rrule v2.7.1, using tzid: 'Europe/London' when calling between:

(byweekday is [ RRule.WE ])

Local timezone is set to UTC (TZ=UTC). No Invalid Date are returned when not using tzid.
The valid dates are correct (as expected returns the dates in the local timezone)

The results are the same when using .all() with dtstart and until in the rule options.

This here works tho:

console.log(
  new Date(new Date().toLocaleString(undefined, { timeZone: 'Europe/London' }))
)
// output: 2022-11-07T14:25:59.000Z

Workaround

Switching to rrule@2.6.9 works (here with missing Luxon that is failing to be found, even when installed):

To avoid the luxon warning rrule@2.6.4 can be used instead, see #427

[hucki]

I am having the same issue with rrule@2.7.1 in a node v14.18.3 environment.

using the following config:

  freq: RRule.WEEKLY,
  interval: 1,
  tzid: 'Europe/Amsterdam',
  count: 10,
  dtstart: getNewUTCDate(currentEvent.startTime), // returns a `new Date()` derived from a `Date.UTC()`.

As workaround for my usecase I am currently leaving out the timezone tzid: 'Europe/Amsterdam'.
This fixes this issue for now.

[Stf-F]

Same problem here with node v16.13.1 and rrule@2.7.1.

const rruleOne =
  "DTSTART;TZID=Europe/London:20220822T060500\n" +
  "RRULE:FREQ=WEEKLY;INTERVAL=1;BYDAY=MO;UNTIL=99991230T000000";

const ruleOne = RRule.fromString(rruleOne);
const dates = ruleOne.between(
  new Date(
    Date.UTC(
      new Date().getFullYear(),
      new Date().getMonth(),
      new Date().getDay()
    )
  ),
  new Date("2023-11-01")
);

Downgrading to rrule@2.6.9 while waiting for the fix to be merged.

[davidgoli]

My apologies for letting this sit so long, I needed to take a break from this project for a while as I no longer rely on it for work. But I feel the pain in this thread, can confirm it is an actual bug, and want to work toward a solution.

It looks like the common theme here is everyone on this thread, as far as I can tell, is working from Europe, is that correct? I am unable to reproduce this in the US - whether in Node 14, 16, or in latest Chrome (109). This would be consistent with the fact that my system's default locale is indeed en-US. What I'm not clear on is why new Date().toLocaleString(undefined, { timeZone: 'Europe/London' }) would return a month/day/year formatted string with undefined passed as a locale, since this format isn't widely used outside the US (as a US-ian, my apologies for this as well as for the imperial system - we aren't helping things!). My assumption would be that with undefined, the runtime should format the date string correctly for the system locale, which might vary from locale to locale but should always be a valid Date string. Apparently, this is not the case.

The ideal solution here would be to get a timezoned-ified date as an ISO8601 string, but I'm not finding the native JS APIs to be very helpful here: toISOString doesn't accept a timeZone argument or option, and toLocaleString appears to be designed to format user-facing strings and thus has no option to format an ISO string. This is unfortunate, since these strings are not guaranteed to be compatible with the built-in Date constructor:

Note: When parsing date strings with the Date constructor (and Date.parse, they are equivalent), always make sure that the input conforms to the ISO 8601 format (YYYY-MM-DDTHH:mm:ss.sssZ) — the parsing behavior with other formats is implementation-defined and may not work across all browsers. Support for RFC 2822 format strings is by convention only.
https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/Date

Regardless, I want to make sure any implementation that works in one locale will work in all locales (this is clearly not currently the case). It looks like we can use the LANG env var to override the system locale - similar to how one can use TZ to set the time zone, eg:

$ TZ=UTC yarn test
$ LANG=en-GB yarn test

So, to sum up, there's an incompatibility between the Intl APIs (which toLocaleString is built on) and the Date API. The former is currently the only way to convert between time zones, while the latter is the only way to create date instances from strings.

I can thus now see why hardcoding en-US would be an appealing solution (rather than using the system default locale). I think this is in the right direction - we want the locale to be hardcoded - but I'm not convinced en-US is the right answer. Rather, I'd like to try getting the format closer to an ISO string before passing in to new Date() because it seems risky to rely on the "implementation-defined" behavior of the Date constructor parsing US-formatted date strings. Perhaps an more resilient approach might use a ISO-like locale, such as sv-SE:

> new Date().toLocaleString("sv-SE")
'2023-02-07 10:41:36'

or even:

> new Date().toLocaleString("sv-SE").replace(' ', 'T')+'Z'
'2023-02-07T10:42:50Z'

Thoughts?

[db82407]

Hi David, Thanks for finding time to look at this. I agree with all your analysis. My fix used ‘en-US’ as it’s likely that any implementation of new Date() will parse dates in US format - although I recognise that this is not guaranteed. So creating an ISO8601 date is better. Using the ’sv-SE’ locale makes this easier as the format is almost correct. If you’d rather not rely on the ’sv-SE’ date format, you can use Intl.DateTimeFormat() with ‘en-US’ (which is used by Date.toLocaleString()), but it’s more verbose:

fmt = new Intl.DateTimeFormat('en-US', {year: 'numeric', month: '2-digit', day: '2-digit', hour: '2-digit', minute: '2-digit', second: '2-digit', hour12: false}).format(new Date())

'02/08/2023, 12:02:03'

regex = /(\d+)\/(\d+)\/(\d+), ([\d:]+)/

/(\d+)\/(\d+)\/(\d+), ([\d:]+)/

fmt.replace(regex, "$3-$1-$2T$4Z")

'2023-02-08T12:02:03Z' Do you want me to update my PR using either of these approaches? Thanks, — Derek

…

On Feb 7, 2023, at 6:43 PM, David Golightly ***@***.***> wrote: My apologies for letting this sit so long, I needed to take a break from this project for a while as I no longer rely on it for work. But I feel the pain in this thread, can confirm it is an actual bug, and want to work toward a solution. It looks like the common theme here is everyone on this thread, as far as I can tell, is working from Europe, is that correct? I am unable to reproduce this in the US - whether in Node 14, 16, or in latest Chrome (109). This would be consistent with the fact that my system's default locale is indeed en-US. What I'm not clear on is why new Date().toLocaleString(undefined, { timeZone: 'Europe/London' }) would return a month/day/year formatted string with undefined passed as a locale, since this format isn't widely used outside the US (as a US-ian, my apologies for this as well as for the imperial system - we aren't helping things!). My assumption would be that with undefined, the runtime should format the date string correctly for the system locale, which might vary from locale to locale but should always be a valid Date string. Apparently, this is not the case. The ideal solution here would be to get a timezoned-ified date as an ISO8601 string, but I'm not finding the native JS APIs to be very helpful here: toISOString doesn't accept a timeZone argument or option, and toLocaleString appears to be designed to format user-facing strings and thus has no option to format an ISO string. This is unfortunate, since these strings are not guaranteed to be compatible with the built-in Date constructor: Note: When parsing date strings with the Date constructor (and Date.parse, they are equivalent), always make sure that the input conforms to the ISO 8601 format (YYYY-MM-DDTHH:mm:ss.sssZ) — the parsing behavior with other formats is implementation-defined and may not work across all browsers. Support for RFC 2822 <https://datatracker.ietf.org/doc/html/rfc2822> format strings is by convention only. https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/Date Regardless, I want to make sure any implementation that works in one locale will work in all locales (this is clearly not currently the case). It looks like we can use the LANG env var to override the system locale - similar to how one can use TZ to set the time zone, eg: $ TZ=UTC yarn test $ LANG=en-GB yarn test So, to sum up, there's an incompatibility between the Intl APIs (which toLocaleString is built on) and the Date API. The former is currently the only way to convert between time zones, while the latter is the only way to create date instances from strings. I can thus now see why hardcoding en-US would be an appealing solution (rather than using the system default locale). I think this is in the right direction - we want the locale to be hardcoded - but I'm not convinced en-US is the right answer. Rather, I'd like to try getting the format closer to an ISO string before passing in to new Date() because it seems risky to rely on the "implementation-defined" behavior of the Date constructor parsing US-formatted date strings. Perhaps an more resilient approach might use a ISO-like locale, such as sv-SE: > new Date().toLocaleString("sv-SE") '2023-02-07 10:41:36' or even: > new Date().toLocaleString("sv-SE").replace(' ', 'T')+'Z' '2023-02-07T10:42:50Z' Thoughts? — Reply to this email directly, view it on GitHub <#523 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAKLLOTSTXAKMR67SVXFSSTWWKJV3ANCNFSM5YUF5WGA>. You are receiving this because you commented.

[davidgoli]

I don't see a reason not to trust sv-SE; if the format changes dramatically, a lot of things will break & we'll have to scramble to fix it, but that seems remote. So yes, please update to use sv-SE as you've outlined here. Thanks!

Also: it would be so nice if toLocaleString supported ISO8601 directly out of the box. Feels like a really half-baked feature without that.

[db82407]

So yes, please update to use sv-SE as you've outlined here. Thanks!

PR updated. It's awaiting approval to run workflows.