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

moment.date($day).month($month).year($year) behaves different than moment([$year, $month, $date]) #2725

Closed
ghost opened this issue Nov 2, 2015 · 4 comments

Comments

@ghost
Copy link

ghost commented Nov 2, 2015

The date is parsed differently using these two setter methods.
When setting the date to October 31, 2015. The chained method returns 2015-10-01 while the init method returns 2015-10-31

var year = 2015;
var month = 9; //october
var day = 31;

console.log(moment().date(day).month(month).year(year).format('YYYY-MM-DD'));
//2015-10-01

console.log(moment([year, month, day]).format('YYYY-MM-DD'));
//2015-10-31

JS Fiddle: http://jsfiddle.net/dgnjer7z/

@jakub-g
Copy link

jakub-g commented Nov 3, 2015

Note that this

$('#chained').val(moment().year(2015).month(9).date(31).format('YYYY-MM-DD'));

properly returns 2015-10-31

In fact moment mimics the behavior of JS date object.

First you create moment() that gives you current date (November 3rd as of today). You try to set its day to 31 but November has only 30 days, so JS native date object "intelligently" adds one day and moves date to 1st December (you can set day to any crazy number, like 42, and JS will do an arithmetic by adding a proper number of days and rewinding month and year if needed, not the best practice, it would have been better if it was throwing).

The solution is to always set year, month, day, hour, minute, in this order to avoid issues like this.

@jakub-g
Copy link

jakub-g commented Nov 3, 2015

The fact about overflow handling is indeed documented:

http://momentjs.com/docs/#/get-set/date/

Gets or sets the day of the month.

Accepts numbers from 1 to 31. If the range is exceeded, it will bubble up to the months.

Though the behavior in the corner case like yours is indeed surprising.

@jakub-g
Copy link

jakub-g commented Nov 10, 2015

I opened a PR in documentation repo the explains this common issue
moment/momentjs.com#244

@icambron
Copy link
Member

Yeah, I'm not sure this is fixable. Moment doesn't know that you're going to set the month, so it tries to set the day on the current month, which may or may not have a 31. It's ignorant of your intensions. I think @jakub-g's doc PR is good; let's do that.

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

2 participants