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 differenceInDays across DST (#533) #1630
Conversation
For reference, BEFORE THIS PATCH the Please note, I have used ISO-8601 timestamps to avoid ambiguity. const { differenceInDays } = require('date-fns')
const a = new Date('2019-10-05T15:00:00.000Z') // for TZ=Australia/Sydney, this is 1 hour before DST
const b = new Date('2019-10-06T14:00:00.000Z') // for TZ=Australia/Sydney, this is 1 day after DST
const c = new Date('2019-10-07T14:00:00.000Z') // for TZ=Australia/Sydney, this is 2 days after DST
console.log(differenceInDays(c, b))
console.log(differenceInDays(b, a))
console.log(differenceInDays(c, a)) NOTE: These results are from before this patch. For 1
1
2 For 1
0
1 |
@kossnocorp anything I need to do to help this? |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Would be cool if you can add a bit info about the case fixed in this PR to the function description?
src/differenceInDays/index.js
Outdated
@@ -42,15 +40,9 @@ export default function differenceInDays(dirtyDateLeft, dirtyDateRight) { | |||
var dateLeft = toDate(dirtyDateLeft) | |||
var dateRight = toDate(dirtyDateRight) | |||
|
|||
var sign = compareAsc(dateLeft, dateRight) | |||
var difference = Math.abs(differenceInCalendarDays(dateLeft, dateRight)) | |||
var result = (dateLeft - dateRight) / 86400000 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please move 86400000
into a var MILLISECONDS_IN_DAY
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@dkozickis moved
What about
|
@dcousens sounds logical to me |
Resolves #533
I'm happy to make changes where necessary, this was simply the simplest solution for the primitives.