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

updateLocale does not keep the correct predefined locale but overwrites with default (en) #3626

Closed
seriousManual opened this Issue Nov 28, 2016 · 4 comments

Comments

Projects
None yet
3 participants
@seriousManual

seriousManual commented Nov 28, 2016

Description of the Issue and Steps to Reproduce:

I wanted to update the 'monthShort' of the german Locale with the follwoging code:

var moment = require('moment');

moment.updateLocale('de', {monthsShort: ["JAN", "FEB", "MÄR", "APR", "MAI", "JUN", "JUL", "AUG", "SEP", "OKT", "NOV", "DEZ"]});

var date = moment('2011-02-02');

date.locale('de');

console.log(date.format('YYYY MMM MMMM'));

Expected would be an output of '2011 FEB Februar', instead the output is '2011 FEB February'.
When updateing the 'monthsShort' key in the locale definition the resulting locale is not based on the german definition anymore but on the english Definition, hence 'Febraury' instead of 'Februar'.

Environment:

Node 6.3.1 on LInux

Other information that may be helpful:

If you are reporting an issue, please run the following code in the environment you are using and include the output:

console.log( (new Date()).toString())
console.log((new Date()).toLocaleString())
console.log( (new Date()).getTimezoneOffset())
//console.log( navigator.userAgent)
console.log(moment.version)

Mon Nov 28 2016 11:23:45 GMT+0100 (STD)
2016-11-28 11:23:45
-60
//omitted the 'navigator' potion
2.17.0

@seriousManual

This comment has been minimized.

Show comment
Hide comment
@seriousManual

seriousManual Nov 28, 2016

Update:

When the locale that is to be changed has been explicitly used prior to the 'changeUpdate' function call, the update works correctly:

var moment = require('moment');

var date1 = moment('2011-02-02');
date1.locale('de');
console.log(date1.format('YYYY MMM MMMM'));
//output: 2011 Febr. Februar

moment.updateLocale('de', {monthsShort: ["JAN", "FEB", "MÄR", "APR", "MAI", "JUN", "JUL", "AUG", "SEP", "OKT", "NOV", "DEZ"]});

var date = moment('2011-02-02');
date.locale('de');
console.log(date.format('YYYY MMM MMMM'));
//output: 2011 FEB Februar

seriousManual commented Nov 28, 2016

Update:

When the locale that is to be changed has been explicitly used prior to the 'changeUpdate' function call, the update works correctly:

var moment = require('moment');

var date1 = moment('2011-02-02');
date1.locale('de');
console.log(date1.format('YYYY MMM MMMM'));
//output: 2011 Febr. Februar

moment.updateLocale('de', {monthsShort: ["JAN", "FEB", "MÄR", "APR", "MAI", "JUN", "JUL", "AUG", "SEP", "OKT", "NOV", "DEZ"]});

var date = moment('2011-02-02');
date.locale('de');
console.log(date.format('YYYY MMM MMMM'));
//output: 2011 FEB Februar
@seriousManual

This comment has been minimized.

Show comment
Hide comment
@seriousManual

seriousManual Nov 28, 2016

Update:

I looked into this a bit more and came up with a possible solution:
image

Let me know what you think and if I should do a PR.

seriousManual commented Nov 28, 2016

Update:

I looked into this a bit more and came up with a possible solution:
image

Let me know what you think and if I should do a PR.

@icambron

This comment has been minimized.

Show comment
Hide comment
@icambron

icambron Dec 4, 2016

Member

Looks like a bug to me. I can't really comment on your specific proposed fix without spending some more time, but in general we'd take a PR for this.

Member

icambron commented Dec 4, 2016

Looks like a bug to me. I can't really comment on your specific proposed fix without spending some more time, but in general we'd take a PR for this.

@cmyers

This comment has been minimized.

Show comment
Hide comment
@cmyers

cmyers Oct 14, 2017

Contributor

Been looking at bugs to work on and I see this one is still apparent and can still be reproduced. I've refactored the above suggested solution and submitted a PR.

Contributor

cmyers commented Oct 14, 2017

Been looking at bugs to work on and I see this one is still apparent and can still be reproduced. I've refactored the above suggested solution and submitted a PR.

cmyers added a commit to cmyers/moment that referenced this issue Oct 19, 2017

@ichernev ichernev closed this in 9756e86 Nov 11, 2017

ichernev added a commit that referenced this issue Nov 11, 2017

ichernev added a commit that referenced this issue Nov 11, 2017

Merge pull request #4242 from cmyers:develop
[bugfix] updateLocale now tries to load parent, fixes #3626

cmyers added a commit to cmyers/moment that referenced this issue Nov 14, 2017

cmyers added a commit to cmyers/moment that referenced this issue Nov 14, 2017

cmyers added a commit to cmyers/moment that referenced this issue Nov 14, 2017

cmyers added a commit to cmyers/moment that referenced this issue Nov 14, 2017

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment