luxon icon indicating copy to clipboard operation
luxon copied to clipboard

`toRelativeCalendar` doesn't really work like you'd expect

Open icambron opened this issue 6 years ago • 12 comments

This is a little funny:

base = DateTime.local(2018, 12, 31);
base.plus({ days: 1 }).toRelativeCalendar({base}) //=> "next year"

This is technically correct, but I suspect most people would want that to say "tomorrow". Same issue with the end of months.

The question is: when should it say next year?

  1. whenever it is next year? (current behavior)
  2. never?
  3. When it's over 1 year in the future? (this is pretty weird IMO; in our 12/31 example, it would only say "next year" for one day at the end of next year")
  4. when it's over N months in the future? For what N?

Same problem for months. And I haven't even introduced weeks, which is surely similar.

icambron avatar Dec 31 '18 21:12 icambron

Optional parameter? Let's say: we define a default value 'next year' but the developer can send a parameter which indicates the value to be return

djhvscf avatar Jan 06 '19 03:01 djhvscf

I'd say the closest boundary is the correct one, but overriding it with a parameter sounds interesting. I grant it's likely a major version bump, but I'd say if it's within 7 days and hits such a boundary, it should say "tomorrow, if it's within 8 days and hits a boundary, "next week", if it's within 32 days and hits a boundary, "next month". It sounds like we need to decide what "correct" is before we PR, right?

robrich avatar Dec 31 '19 23:12 robrich

Yes, we need a spec. I like the idea that the threshold matches the size of the unit. I don't like the idea of overrides because I think it would be a confusing API. We can always add it later but we can't take it back once we do. So I'd like to make this change without expanding the API surface area.

I don't think this really requires a major version bump, either.

icambron avatar Jan 01 '20 19:01 icambron

Here's a suggestion that I'm using in one of my projects where I face this problem frequently. I run some TDD in it that's why you can see some pattern generalization.

Feel free to use it as much as MIT and WTFPL allows you :P

TL;DR:

If it's one or more years apart, say "next year" or "in x years" If it's one or more months apart, say "next month" or "in x months" if it's one of more days apart, say "tomorrow" or "in x days"

Anything that is less than 1 day apart is counted as "in x hours", "in x minutes"... and so forth

If the next day ends up in a different month or year, then it's counted as "tomorrow"

const toRelativeYears = years => {
  return years === 1
    ? 'next year'
    : `in ${years} years`;
};

const toRelativeMonths = months => {
  return months === 1
    ? 'next month'
    : `in ${months} months`;
};

const toRelativeDays = days => {
  return days === 1
    ? 'tomorrow'
    : `in ${days} days`;
};

const toRelative = (value, unit) => {
  return value === 1
    ? `in 1 ${unit.slice(0, unit.length - 1)}`
    : `in ${value} ${unit}`;
};

module.exports = (source, comparison) => {
  const units = ['days', 'hours', 'minutes', 'seconds'];
  const diffObject = comparison.diff(source, units).toObject();

  if (diffObject.days < 0 || diffObject.hours < 0 || diffObject.minutes < 0 || diffObject.seconds < 0) {
    return 'in the past';
  }

  if (source.month !== comparison.month && source.plus({ days: 1 }).month === comparison.month) {
    return 'tomorrow';
  }

  for (let i = 0; i < units.length; i++) {
    const withinUnit = diffObject[units[i]] < 1 && diffObject[units[i]] >= 0;
    if (withinUnit) {
      const nextUnitIsGreaterThanOne = diffObject[units[i + 1]] >= 1;
      if (nextUnitIsGreaterThanOne) {
        return toRelative(diffObject[units[i + 1]], units[i + 1]);
      }
    }
  }

  const yearsApart = comparison.year - source.year;
  const monthsApart = comparison.month - source.month;
  const daysApart = comparison.day - source.day;

  if (yearsApart >= 1) {
    return toRelativeYears(yearsApart);
  }
  if (monthsApart >= 1) {
    return toRelativeMonths(monthsApart);
  }
  if (daysApart >= 1) {
    return toRelativeDays(daysApart)
  }

  return 'now';
};

Tests

const { expect } = require('chai');
const { DateTime } = require('luxon');

const toRelativeString = require('./to-relative-string');

const ClockAt = (timestamp) => DateTime.fromISO(timestamp, { zone: 'utc' })

