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

weekOfYear week edge cases with 2015-12-28 and 2016-10-30 #772

Closed
kwypchlo opened this issue Jan 7, 2020 · 7 comments · Fixed by #769
Closed

weekOfYear week edge cases with 2015-12-28 and 2016-10-30 #772

kwypchlo opened this issue Jan 7, 2020 · 7 comments · Fixed by #769
Labels

Comments

@kwypchlo
Copy link

kwypchlo commented Jan 7, 2020

Describe the bug
weekOfYear week edge cases do not match moment week calculations

expect(dayjs('2015-12-28').week()).toBe(moment('2015-12-28').week())
// dayjs: 1 , moment: 53

expect(dayjs('2016-10-30').week()).toBe(moment('2016-10-30').week())
// dayjs: 44, moment: 43

Expected behavior
dayjs should return correct week of year, according to http://www.whatweekisit.org/ moment is correct

Information

  • Day.js Version [1.8.19]
  • OS: [Mac OS]
  • Browser [running as unit test on Node 12.14.0]
  • Time zone: [CET]
@iamkun
Copy link
Owner

iamkun commented Feb 4, 2020

🎉 This issue has been resolved in version 1.8.20 🎉

The release is available on:

Your semantic-release bot 📦🚀

@iamkun iamkun added the released label Feb 4, 2020
@kwypchlo
Copy link
Author

kwypchlo commented Feb 4, 2020

@iamkun this one is still not working properly, 1.8.20 did not fix it

@iamkun
Copy link
Owner

iamkun commented Feb 4, 2020

It should be fixed.

the test date, version, and locale, please?

@kwypchlo
Copy link
Author

kwypchlo commented Feb 4, 2020

@iamkun it's current version "dayjs": "^1.8.20"

dayjs@^1.8.20:
  version "1.8.20"
  resolved "https://ui.nexus.crint.net/repository/npm-proxy/dayjs/-/dayjs-1.8.20.tgz#724a5cb6ad1f6fc066b0bd9a800dedcc7886f19e"
  integrity sha512-mH0MCDxw6UCGJYxVN78h8ugWycZAO8thkj3bW6vApL5tS0hQplIDdAQcmbvl7n35H0AKdCJQaArTrIQw2xt4Qg==
> dayjs.locale()
'en'
> dayjs.utc('2015-12-28T18:59:32').week()
1 // should be 53
dayjs.utc('2016-10-30T09:03').week()
45 // should be 43

@iamkun
Copy link
Owner

iamkun commented Feb 5, 2020

@kwypchlo test with moment.js and got the same result https://runkit.com/embed/5bhn5xbhf6ks
Maybe you should use the correct locale that week starting at monday

@kwypchlo
Copy link
Author

kwypchlo commented Feb 5, 2020

@iamkun you are right, sorry for the confusion! Thank you for your work!

@4rno
Copy link

4rno commented Jul 8, 2020

Hi,
I still an issue with the week() method compared with the moment.js one.
Here is an example of what moment.js week() method return for fr and en-gb locales.

I tried with that date: 2020-12-31, which is supposed to be a day in the week number 53. (if i refer to this http://www.whatweekisit.org/ or any french calendar)

moment.js returns week 53 : https://runkit.com/4rno/5f05ee0eef35c50013368d1b

I have different results with dayjs: https://runkit.com/4rno/5f05ebfc841930001389a53e
In fr locale, it returns week number 1. But in en-gb it returns correctly week 53.

It seems to be linked with the parameter yearStart in locale config files. But i don't know exactly how it is supposed to work.

Maybe i'm wrong somewhere.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants