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

Bug with hours format on diff moment object #1597

Closed
mattsintellectualproperty opened this issue Apr 8, 2014 · 4 comments
Closed

Bug with hours format on diff moment object #1597

mattsintellectualproperty opened this issue Apr 8, 2014 · 4 comments

Comments

@mattsintellectualproperty

I'm getting bad data back when trying to do a difference between two moment date objects. Only appears to occur for the hour format. I've attached the debug code that is working for me so you can simulate the issue. Could be bad code, but I couldn't find anything wrong with this after looking through the documentation for a few hours and only have problem trying to use moment to get back the remainder hours...

moment.lang('en', {
    relativeTime: {
        future: "in %s",
        past: "%s ago",
        s: "seconds",
        m: "a minute",
        mm: "%d minutes",
        h: "an hour",
        hh: "%d hours",
        d: "a day",
        dd: "%d days",
        M: "a month",
        MM: "%d months",
        y: "a year",
        yy: "%d years"
    }
});

var currentTime = moment();

var postDate = moment('Sun Apr 06 2014 03:48:38 GMT-0400').utc();

var testPostDate = moment(currentTime).subtract(3483, 'seconds');

console.log(currentTime);
console.log(testPostDate);

var totalTimeAgo = currentTime.diff(testPostDate);

var totalDaysAgo = currentTime.diff(testPostDate, 'days');

var totalHoursAgo = currentTime.diff(testPostDate, 'hours');

var totalMinutesAgo = currentTime.diff(testPostDate, 'minutes');

var totalSecondsAgo = currentTime.diff(testPostDate, 'seconds');


console.log('timeAgo----')
console.log(totalTimeAgo);

console.log('days---');
console.log(totalDaysAgo);
if (totalDaysAgo == 0) {
    totalDaysAgoText = '';
} else if (totalDaysAgo == 1) {
    totalDaysAgoText = totalDaysAgo + ' day';
} else if (totalDaysAgo > 1) {
    totalDaysAgoText = totalDaysAgo + ' days';
};
console.log(totalDaysAgo + ' days ago'); // Returns correct

console.log('hours---');
console.info(currentTime.diff(testPostDate, 'hours'));
// console.info(totalMinutesAgo / (60));
console.error(totalHoursAgo % 24); // What it should return
// console.error(moment(totalHoursAgo).format("hh") + ' hours ago'); // Returns correct with h, hh, H, and HH, no way to correctly return hour remainder that I could find based on documentation

var totalHoursAgoText = '';
if (totalDaysAgo >= 1) {
    totalHoursAgoText = ', ';
};
if (totalHoursAgo % 24 == 0) {
    totalHoursAgoText = '';
} else if ((totalHoursAgo % 24 != 0) && (totalHoursAgo % 24 > 1)) {
    totalHoursAgoText = totalHoursAgoText + totalHoursAgo % 24 + ' hours';
} else if ((totalHoursAgo % 24 != 0) && (totalHoursAgo % 24 == 1)) {
    totalHoursAgoText = totalHoursAgoText + totalHoursAgo % 24 + ' , hour ';
};
console.log('minutes---');
console.log(totalMinutesAgo);
console.log(moment(totalTimeAgo).format('m') + ' minutes ago'); // Returns correct


var totalMinutesAgoText = '';
if (totalHoursAgo % 24 >= 1) {
    totalMinutesAgoText = ', ';
};
if (moment(totalTimeAgo).format('m') == 0) {
    totalMinutesAgoText = '';
} else if (moment(totalTimeAgo).format('m') == 1) {
    totalMinutesAgoText = totalMinutesAgoText + moment(totalTimeAgo).format('m') + ' minute';
} else if (moment(totalTimeAgo).format('m') > 1) {
    totalMinutesAgoText = totalMinutesAgoText + moment(totalTimeAgo).format('m') + ' minutes';
};


console.log('seconds---');
console.log(totalSecondsAgo);
console.log(moment(totalTimeAgo).format('s') + ' seconds ago'); // Returns correct
if (moment(totalTimeAgo).format('s') == 0) {
    totalSecondsAgoText = '';
} else if (moment(totalTimeAgo).format('s') > 1) {
    totalSecondsAgoText = ', ' + moment(totalTimeAgo).format('s') + ' seconds';
};

console.log('\n\n');
console.log('NICELY FORMATTED -----');
console.log(totalDaysAgoText + totalHoursAgoText + totalMinutesAgoText + totalSecondsAgoText);
@icambron
Copy link
Member

icambron commented Apr 8, 2014

Can you boil this down to as few lines as possible with an expected result and an actual result? I'm not quite sure what I'm looking at here.

@mattsintellectualproperty
Copy link
Author

Sure thing, that was a snippet you could copy and paste into some client-side code to replicate, but this also reproduces the discrepancy.

var currentTime = moment();
var testPostDate = moment(currentTime).subtract(3483, 'seconds');
var totalTimeAgo = currentTime.diff(testPostDate);

console.error(moment(totalTimeAgo).format("hh") + ' hours ago'); // Returns incorrect with h, hh, H, and HH, no way to correctly return hour remainder that I could find based on documentation
console.log(moment(totalTimeAgo).format('m') + ' minutes ago'); // Returns correct
console.log(moment(totalTimeAgo).format('s') + ' seconds ago'); // Returns correct

@icambron
Copy link
Member

icambron commented Apr 8, 2014

Thanks. The problem is that moment(totalTimeAgo) is setting the time to the Unix timestamp of that diff:

var totalTimeAgo = currentTime.diff(testPostDate); // 3483000
moment(totalTimeAgo).format(); //=> "1969-12-31T19:58:03-05:00"

The reason that's 19:58 instead of 00:58 is that it's in local time. So you can fix this by using UTC:

moment(totalTimeAgo).utc().format(); //=> "1970-01-01T00:58:03+00:00"
//thus
moment(totalTimeAgo).utc().format('H'); //=> "0"

Note that all of this is sort of a hack. The really right way to do this is through Durations:

var totalTimeAgo = currentTime.diff(testPostDate);
moment.duration(totalTimeAgo).hours(); //=> 0
//or if you like decimals
moment.duration(totalTimeAgo).asHours(); //=> 0.9675

The issue is that we haven't yet implemented duration#format, so in the meantime most people are formatting the time against 1970 instead of using durations. See #1048.

@icambron icambron closed this as completed Apr 8, 2014
@mattsintellectualproperty
Copy link
Author

Okay good to know, I was looking at durations, and will switch over to that instead. Thanks!

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

No branches or pull requests

2 participants