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

Calling moment.utc("2016-07-01").year(2013).toString() provides the wrong date #4238

Closed

Comments

@stringbeans
Copy link

stringbeans commented Oct 13, 2017

Description of the Issue and Steps to Reproduce:

It seems like when setting the date to a date in the past and then setting the year to anything will push the outputted date to be at the end of the month.

If you call moment.utc("2016-07-01").year(2013).toString() you will get Wed Jul 31 2013 00:00:00 GMT+0000. Notice how it is Jul 31 and not Jul 01.

Environment:

I've reproduced this in Chrome 61 as well as in Node 5.10.1

Other information that may be helpful:

This seems to only be an issue if you are using a date from the year 2016. If you try moment.utc("2015-07-01").year(2013).toString() it works fine.

I can confirm that this is not an issue previous to moment 2.19.0+. It only is an issue from 2.19.0+

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)
Thu Oct 12 2017 23:10:36 GMT-0300 (ADT)
10/12/2017, 11:10:36 PM
180
Mozilla/5.0 (Macintosh; Intel Mac OS X 10_12_6) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/61.0.3163.100 Safari/537.36
2.19.1
@HolgerFrank
Copy link

In version 2.19.0 the function set$1() was changed.

Because of this change, setting a new year for a leap year moment, it always sets the last day of the month as date.

moment('2010-02-25').year(2017) // result is: 2017-02-28
moment('2010-10-15').year(2017) // result is: 2017-10-31

Current (incorrect) implementation

function set$1 (mom, unit, value) {
    if (mom.isValid() && !isNaN(value)) {
        if (unit === 'FullYear' && isLeapYear(mom.year())) {
            mom._d['set' + (mom._isUTC ? 'UTC' : '') + unit](value, mom.month(), daysInMonth(value, mom.month()));
        }
        else {
            mom._d['set' + (mom._isUTC ? 'UTC' : '') + unit](value);
        }
    }
}

Because the special case is only needed for th 29th of Februar in leap years, the correct implementation should be:

function set$1 (mom, unit, value) {
    if (mom.isValid() && !isNaN(value)) {
        if (unit === 'FullYear' && isLeapYear(mom.year()) && mom.month() === 1 && mom.date() === 29) {
            mom._d['set' + (mom._isUTC ? 'UTC' : '') + unit](value, 1, daysInMonth(value, 1));
        }
        else {
            mom._d['set' + (mom._isUTC ? 'UTC' : '') + unit](value);
        }
    }
}

Because this is a critical error it should be corrected as soon as possible.

@stringbeans
Copy link
Author

sorry just noticed i wrote "I can confirm that this is not an issue previous to moment 1.19.0+. It only is an issue from 1.19.0+" but I actually meant 2.19... ive updated this in the original post

@Kerumen
Copy link
Contributor

Kerumen commented Oct 19, 2017

@HolgerFrank Would you mind submit a PR with your change to fix this? It's a pretty critical bug which should be fixed asap..

Edit: I submitted the PR, couldn't wait.

@maggiepint
Copy link
Member

Closing in favor of PR.

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