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

startOf/endOf("week") are not locale aware #727

Closed
fuwaneko opened this issue Apr 14, 2013 · 22 comments
Closed

startOf/endOf("week") are not locale aware #727

fuwaneko opened this issue Apr 14, 2013 · 22 comments

Comments

@fuwaneko
Copy link

When I set week.dow to 1 and let's assume it's Saturday 14th April today and I say

moment().startOf("week")

It gives me today instead of Monday 8th April.

http://jsfiddle.net/zQx23/

@mlilley
Copy link

mlilley commented Apr 15, 2013

I concur:

// In Australia, weeks start on Monday (not Sunday)
moment.lang('en', { week: { dow: 1, doy: 4 } });
moment().startOf('week').format('dddd');      // --> "Sunday"  (expecting "Monday")
moment().endOf('week').format('dddd');       // --> "Saturday"  (expecting "Sunday")

Moment appears to be ignoring the attempted configuration change. See: http://jsfiddle.net/PPu8E/2/

It also seems to be ignoring the change when applied as part of the supplied moment/lang/en-gb.js configuration (which includes the above "week" config):

moment.lang('en-gb');
moment().startOf('week').format('dddd');      // --> "Sunday"  (expecting "Monday")
moment().endOf('week').format('dddd');       // --> "Saturday"  (expecting "Sunday")

and:

moment().lang('en-gb').startOf('week').format('dddd');    // --> "Sunday"  (expecting "Monday")

Australia subscribes to ISO 8601, via Australian standard AS ISO 8601-2007, which defines the first day of the week to be Monday. Not sure if there's a case for having an en-au.js language config also, or whether you would recommend we just use en-gb.

@fuwaneko
Copy link
Author

I think the problem is in day() function which is not locale-aware. I changed it in my local copy of moment.js, but I'm not sure if it's the right thing to do. Also, weekdays, weekdaysShort and weekdaysMin must be changed in language options (maybe in Language.prototype) or date will be messed like "Sunday 8th April".

I'm actually really surprised that this issue was not mentioned earlier by someone.

@mlilley
Copy link

mlilley commented Apr 15, 2013

Thanks! I ran into the issue trying to get the sunday of last week (for any moment()), and wanted it to be the Sunday irrespective of timezone.

moment().substract('weeks', 1).endOf('week')   // saturday ... fail

Your comment helped me workaround the issue by pointing me towards the day() function, which I'd overlooked:

moment().subtract('weeks', 1).day(7) [.endOf('day')]    // last sunday, [23:59:59] ... win!

@fuwaneko
Copy link
Author

There is actually a pull request for this bug, but it does not work properly. The correct solution was provided in comments to pull request.

@timrwood
Copy link
Member

Related to #634 and #613.

This comment outlines my proposed fix, but I haven't had time to write it yet. #613 (comment)

@timrwood
Copy link
Member

This is fixed via a58e5b7, it should go out in the 2.1.0 release. See #634 for the full discussion.

timrwood added a commit that referenced this issue May 20, 2013
* 'develop' of github.com:timrwood/moment: (53 commits)
  Use brackets to escape characters
  Add composer.json
  fix test and some translations
  Update it.js
  Update br.js
  Fixing indentation
  Fixing indentation
  Fixing indentation
  making startOf('week') use locale week start #727 #634 #699
  adding isDST teste
  fixing zone getter and setter test
  Updated minified files
  Added mutations for years
  Format function now uses "instanceof" on a var rather than "typeof" on that var's call to determine if the variable is a function. This fixes an incompatibility with ClojureScript, which defines String.prototype.call as a function.
  Corrected Ukrainian and optimized Russian
  toJSON should not call lang.postformat
  set hook to add ordinal for Chinese(cn && tw) with modifing the tests
  Update nn.js
  Update nn.js
  Better tests for (iso)weekYear and (iso)weekday and formatting
  ...
@MaestroJurko
Copy link

I updated from 2.0 to 2.1, tried 2.7 and this issue is still present.

moment.lang('en', {
    week: {
        dow: 1 // Monday is the first day of the week.
    }
});

moment().startOf('week').format('dddd'); // Sunday

I cannot update the library to much, the project is 1 year old.
??

@emamirazavi
Copy link

dow does not work correctly! I have problem too!!! my version is 2.8.4
I set it to 6 to start weeks with Sat but it starts with Mon!!! Any solution my experts?!

@algesten
Copy link

same here. 2.8.4

moment(Date.now()).lang('sv',{week:{dow:1}}).startOf('week').format('ddd')
...
"Sun"

@emamirazavi
Copy link

my problem was about bootstrap datepicker and it does not relate to moment

@kimmobrunfeldt
Copy link

I'm using moment 2.8.4 and setting locale works for me:

$ node
> var moment = require('moment');
undefined
> console.log('Version', moment.version);
Version 2.8.4
undefined
> moment.locale('en', {
>     week: {
>         dow: 1 // Monday is the first day of the week.
>     }
> });
'en'
> moment().startOf('week').format('dddd');
'Monday'
>

It also seems that .lang is now deprecated:

> moment.lang('en', {
>     week: {
>         dow: 1 // Monday is the first day of the week.
>     }
> });
Deprecation warning: moment.lang is deprecated. Use moment.locale instead.

@sahanDissanayake
Copy link

@kimmobrunfeldt
Guys this solution seems to break, the following code to get the current week number

moment().week() // NaN 

EDIT

You have to add doy: 6 as default to make it work

@uriklar
Copy link

uriklar commented Mar 20, 2016

@sahanDissanayake Could you please explain why you set doy to 6? The docs say it should be set to 4

@sahanDissanayake
Copy link

oh mate I'm not sure, At the time it seemed to get the job done which is to make Monday the starting day of the week, I have moved on to other projects now.

@sahanDissanayake
Copy link

@sahanDissanayake
Copy link

sahanDissanayake commented Jul 25, 2016

is this solved ??
Im using v2.13.0

I tried to do moment.weekdays() but still the start day is Sunday.. it should have been Monday

@RomainSF
Copy link

RomainSF commented Aug 5, 2016

this is ridiculous, can someone fix this?

@fuwaneko
Copy link
Author

fuwaneko commented Aug 5, 2016

@RomainSF @sahanDissanayake I think you should file separate bug for moment.weekdays() since startOf/endOf("week") work properly.

@offirpeer
Copy link

offirpeer commented Oct 31, 2016

I am using version : 2.15.2 and

moment().subtract(1, 'weeks').startOf('week') return sunday , and when I change my ip to united states I still get sunday when I expect to get monday.
any solution for that?

@butterflyhug
Copy link
Contributor

@offirpeer Moment supports a wide range of locales, but you will need to explicitly select the locale that is most appropriate for your application in a particular location. A bunch of folks use moment.locale(window.navigator.language) to autodetect an appropriate locale in most circumstances, but that's based on the locale settings of the user's computer and browser -- not their IP address.

Also, note that .startOf('week') is Sunday in the default U.S. locale -- U.S. calendars generally print Sunday as the first day of the week. If none of the built-in locale definitions will meet your needs, you can always create a custom locale (as described in the docs) and then select that for use on your site.

Please open a new issue or reach out on Gitter if you still have questions.

@efroim102
Copy link

@offirpeer Use 'isoweek' instead of 'week' if your week starts on Monday.

@seltix5
Copy link

seltix5 commented Mar 25, 2018

Hello,
I found this problem too, in my case I wanted the startOf Day using moment-with-locales.min.js 2.21.0

var a = moment("2018-03-25T22:24:03+01:00");
a.format()
 // 2018-03-25T22:24:03+01:00

var b = a.startOf('day');
b.format()
 // 2018-03-25T00:00:00+00:00

after using "startOf" the timezone is set to zero.
I reported the same problem here : moment/moment-timezone#208

UPDATE : this was not a error! it is correct since there was the hour change at this date. Credits to @reinhrst.
moment/moment-timezone#208 (comment)

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

No branches or pull requests