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

Duration.diff returns negative number #1165

Closed
joshua0308 opened this issue Mar 27, 2022 · 4 comments
Closed

Duration.diff returns negative number #1165

joshua0308 opened this issue Mar 27, 2022 · 4 comments

Comments

@joshua0308
Copy link

Describe the bug
I get a negative number for a diff although the end date is after the start date.

To Reproduce

const start = DateTime.fromISO("2022-05-05T23:00", { zone: "UTC" });
const end = DateTime.fromISO("2022-05-10T00:00", { zone: "Europe/Madrid" }); // 2022-05-09T22:00Z
const diff = end.diff(start, ['days', 'hours', 'minutes']).toObject();

// diff === { days: 4, hours: -1, minutes: 0 }

Actual vs Expected behavior
I expect the diff to equal { days: 3, hours: 23, minutes: 0} instead of { days: 4, hours: -1, minutes: 0 }. It seems unintuitive to me that I have to think about adding and then subtracting to get to the diff.

Desktop (please complete the following information):

  • OS: Mac OS
  • Browser Chrome 99
  • Luxon version 2.3.1
  • Your timezone "America/New_York"
@joshua0308 joshua0308 changed the title Duration.diff return negative number Duration.diff returns negative number Mar 27, 2022
@JoaoAugustoPansani
Copy link

May I try to fix this?

@icambron
Copy link
Member

@JoaoAugustoPansani sure

@JoaoAugustoPansani
Copy link

JoaoAugustoPansani commented Apr 20, 2022

Hi @icambron , I don't have permission to make a branch, so I'll propose the solution:

On the file ./scripts/src/impl/diff.js there's a function dayDiff:

function dayDiff(earlier, later) {
  const utcDayStart = (dt) => dt.toUTC(0, { keepLocalTime: true }).startOf("day").valueOf(),
    ms = utcDayStart(later) - utcDayStart(earlier);
  return Math.floor(Duration.fromMillis(ms).as("days"));
}

What I did for solving this issue, was delete the second argument inside .toUTC:

function dayDiff(earlier, later) {
  const utcDayStart = (dt) => dt.toUTC(0).startOf("day").valueOf(),
    ms = utcDayStart(later) - utcDayStart(earlier);
  return Math.floor(Duration.fromMillis(ms).as("days"));
}

Then I wrote a test inside ./scripts/test/datetime/diff.test.js:

test("DateTime#diff works when date has a zone object", () => {
  const start = DateTime.fromISO("2022-05-05T23:00", { zone: "UTC" });
  const end = DateTime.fromISO("2022-05-10T00:00", { zone: "Europe/Madrid" });

  const diff = end.diff(start, ["days", "hours", "minutes"]).toObject();
  expect(diff.days).toBe(3);
  expect(diff.hours).toBe(23);
  expect(diff.minutes).toBe(0);
});

It worked, and didn't crash any other test.
Hope I helped with this one.

@icambron
Copy link
Member

@JoaoAugustoPansani on github, instead of making a branch on my repository, you create a fork so that you have your own Luxon repository, then you make your changes there as you see fit on whatever branch you want, and then you post a PR across the repos.

JoaoAugustoPansani added a commit to JoaoAugustoPansani/luxon that referenced this issue Apr 21, 2022
JoaoAugustoPansani added a commit to JoaoAugustoPansani/luxon that referenced this issue Apr 21, 2022
ConnorLanglois added a commit to ConnorLanglois/luxon that referenced this issue Mar 17, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
3 participants