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

add & subtract method inconsistency on negative and positive floating numbers with bigger units #2763

Closed
huttli opened this issue Nov 20, 2015 · 3 comments
Labels

Comments

@huttli
Copy link

huttli commented Nov 20, 2015

m().add(1.2, 'months').toString();
'Sun Dec 20 2015 12:12:57 GMT+1030'
m().add(1, 'months').toString();
'Sun Dec 20 2015 12:13:01 GMT+1030'
m().add(-1, 'months').toString();
'Tue Oct 20 2015 12:13:06 GMT+1030'
m().add(-1.2, 'months').toString();
'Sun Sep 20 2015 12:13:10 GMT+0930'

I don't expect the floating numbers to be exactly calculated into smaller units fractions given the known two issues here:
#2305
#2430

But adding with negative floating number seems give the wrong month, too. I'd expect the last line to be
'Sun Oct 20 2015 12:13:10 GMT+1030'

A Math.round() vs Math.floor() difference?

I live in an area doing DST change in early Ocotober, please ignore the GMT offset differences. :)

@huttli
Copy link
Author

huttli commented Nov 20, 2015

The same for subtract with a positive floating point number.

We're relying on the native Date.setMonth/setUTCMonth(https://github.com/moment/moment/blob/develop/moment.js#L693) to convert the floating point number to integer number, where ECMA defines as Math.floor with its absolute value.
http://www.ecma-international.org/ecma-262/5.1/#sec-9.4

So subtract 1.2 months from November becomes converting (10-1.2) to integer, which becomes 8, hence the September result.

I still expect our feature rich momentjs to perform a bit more intelligent than the native Date object. :D

@icambron
Copy link
Member

I think we'd take a PR for this, sure.

@mattjohnsonpint
Copy link
Contributor

We'll track in PR #2970. Thanks.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants