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.

Sign up for GitHub

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

Jump to bottom

Incorrect day/week calculations across daylight saving boundaries #3519

Open
campbellwmorgan opened this issue Sep 22, 2023 · 7 comments · May be fixed by #3556
Open

Incorrect day/week calculations across daylight saving boundaries #3519

campbellwmorgan opened this issue Sep 22, 2023 · 7 comments · May be fixed by #3556

Comments

@campbellwmorgan
Copy link

I have been debugging a snapshot unit test involving subtracting days from a UTC date. When adding / subtracting days where the result crosses a daylight saving boundary and the timezone of the machine performing this calculation is in a timezone with daylight saving, an extra hour is subtracted to the underlying UTC date.

Example

When the following is executed on a machine with a non-UTC timezone (in my case CEST)

const startDate = new Date(Date.UTC(2021, 10,5)); // .toISOString() = 2021-11-05T00:00:00.000Z
const fourDaysLess = subDays(startDate, 4).toISOString(); // 2021-11-01T00:00:00.000Z
const fiveDaysLess = subDays(startDate, 5).toISOString(); // 2021-10-30T23:00:00.000Z

The expected result of fiveDaysLess should be 2021-10-31T00:00:00.000Z

However, when executed on a machine with UTC timezone, the results for fiveDaysLess returns 2021-10-31T00:00:00.000Z correctly.

Having looked into the implementation of subDays, I see that it uses the setDate and getDate javascript API underneath.

Would subDays and thus addDays, addWeeks, subWeeks not be better implemented by adding / subtracting 1000*60*60*24 to the underlying raw date value?

I guess this is partly an expectation of how the API should work in localised scenarios.
@matejtuke
Copy link

Hello, in which file is this issue located?

@campbellwmorgan
Copy link
Author

Hello, in which file is this issue located?

@matejtuke if you are referring to where the calculation happens: it's here https://github.com/date-fns/date-fns/blob/main/src/addDays/index.ts#L34

@matejtuke
Copy link

can you assign this to me please?

@campbellwmorgan
Copy link
Author

@matejtuke i'm not a mod. I guess the best thing to do would create a pull request and reference this issue

@matejtuke
Copy link

Hi @campbellwmorgan , I guess I found the source of the problem.

You said that when you did the calculation, in the result was an extra hour substracted. I think that the problem is not in the calculations, but in the .toISOString() or .toUTCString().

As you can see in the code bellow, you dont have to do any type of calculation and the result could be displayed in 2 forms.

    const end_of23 = new Date(2023,11,31)
    console.log("End of 2023 > " + end_of23)
    console.log("End of 2023 > " + end_of23.toISOString())
    console.log("End of 2023 > " + end_of23.toUTCString())

and the output:

End of 2023 > Sun Dec 31 2023 00:00:00 GMT+0100 (Central European Standard Time)
End of 2023 > 2023-12-30T23:00:00.000Z
End of 2023 > Sat, 30 Dec 2023 23:00:00 GMT

So, it does not matter if you do calculation or not, the source of the problem is just in the displaying the date and time, which has nothing to do with the actual code of the project.

@campbellwmorgan
Copy link
Author

@matejtuke I'm afraid it's not that.

If you look at the output of my example above, you'll see that I'm only using .toISOString() and that returns the date always as UTC time, not in the local time zone (https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Date/toISOString)

If you execute my above example on a machine on UTC time and then execute the same code on a machine on CET time you will observe a different output.

It's also worth pointing out that you are not using Date.UTC in the Date constructor which means that the date you are creating is localised to your time zone rather than in UTC time which is why when you output the .toISOString() it is showing as an hour earlier because CET time is one hour ahead of UTC in the winter.

@matejtuke
Copy link

Good Point, thanks

matejtuke added a commit to matejtuke/date-fns that referenced this issue Oct 23, 2023
@matejtuke
changes for issue date-fns#3519
4ffc74f
@matejtuke matejtuke linked a pull request Oct 25, 2023 that will close this issue
Open
8 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

solution and changes for issue #3519 matejtuke/date-fns
2 participants
@campbellwmorgan @matejtuke