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
stringbeans opened this Issue Oct 13, 2017 · 4 comments

Comments

Projects
None yet
4 participants
@stringbeans

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

This comment has been minimized.

Show comment
Hide comment
@HolgerFrank

HolgerFrank Oct 13, 2017

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.

HolgerFrank commented Oct 13, 2017

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

This comment has been minimized.

Show comment
Hide comment
@stringbeans

stringbeans Oct 13, 2017

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

stringbeans commented Oct 13, 2017

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

This comment has been minimized.

Show comment
Hide comment
@Kerumen

Kerumen Oct 19, 2017

Contributor

@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.

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

This comment has been minimized.

Show comment
Hide comment
@maggiepint

maggiepint Oct 19, 2017

Member

Closing in favor of PR.

Member

maggiepint commented Oct 19, 2017

Closing in favor of PR.

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