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

Problem with parsing Unixtime format #1075

Closed
allaud opened this issue Sep 13, 2013 · 6 comments

Comments

Projects
None yet
4 participants
@allaud
Copy link
Contributor

commented Sep 13, 2013

This problem is:

moment('1371065286', ['X']).isValid()
false
@Xotic750

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2013

Certainly looks fishy. I tested with v2.2.0

var unixTime = (new Date().getTime() / 1000).toFixed(3),
    date = moment.utc(unixTime, "X");

console.log(unixTime, date.toISOString(), date.isValid());

Gave an output

1379066897.157 2013-09-13T10:08:17.157Z false 

jsfiddle

The documentation for String + Format states:

If the moment that results from the parsed input does not exist, moment#isValid will return false.

@allaud

This comment has been minimized.

Copy link
Contributor Author

commented Sep 13, 2013

@Xotic750 so what should I do to fix the issue?

@Xotic750

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2013

Well you have opened a ticket for a developer to take a look, but you could always dive into the code yourself and try to debug and fix the problem. I just took a look because I would have thought this usage would have been well tested and function without issue, but as you can see, I saw the same issue that you described.

@icambron

This comment has been minimized.

Copy link
Member

commented Sep 13, 2013

Yup, that's a bug. Note that the bug is in isValid(), not in parsing the date, and moment objects you create this way should work fine. So if you know you have a valid ISO string, you're set. isValid() is fairly new, and we're still seeing a few issues with it.

On the bug itself, the issue is that we normally expect getLangDefinition() to populate the _a array, but in the case of Unix timestamps, we just set the date directly on the moment. That leaves _a as an empty array. In turn, that mixes up isValid(), which does this:

if (this._a) {
    this._isValid = !compareArrays(this._a, (this._isUTC ? moment.utc(this._a) : moment(this._a)).toArray());
} else {

I suspect that the best fix is to undefine the array if parsing the input string yields an actual date, somewhere around line 900.

@Xotic750

This comment has been minimized.

Copy link
Contributor

commented Sep 13, 2013

How about:

       case 'X':
            config._d = new Date(parseFloat(input) * 1000);
            config._isValid = !isNaN(config._d.getTime());
            break;
@icambron

This comment has been minimized.

Copy link
Member

commented Sep 13, 2013

You're right. Let's do that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.