Unix timestamps are parsed incorrectly when using 'x' or 'X' with moment.tz

[joekrill]

When parsing a unix timestamp (milliseconds since the epoch), moment.tz seems to parse the timestamp, then modify it by applying an offset? At least I think that's what is happening. Example:

moment.tz(1464194866106, 'x', true, 'America/New_York').format('x');

I would expect this to simply output the original timestamp, but it actually outputs 1464209266106, which is a 240 minute difference (or, the timezone offset for the given timezone).

On the other hand, this:

moment(1464194866106, 'x', true).tz('America/New_York').format('x');

outputs the correct and expected value of 1464194866106. It's late, so I feel like I must be missing something here. But a timestamp is a timestamp -- so parsing it in the context of a timezone shouldn't change the underlying timestamp value in any way..

[mattjohnsonpint]

Indeed, changing the time zone, mode, or offset should not change the underlying timestamp.
This bug appears to only occur when parsing using the 'x' or 'X' formats with moment.tz.

Keep in mind that if the input is numeric, then you can use this form, which works:

moment.tz(1464194866106, 'America/New_York')

Likewise, the .valueOf() function will give the numeric timestamps without going through formatting.

[joekrill]

It looks like a fairly quick fix would be to alter the needsOffset function from:

function needsOffset (m) {
    return !!(m._a && (m._tzm === undefined));
}

to:

function needsOffset (m) {
    return !!(m._a && (m._tzm === undefined) && m._f !== 'x' && m._f !== 'X');
}

to check the incoming format for those specific values. I believe that's what the _f property contains.

While that works, it doesn't seem like the "correct" approach. I'm not familiar enough with the purpose of the _a property, though, or why it is being checked in the way it is here. As far as I can tell, it is set to an array of individual time components. It gets set whenever a moment is created from an initial value, but it is undefined when creating a "Now" moment (i.e. simply calling moment() or moment.tz('America/New_York)). In the case of an 'X' or 'x' formatted input string, though, it contains an array with all undefined values:

[ , , , undefined ]

So perhaps the correct approach -- instead of checking _f for specific value -- would be to check that _a not only exists, but also contains a value?

/** Checks that an array contains at least one value which 
  * is not undefined. Is there a better way to do this and still 
  * support IE8? */
function arrayHasValue(a) {
    if(!a) {
        return false;
    }

    for (var i = 0; i < a.length; i++ ) {
        if(typeof a[i] !== 'undefined') {
            return true;
        }
    }

   return false;
}

function needsOffset (m) {
    return !!(arrayHasValue(m._a) && (m._tzm === undefined));
}

I'm happy to do a PR to fix this, I'm just not entirely sure of the correct approach (because I'm not entirely sure why _a is checked in the first place).

[maggiepint]

I never write code for moment timezone, only core, so please don't take my word for anything. This code is Tim's territory. That said, what _a is is the initial array of values used to construct a date.
That array is not kept up to date after any mutations occur. What you are suggesting should be fine to do, provided your code path only looks at it at construction time, not at a later time after a mutation has occurred.
Moment core has a util function that shims array.prototype.some if you are in IE 8, or uses it directly if you have it. Using the approach taken there to do what your arrayHasValue function does might make sense.

var some;
if (Array.prototype.some) {
    some = Array.prototype.some;
} else {
    some = function (fun) {
        var t = Object(this);
        var len = t.length >>> 0;

        for (var i = 0; i < len; i++) {
            if (i in t && fun.call(this, t[i], i, t)) {
                return true;
            }
        }

        return false;
    };
}

export { some as default };

Then it's invoked this way:

var parsedParts = some.call(flags.parsedDateParts, function (i) {
            return i != null;
        });

Also of note, you could use the value _pf.parsedDateParts that was added to 2.13.0. This is an array of the actual date parts parsed from the initial format (_a is an array that has had default data filled in).

[joekrill]

Thanks @maggiepint -- that's super helpful. Given that, I did some more digging. It sounds like it would be ideal to use parsedDateParts as it is part of the public API, but it looks like that is empty when constructing a moment from anything other than a string -- so in the case of creation from an array or object, _a has the provided values, but parsedDateParts is empty.

The some method doesn't appear to be accessible from moment-timezone. But the code can be duplicated there, at least, if needed.

But now I'm thinking the safest approach probably is to simply check explicitly for the 'X' or 'x' format. There doesn't seem to any easier/global way to answer the question: "was this moment initially created using an absolute time?". And relying on the values in _a seems risky since it's not really documented anywhere, and it's much less explicit as to what exactly is being checked for. For now I'll put in a PR taking that approach.

[mattjohnsonpint]

Hmmm... shouldn't this also then affect ISO input with an explicit Z? It doesn't.

moment.tz("2016-05-25T16:47:46.106Z", 'YYYY-MM-DD[T]HH:mm:ss.SSSZ', true, 'America/New_York').toISOString()
// "2016-05-25T16:47:46.106Z"

Perhaps because Z is binding the offset to zero? Whatever does this should probably be able to be modified for X and x also.

[joekrill]

@mj1856 The needsOffset method returns false if the moment object has the _tzm property defined. In the case of the ISO input with an explicit Z, _tzm is given a value of -0, so needsOffset returns false and the time is not adjusted. In the case of a timestamp, _tzm is still undefined, so it attempts to apply to adjust the timestamp with the offset.

So I believe the current behavior is correct (if I'm understanding correctly):

moment.tz("2016-05-25T16:47:46.106Z", 'YYYY-MM-DD[T]HH:mm:ss.SSSZ', true, 'America/New_York').format()
// "2016-05-25T12:47:46-04:00"