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

fix(dayjs.add): support daylight saving time #319

Merged
merged 3 commits into from
Sep 26, 2018
Merged

Conversation

farnabaz
Copy link
Contributor

It's not safe to add 24 hours to manipulate date, There are 2 days of the year that do not have 24 hours
In order to get rid of this issue its good to use build-in setDate
See: https://en.wikipedia.org/wiki/Daylight_saving_time

It's not safe to add 24 hours to manipulate date, There are 2 days of the year that do not have 24 hours
In order to get rid of this issue its good to use build-in `setDate`
See: https://en.wikipedia.org/wiki/Daylight_saving_time
@codecov-io
Copy link

codecov-io commented Aug 31, 2018

Codecov Report

Merging #319 into master will not change coverage.
The diff coverage is 100%.

Impacted file tree graph

@@          Coverage Diff          @@
##           master   #319   +/-   ##
=====================================
  Coverage     100%   100%           
=====================================
  Files          37     37           
  Lines         403    407    +4     
  Branches       53     55    +2     
=====================================
+ Hits          403    407    +4
Impacted Files Coverage Δ
src/index.js 100% <100%> (ø) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update d0b1419...7677327. Read the comment docs.

@prantlf
Copy link
Contributor

prantlf commented Sep 15, 2018

I wonder, what is the expected behaviour, if you add one day. The day, when the daylight-saving change occurs, is shorter or longer. Do you want to just flip the day number, or do you want to consider the right hours? You may need to wait for the "UTC mode" (#326), which would "only flip the day number". Day.js is not a "date only" type. There is a time part in the instance too.

This is what happened with Day.js Node.js in Central Europe this year. The day had 23 hours and both adding 1 day and 24 hours ended well:

> original = dayjs('2018-03-25')
'2018-03-24T23:00:00.000Z'
> original.toDate().toLocaleString()
'2018-3-25 00:00:00'
> tomorrow = original.add(1, 'day')
'2018-03-25T22:00:00.000Z'
> tomorrow.toDate().toLocaleString()
'2018-3-26 00:00:00'
> tomorrow24 = original.add(24, 'hour')
'2018-03-25T23:00:00.000Z'
> tomorrow24.toDate().toLocaleString()
'2018-3-26 01:00:00'

And just one day later, everything was simpler:

> original = dayjs('2018-03-26')
'2018-03-25T22:00:00.000Z'
> original.toDate().toLocaleString()
'2018-3-26 00:00:00'
> tomorrow = original.add(1, 'day')
'2018-03-26T22:00:00.000Z'
> tomorrow.toDate().toLocaleString()
'2018-3-27 00:00:00'
> tomorrow24 = original.add(24, 'hour')
'2018-03-26T22:00:00.000Z'
> tomorrow24.toDate().toLocaleString()
'2018-3-27 00:00:00'

@iamkun iamkun merged commit 969aced into iamkun:master Sep 26, 2018
@iamkun
Copy link
Owner

iamkun commented Sep 26, 2018

🎉 This PR is included in version 1.7.7 🎉

The release is available on:

Your semantic-release bot 📦🚀

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

Successfully merging this pull request may close these issues.

None yet

4 participants