Formatting a moment object breaks equality comparisons

[zakhenry]

When a moment object is formatted, the equality of the object with a clone of the unformatted object becomes false. This is unexpected, as formatting operations should be nullipotent (not change the underlying object).

Small example:

foo = moment();
bar = foo.clone();
_.isEqual(foo, bar); //true
angular.equals(foo, bar); //true
foo.format();
_.isEqual(foo, bar); //false
angular.equals(foo, bar); //false

See https://jsfiddle.net/x4hr9qmq/1/for fiddle

Note that === will always fail, but this is expected as moment.clone() should return a different object, but equality comparitors like lodash's _.isEqual and angular angular.equals should return true as they compare properties not object identity.

[maggiepint]

The reason for this is that .format() checks the isValid property for the first time. That property is evaluated and set. You're basically seeing the same issue that is described in #2894.

I'm curious though, why do you want to do this sort of comparison? There are tons of properties on a moment that really have nothing to do with the actual date value. For instance, because moment has an internal property _i that tracks the original input value, the following two moments won't be the same in that sort of equality comparison, even though they are the exact same point in time:

moment('2016-01-01')
moment('01/01/2016', 'MM/DD/YYYY')

If you want to know whether two moments are at the same point in time, .isSame() is the proper way to do it.

[zakhenry]

@maggiepint the reason this is a problem is for tracking changes to a model. In my example I have a User model with an updatedAt property. When I check the model if it has changed so I can see if a save is needed, my checker is reporting that the updatedAt property has changed even though all that has happened is the property has been formatted by moment.

As you can see this is less of a comparison of two different objects, more of a comparison in time of the same object.

[maggiepint]

I'll have to think about the best solution for your situation, but I think it's worth mentioning that moments have a lot of internal properties. Run moment() in the REPL and check it out.
That being the case, I don't know if a deep comparison of the sort you are attempting is the right thing to do with a moment, even if it did work. It's not going to result in great performance.

You maybe want to preserve either moment().valueOf() or just the formatted string on your model for comparison purposes.

[zakhenry]

@maggiepint yea I understand there are lots of internal properties, and that the deep checking is probably not performant, but for the needs I have, performance is nearly irrelevant as it is a on-save event, so very infrequent and client-side.

I think a better solution would be to store the internal moment properties as non-enumerable, as both lodash _.isEqual and angular .equals both only check enumerable properties.

[zakhenry]

@maggiepint to resolve my immediate problem, I can just define in my angular filters/formatting to first run a .clone() on the moment object before formatting, but this is not an intuitive solution, nor particularly memory efficient, when I'm not really modifying anything

[maggiepint]

You make an interesting point. FWIW, I'm the new kid on the maintainers team. I'd be curious if @ichernev, @timrwood or @icambron had an opinion.

[icambron]

I suspect you should usually call valueOf() on it for comparison purposes. IIRC, we don't use non-enumerable properties because they're not really supported in IE8. I think dropping support for that is on the table, in which case, yeah, we should totally hide those "private" fields.

[maggiepint]

Should we drop support for IE8? This escalated quickly!

[icambron]

I'm +1 on dropping IE8, but it should be on a major version change.

[marwahaha]

I think this works now. Just tested on Chrome.

Please re-open if it's still a problem.