describe('Special conditions', () => {
  it('prints "now" if both compared dates are the same', () => {
    expect(
      toRelativeString(
        ClockAt('2020-08-31T10:00:00.000Z'),
        ClockAt('2020-08-31T00:00.000Z')
      )
    ).to.eql('now');
  });
  
  describe('One day away', () => {
    it('does not show as "next month" when "next month" is one day away', () => {
      expect(
        toRelativeString(
          ClockAt('2020-08-31T10:00:00.000Z'),
          ClockAt('2020-09-01T13:00:00.000Z')
        )
      ).to.eql('tomorrow');
    });
    it('does not show as "next year" when "next year" is one day away', () => {
      expect(
        toRelativeString(
          ClockAt('2020-12-31T10:00:00.000Z'),
          ClockAt('2021-01-01T13:00:00.000Z')
        )
      ).to.eql('tomorrow');
    });
  });
});

describe('In x years', () => {
  it('calculates next year when dates are less than 12 months apart', () => {
    expect(
      toRelativeString(
        ClockAt('2020-03-10T10:00:00Z'),
        ClockAt('2021-02-10T10:00:00Z')
      )
    ).to.eql('next year');
  });
  it('calculates "in 2 years" when dates are less than 24 months days apart', () => {
    expect(
      toRelativeString(
        ClockAt('2020-03-10T10:00:00Z'),
        ClockAt('2022-02-10T10:00:00Z')
      )
    ).to.eql('in 2 years');
  });
});

describe('In x months', () => {
  it('calculates next month when dates are less than 30 days apart', () => {
    expect(
      toRelativeString(
        ClockAt('2020-01-10T10:00:00Z'),
        ClockAt('2020-02-05T10:00:00Z')
      )
    ).to.eql('next month');
  });
  it('calculates "in 2 months" when dates are less than 60 days apart', () => {
    expect(
      toRelativeString(
        ClockAt('2020-01-10T10:00:00Z'),
        ClockAt('2020-03-05T10:00:00Z')
      )
    ).to.eql('in 2 months');
  });
});

describe('In x Days', () => {
  it('calculates next day when dates are exactly 24 hours apart', () => {
    expect(
      toRelativeString(
        ClockAt('2020-01-01T10:00:00Z'),
        ClockAt('2020-01-02T10:00:00Z')
      )
    ).to.eql('tomorrow');
  });

  it('calculates the next day when dates are 25 hours apart on the next day', () => {
    expect(
      toRelativeString(
        ClockAt('2020-01-01T10:00:00Z'),
        ClockAt('2020-01-02T11:00:00Z')
      )
    ).to.eql('tomorrow');
  });

  it('calculates "in 2 days" on the SAME MONTH but DIFFERENT DAY and less than 48h', () => {
    expect(
      toRelativeString(
        ClockAt('2020-01-01T10:00:00Z'),
        ClockAt('2020-01-03T09:00:00Z')
      )
    ).to.eql('in 2 days');
  });
});

describe('In x hours', () => {
  it('calculates the relative date when dates are in 1 hour', () => {
    expect(
      toRelativeString(
        ClockAt('2020-01-01T10:00:00Z'),
        ClockAt('2020-01-01T11:00:00Z')
      )
    ).to.eql('in 1 hour');
  });
  
  it('calculates the relative date when dates are in 2 hours', () => {
    expect(
      toRelativeString(
        ClockAt('2020-01-01T10:00:00Z'),
        ClockAt('2020-01-01T12:00:00Z')
      )
    ).to.eql('in 2 hours');
  });
});

describe('In x minutes', () => {
  it('calculates the relative date when dates are in 1 minute', () => {
    expect(
      toRelativeString(
        ClockAt('2020-01-01T10:00:00Z'),
        ClockAt('2020-01-01T10:01:00Z')
      )
    ).to.eql('in 1 minute');
  });
  
  it('calculates the relative date when dates are in 2 minutes', () => {
    expect(
      toRelativeString(
        ClockAt('2020-01-01T10:00:00Z'),
        ClockAt('2020-01-01T10:02:00Z')
      )
    ).to.eql('in 2 minutes');
  });
});

describe('In x seconds', () => {
  it('calculates the relative date when dates are in 1 second', () => {
    expect(
      toRelativeString(
        ClockAt('2020-01-01T10:00:00Z'),
        ClockAt('2020-01-01T10:00:01Z')
      )
    ).to.eql('in 1 second');
  });
  
  it('calculates the relative date when dates are in 2 seconds', () => {
    expect(
      toRelativeString(
        ClockAt('2020-01-01T10:00:00Z'),
        ClockAt('2020-01-01T10:00:02Z')
      )
    ).to.eql('in 2 seconds');
  });
});

