rluiten/elm-date-extra

Utils.isoWeek may return wrong results

Closed this issue · 3 comments

I've noticed that Utils.isoWeek may return wrong week number. Here's an example:

> sep12 = Utils.unsafeFromString "2016-9-12"
<Mon Sep 12 2016 00:00:00 GMT+0300 (EEST)> : Date.Date
> Utils.isoWeek sep12
(2016,36,1) : ( Int, Int, Int )
> Utils.isoWeek (Utils.unsafeFromString "2016-9-13")
(2016,37,2) : ( Int, Int, Int )
> Utils.isoWeek (Utils.unsafeFromString "2016-3-21")
(2016,12,1) : ( Int, Int, Int )
> Utils.isoWeek (Utils.unsafeFromString "2016-3-28")
(2016,12,1) : ( Int, Int, Int )

I'm quite sure the issue is daylight saving time. Basically, isoWeek returns wrong week number for all summer time Mondays. (Also here's a good write up on the same problem with JS Date: http://blog.synyx.de/2012/11/properly-calculating-time-differences-in-javascript/). This also affects Period.diff:

> Period.diff (Utils.unsafeFromString "2016-3-21") (Utils.unsafeFromString "2016-3-14")
{ week = 1, day = 0, hour = 0, minute = 0, second = 0, millisecond = 0 }
    : Date.Extra.Period.DeltaRecord
> Period.diff (Utils.unsafeFromString "2016-3-28") (Utils.unsafeFromString "2016-3-21")
{ week = 0, day = 6, hour = 0, minute = 0, second = 0, millisecond = 0 }
    : Date.Extra.Period.DeltaRecord

As far as I know, Elm doesn't have a good support for UTC yet (https://github.com/elm-lang/core/issues/486), so maybe it would be acceptable workaround to use round when calculating day difference between dates for now?

Hi @artbq sorry for the long delay I have a fix for this.
It was just code making bad assumptions.
I just started holidays and have dug into this today.
If you are interested I have pushed the change to github but wont publish to elm till i have a look at the other issue here for dayList in next day or so.

Awesome! Thanks!

I have published the fix along with some other fixes as 8.5.0.
Please let me know if you have any further issues

Thanks for your input @artbq and your comments in the related pull request.