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

Inconsistent diff() with .format() output #1362

Closed
jathanasiou opened this issue Jan 28, 2021 · 4 comments
Closed

Inconsistent diff() with .format() output #1362

jathanasiou opened this issue Jan 28, 2021 · 4 comments

Comments

@jathanasiou
Copy link

It seems that that dayjs operation for .format() is not reversible via parsing its output with dayjs(dayjs().format()), not regards to the diff()` function at least.

To demonstrate, I experimented with the following snippet.

// Node.js v12.20.1
// "dayjs": "^1.10.4"
// OS Windows 10
const dayjs = require('dayjs');

const now = dayjs();

const futureDayjs = dayjs().add(49, 'day');
const futureString = futureDayjs.format();

console.log(`Now <diff> daysjs date: ${now.diff(futureDayjs, 'day')}`);
console.log(`Now <diff> string date: ${now.diff(futureString, 'day')}`);
console.log(`Now <diff> dayjs(string) date: ${now.diff(dayjs(futureString), 'day')}`);

And getting the output

Now <diff> daysjs date: -49
Now <diff> string date: -48
Now <diff> dayjs(string) date: -48

Which is not what I'd expect. Is there some timezone or other attribute that I'm missing that could be causing this or is this a bug?

@iamkun
Copy link
Owner

iamkun commented Jan 28, 2021

please pass true as the second argument of .diff

@jathanasiou
Copy link
Author

please pass true as the second argument of .diff

console.log('with true')
console.log(`Now <diff> daysjs date: ${now.diff(futureDayjs, 'day', true)}`); // -49.00000002314815
console.log(`Now <diff> string date: ${now.diff(futureString, 'day', true)}`); // -48.99999065972222

So, while this does highlight the rounding issue, it still seems inconsistent that diff is providing different results for (arguably) the same date.

This can be very important for some edge cases as I discovered this during unit testing.

@iamkun
Copy link
Owner

iamkun commented Jan 29, 2021

this is because you just drop the millisecond in this line

const futureString = futureDayjs.format();

@jathanasiou
Copy link
Author

Yeah that makes sense now that I think about it. I should be able to write tests that simply do not take MS into account then. Thanks!

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

No branches or pull requests

2 participants