describe('Relative time in the past', () => {
  it('One day', () => {
    expect(
      toRelativeString(
        ClockAt('2020-01-02T10:00:00Z'),
        ClockAt('2020-01-01T10:00:00Z')
      )
    ).to.eql('in the past');
  });

  it('One hour', () => {
    expect(
      toRelativeString(
        ClockAt('2020-01-01T10:00:00Z'),
        ClockAt('2020-01-01T09:00:00Z')
      )
    ).to.eql('in the past');
  });

  it('One minute', () => {
    expect(
      toRelativeString(
        ClockAt('2020-01-01T10:02:00Z'),
        ClockAt('2020-01-01T10:01:00Z')
      )
    ).to.eql('in the past');
  });
  
  it('One second', () => {
    expect(
      toRelativeString(
        ClockAt('2020-01-01T10:01:59Z'),
        ClockAt('2020-01-01T10:01:58Z')
      )
    ).to.eql('in the past');
  });
});

FagnerMartinsBrack avatar Aug 31 '20 13:08 FagnerMartinsBrack

Bump! This also is a problem if tomorrow is another month. No one would say next month, they would say 'tomorrow'

bravadomizzou avatar Nov 30 '20 17:11 bravadomizzou

DateTime.local(2021, 6, 1).toRelativeCalendar({
      base: DateTime.local(2021, 5, 31),
    })

This gives 'next month' instead of 'tomorrow', which is strange to me

basslagter avatar Jun 08 '21 09:06 basslagter

It happens with me also ... Is there any plan to fix it?

Doaa-Ismael avatar Aug 31 '21 16:08 Doaa-Ismael

Any solution on this ? It really makes no sense to return "next month" instead of "tomorrow".

tominou avatar May 31 '22 06:05 tominou

I've found the issue and solution if someone wants to write up a pull request? Options passed through attributes should not be overwritten by defaults, instead they should overwrite the default settings. This makes passing { units: ['days'] } impossible to be used.

Current declaration inside toRelativeCalendar:

{
      ...options,
      numeric: "auto",
      units: ["years", "months", "days"],
      calendary: true,
}

Solution, move the spread of the options in the object declaration:

{
      numeric: "auto",
      units: ["years", "months", "days"],
      calendary: true,
      ...options,
}

I would change this in other places in the library as well, as that just makes more sense to me. Although I'm not the author of the luxon library to make that choice.


I'm using this workaround currently (only need yesterday/today/tomorrow/in 2 days/2 days ago being written)

const MAX_DAYS_DIFF_WRITTEN = 2

// Resets to 00:00:00.0000
export const resetTime = (date) => {
  const d = new Date(date || Date.now())
  d.setHours(0)
  d.setMinutes(0)
  d.setSeconds(0)
  d.setMilliseconds(0)
  return d
}

// String/JS date to written format
export const dateWritten = (date) => {
  const d = resetTime(new Date(date || Date.now()))
  const now = resetTime(new Date())
  const dateTime = DateTime.fromMillis(d.getTime())

  const diff = dateTime.diff(DateTime.fromMillis(now.getTime()), 'days')
  if (diff.days >= -MAX_DAYS_DIFF_WRITTEN && diff.days <= MAX_DAYS_DIFF_WRITTEN) {
    // Middle of a month and 00:00:00.0000 time, as base
    const base = DateTime.fromMillis(1642201200000)
    return base.plus({ days: diff.days }).toRelativeCalendar({ base })
  }

 return dateTime.toLocaleString()
}

mattickx avatar May 31 '22 10:05 mattickx

Note that as a workaround you can pass a unit option to toRelativeCalendar if you know which unit you'd like to have it use:

DateTime.local(2022, 8, 1).toRelativeCalendar({ base: DateTime.local(2022, 7, 31) }) === "next month"
DateTime.local(2022, 8, 1).toRelativeCalendar({ base: DateTime.local(2022, 7, 31), unit: "days" }) === "tomorrow"

sciyoshi avatar Jul 26 '22 17:07 sciyoshi

Recently ran into this issue, whereby a date in December 2023 was being reported as 'last year' in January 2024. The expectation is that it should have been reporting 'last month'.

Thinking about it, the logic should probably be (for time in the past):

  • value < 1 hour: relative time in minutes(it could be precise or "a few minutes ago", depending on size of minutes)
    • example: 10 minutes ago
    • example: a few minutes ago
  • value < 1 day: relative time in hours
    • example: 2 hours ago
  • value < 7 days: relative time in days
    • example: 5 days ago
    • example: yesterday
  • value < 1 month: relative time in weeks
    • example: 2 weeks ago
    • example: last week
  • value < 1 year: relative time in months
    • example: 2 months ago
    • example: last month
  • then use years
    • example: 1 year ago
    • example: last year

ajmas avatar Jan 18 '24 22:01 ajmas

Ended up just going with another package for relative time formatting: https://www.npmjs.com/package/javascript-time-ago

ajmas avatar Jan 19 '24 00:01 ajmas