isoWeekday(String) inconsistent with isoWeekday(Number) on Sunday

[nvdh]

isoWeekday(7) gives me Sunday of this iso week (monday-sunday)
isoWeekday("Sunday") gives me the Sunday of previous iso week

version: https://cdnjs.cloudflare.com/ajax/libs/moment.js/2.10.6/moment.min.js

[icambron]

That's correct. This Sunday is isoWeekday(0) and "Sunday" is an alias for 0, not 7.

[nvdh]

I figured that's how it's implemented and understand what you mean now, but the result was a bit counter-intuitive from my point of view. The documentation says:

Gets or sets the ISO day of the week with 1 being Monday and 7 being Sunday.
moment().isoWeekday(1); // Monday
moment().isoWeekday(7); // Sunday

So in an ISO week, 7 is always Sunday. The documentation of the alias says:

As of 2.1.0, a day name is also supported. This is parsed in the moment's current locale.
moment().day("Sunday");

So with those two pieces of information it might be a little bit confusing when you do:

moment().isoWeekday("Sunday")

I would expect it to give the same result as moment().isoWeekday(7).

Apart from that, if you say "Sunday" is an alias for 0, this might conflict with the documentation here:

If the locale assigns Monday as the first day of the week, moment().weekday(0) will be Monday. If Sunday is the first day of the week, moment().weekday(0) will be Sunday.

Not trying to brag, just want to explain why this was unclear for me. Maybe a note in the documentation stating that "sunday" is an alias for 0 would be useful.

[icambron]

Fair point. It could be that it should be an alias for 7 or that 0 vs 7 should behave differently. I'm not immersed enough in the details to be sure what the right thing to do is, so I'll just leave this open.

[mattjohnsonpint]

Taking a look at this again. The docs only talk about being able to pass "Sunday" with regard to the day function, not weekday or isoWeekday. They should probably be updated.

Then, I'd agree that for isoWeekday, the "Sunday" alias should be changed to match 7, not 0. That it currently matches 0 is a bug, since that's the whole point of the isoWeekday function.

[mattjohnsonpint]

A PR would be welcome. First, submit a PR to fix the behavior of the isoWeekday function. Then, submit a separate PR to https://github.com/moment/momentjs.com with the documentation change.

[mmonirul]

I'm trying to generate date via year, week and day number i.e let date = moment().year(2019).isoWeek(1).isoWeekday(1).toDate(); //Mon Dec 31 2018 11:56:45 GMT+0100 (Central European Standard Time)

the problem i'm facing when i want to get year and week from generated date moment(date).year() | moment(date).isoWeek() I got the wrong. NOT SURE HOW TO SOLVE THIS