spencermountain / spacetime

A lightweight javascript timezone library
http://spacetime.how/
Other
3.99k stars 183 forks source link

2019 January 14 is the first week? #167

Closed csabaxyz closed 5 years ago

csabaxyz commented 5 years ago

Hey,

I just found about about your lib, and wanted to have a quick test, but I think right away found a bug:

spacetime('2019-01-14').week() === 1

The second week starts from January 15.

It looks like it is calculated here: https://github.com/spencermountain/spacetime/blob/5c00606869af652d7db6a5a88fe813d0ba3f2963/src/methods/query/03-year.js#L72-L82

If I take out your optimization:

    if (tmp.epoch > thisOne) {
      return 1
    }

Then it would return 0.

So, I'm a bit confused whether this should start from zero or one. Before the optimization it might've been zero indexed. And if I understand correctly this one counts the first 7 days as the first week, instead of changing on either Sunday or Monday, like calendars usually do.

Let me know if I can help somehow.

spencermountain commented 5 years ago

ah sorry @csabaxyz , i'm not sure how i missed this issue a few weeks ago. sorry about that. Thank you for digging into it. You're absolutely right.

I have a head-cold and just spent some time looking at it and now am very confused. I'll look at this next week, when i am 100% cheers

spencermountain commented 5 years ago

hey, i've fixed this in 6.3.0, the .week() outputs look like this now:

2018:
1
1
1
1
1
1
1
--
2
2
2

2019:
1
1
1
1
1
1
---
2
2
2
2
2
2
2
---
3
3
3

this seems to be the expected behaviour, but i have a feeling there are more bugs here. I'll close this issue, but let me know if you can spot any. thanks.