Skip to content
This repository has been archived by the owner on Nov 17, 2022. It is now read-only.

Replace eachMonthOfInterval with timezone-safe local equivalent. #25

Merged

Conversation

soulofmischief
Copy link
Contributor

Fixes #24

@TimDaub
Copy link
Member

TimDaub commented Oct 20, 2021

I was interested in trying to understand the issue at hand and so I set my computer to a time zone on the east coast of the US to try to understand the problem. I'll document it here for posterity.

So in the tests we actually ask to parse a UTC time as the first element of the range:

new Date("2021-01-01T00:00:00.000Z"),

But for that range, when we ask eachMonthOfInterval, we get back "2020-12-01T08:00:00.000Z" in this call:

const months = eachMonthOfInterval({

Curiously, that's because date-fns's eachMonthOfInterval uses setHours and sets it to zero within eachMonthOfInterval as @soulofmischief pointed out: #24 (comment)

Welcome to Node.js v14.18.0.
Type ".help" for more information.
> let d = new Date("2021-01-01T00:00:00.000Z");
undefined
> d.setHours(0, 0, 0, 0);
1609401600000
> d
2020-12-31T08:00:00.000Z
>

It then uses

https://github.com/date-fns/date-fns/blob/f8fb243c55853f90d04efd8933fc291a24402b27/src/eachMonthOfInterval/index.ts#L51

to point to the first day of the month.

I'll merge this PR as it does what we intend to do with this library. But I'm wondering if we should send an issue upstream to date-fns too as this behavior of the method seems rather unexpected.

@TimDaub TimDaub merged commit 93abd7c into rugpullindex:master Oct 20, 2021
1 check passed
@soulofmischief
Copy link
Contributor Author

I also think an upstream issue is a good idea. I'll get that rolling.

@soulofmischief
Copy link
Contributor Author

I looked into it and it appears a UTC version of the library is already in the works.

@TimDaub
Copy link
Member

TimDaub commented Oct 31, 2021

thanks for taking care! 👍🏼

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

Successfully merging this pull request may close these issues.

On PDT timezone, "if labels are shown at right position" test fails with a negative value
2 participants