Interval hasSame minus(1)

[sam-s4s]

Interval.hasSame does a minus(1) on the end date in the calculation, which breaks all comparisons when the end date time has "00:00:00.000" for the time component (for example). It will also break some non-0 time component comparisons, especially when miliseconds are 0).

As per...
https://github.com/moment/luxon/blob/master/src/interval.js#L268
The code is...

return this.isValid ? this.isEmpty() || this.e.minus(1).hasSame(this.s, unit) : false;

A previous issue resolves one bug in hasSame, and @icambron says "That subtraction is meant to make the interval half-open"...
#709

However I am not sure what "half-open" means, or why you would want that behaviour... It literally means that if you pass in a 0 time component, you will always get incorrect results back.

Maybe I'm missing something - is there ever a case where you would want to minus(1) from the end date time?

I have not been able to think of any scenario where you would want to do minus(1), and thus am proposing to remove that piece of code.

[diesieben07]

Half-open means the endpoint is not included in the interval.
Interval("2025-05-01T00:00:00Z", "2025-05-02T00:00:00Z") is an interval that includes the entirety of 2025-05-01, but not the first second of 2025-05-02.
That means hasSame("day") on that interval would be true (the interval is contained entirely within one day).
If e.minus(1) were removed, hasSame("day") on that interval would return false, because e is on a separate day.

You need to create your intervals according to this, the endpoint is the first millisecond that should not be included in the interval. If you show your code where you get a result you do not expect, I can point out what should be done instead.

[sam-s4s]

Thank you for explaining the terminology, I completely understand now. The direct solution to my problem is that I should not be doing a hasSame on the Interval, but instead on my DateTime directly. ie. dateTimeFrom.hasSame(dateTimeTo, 'day').

While I agree with the concept, I feel that, like me, many will fall into the same trap, and while in theory this issue could be closed as there is no bug, I would like to propose a "fix" for making this less of a problem moving forwards.

I honestly feel as though almost everyone attempting to use this would actually be expecting the operation to be performed on the end datetime they passed in, as opposed to the calculated inclusive endpoint.

Option 1) Everyone wins.
If an inclusive = false param was added, it could be a self-documenting way of retaining current behaviour, but informing people "hey, this might not work how you think", while also providing the option to do what they almost always will actually want.
Crudely...

hasSame(unit, inclusive = false) {`
    return this.isValid ? this.isEmpty() || this.e.minus(inclusive ? 0 : 1).hasSame(this.s, unit) : false;
}

The above could could be improved to avoid the minus 0, but I think this option could be the best.

Option 2) New function.
Add a new hasSameInclusive function, that just does the same thing without the minus. Hopefully peopole will notice that there are 2, and choose the appropriate one.

Option 3) Radical.
I am not certain there is actually a use-case for a hasSame on an Interval if left as it currently is (non-inclusive)? I might be wrong, but I haven't been able to come up with one.

[dobon]

I like option 1 (or nil option 0). As for potential use cases: I can see how it might be useful for certain types of scheduling tasks (and also how it would get in the way for other types of scheduling tasks). Just my 2 cents.

[sam-s4s]

Fair :) And I feel like we can't lose with option 1. It doesn't break or change any existing usages. It self-documents the concept of inclusivity (or the fact that it by default is not). It gives the developer the option to use inclusive if they want. Win-win, surely?

[diesieben07]

I fail to understand why hasSame specifically should be changed and not everything else on Interval. Intervals are half-open for a reason, most ranges in programming are half open.
For example:

• "Hello World".substring(1, 3) is "el", 1, 3 describes a half-open interval, the end index is not included.
• ["foo", "bar", "baz", "quuz"].slice(1, 3) is ["bar", "baz"], again the end index is not included.

Why is this? It makes most use-cases more natural.
Let's go back to Interval. Say you have a DateTime and you want to have an interval that includes that DateTime plus the next day:
Interval.fromDateTimes(dt, dt.plus({ days: 1 })).
If this interval were closed (i.e. it would include start and endpoint) you'd now have an Interval that is 24 hours and one millisecond long.

Say you want to know how long an interval is:
millisDuration = iv.end.toMillis() - iv.start.toMillis()
If interval were closed, this would be off by 1.

Say you want to describe an empty interval. You simply can't do it with a closed interval.

---

If you create your intervals correctly, then hasSame works correctly.
You still have not actually shown the code where you are creating your intervals, I am assuming you are not doing it correctly and you are actually creating the interval as if Interval were closed. If you do this, it makes Interval return incorrect results, because Interval is always half-open.

As for your suggestions:
Option 1 and 2: This just add a "please give me a wrong answer" option. It would make hasSame (or hasSameInclusive) act different from all other methods in Interval. Intervals are not inclusive. If you created one as if it were, you have created the wrong interval. The solution is to create the right interval, not to change how hasSame works.
Sure, you could add a parameter, but then you have "fixed" hasSame, but all other methods still treat the interval as half-open and will give you the "wrong result" (because your interval is wrong).

Option 3: This suggests making Interval closed (at least that's how I understand it). Which just has too many drawbacks in my opinion (apart from it being a breaking change).

@sam-s4s You still have not actually shown your code.

The direct solution to my problem is that I should not be doing a hasSame on the Interval, but instead on my DateTime directly. ie. dateTimeFrom.hasSame(dateTimeTo, 'day').

No, the solution is likely that your Interval is wrong. Please show code that shows where you think hasSame returns the wrong result.

[sam-s4s]

Firstly, @diesieben07 thank you for your time. Secondly, sorry for any confusion.

The actual Interval is working perfectly, the calculations are all correct. The only concern I have is strictly within hasSame.

The "problem" is less about the technical way that Intervals work, and more about the human expectation of what is happening. This is exacerbated by the fact that having 0 times are very common, generally causing a large difference, even though we're only talking about a single millisecond change.

The documentation for Interval.hasSame reads:
"Returns whether this Interval's start and end are both in the same unit of time"
But technically that's ambiguous - is it the Interval's inclusive or non-inclusive end? The answer is the inclusive end. Which is, incidentally, not the parameter we passed in, making it a bit confusing, from a human perspective.

The code (example) in question is very simple. We are calculating the time until a certificate expires, and if it's "today" we show a special message...

let today = DateTime.fromISO('2025-06-02');
let expiryDate = DateTime.fromISO('2025-06-03');
let interval = Interval.fromDateTimes(today, expiryDate);
let isToday = interval.hasSame('day');
console.log(isToday); // true

Intuitively this should return false, as the start 2025-06-02 and end 2025-06-03 are clearly on different days. This is because the value we have in our heads, as it is the param we actually passed in, is the non-inclusive end.
So while technically that might be correct, as the inclusive end of the Interval is 2025-06-02 23:59:59.999, I feel like this is a bit of a bear trap, as most people might expect the hasSame calculation to be done on the end param we passed in, not the modified/calculated end.
I would expect the above code to be doing that calculation on the values I passed in, not the inclusive value. I have also asked many other developers to see what they would expect in this scenario, and all have agreed, so I feel at the least it's a very common problem.

However I am not suggesting we make a breaking change, but instead do something to help prevent people from falling into this bear trap, the same way he have.

I feel like adding in a new optional param inclusive = true or nonInclusive = false (or exclusive?) would both self document and solve this problem in a nice and non-breaking way. I don't think there is any down-side to this, and there are many benefits.

But if you are strongly against adding this, I would ask at least we update the documentation on Interval.hasSame to make it very clear that it is operating on the modified inclusive end.

[diesieben07]

I am fine with improving the documentation, I agree that "end" is not the best wording here. I would lean towards "the last DateTime included in the interval", which aligns with Interval#lastDateTime.

As for the parameter, I still don't think it really achieves what you want.
In your code example it is behaving exactly as you described, does the certificate expire today? No, it is valid througout the entirety of today, it expires tomorrow! Hence hasSame('day') should be false.
I realize this doesn't solve your problem, but your question is not "does it expire today", it is "is today the last valid day for the certificate" or in other words "does it expire tomorrow". So you should check for that and not "today".

That said, I am not sure why you're using Interval here at all instead of just expiryDate.hasSame(today, 'day').

I have played with the idea of making the ends of Interval configurable in a future version, for example you could have Interval.closed(a, b) or Interval.halfOpen(a, b) and have all methods in Interval behave accordingly. But I have not checked the feasibility of that feature.

[sam-s4s]

In your code example it is behaving exactly as you described, does the certificate expire today? No, it is valid througout the entirety of today, it expires tomorrow! Hence hasSame('day') should be false.
Oh boy have we had some trouble coming up with wording that everyone understand for expiry LOL
In our current iteration, we have done a survey and it turns out people almost always expect expiry to be inclusive, so if you say "expires today", they assume it's valid today, and dead tomorrow. So this is what we went with, without trying to be too wordy.
But it's a tough one!

That said, I am not sure why you're using Interval here at all instead of just expiryDate.hasSame(today, 'day').
You are correct and that is indeed what the current code has been changed to, once we found this Interval.hasSame behaviour :D
But the actual reason I was previously doing an Interval.hasSame is because this code is just a small piece of the function it's in, which ultimately is used to describe the delta between 2 different DateTimes.
I figured the best way to do that was to create an Interval, and then interrogate it for the different values I'm looking for.

For example, a small snippet...

    returnValues.totalFloats = {
        years: interval.length('years'),
        months: interval.length('months'),
        weeks: interval.length('weeks'),
        days: interval.length('days'),
        hours: interval.length('hours'),
        minutes: interval.length('minutes'),
        seconds: interval.length('seconds')
    };

There are a few things it does, including give you all the total floats of all the values (for example, minutes might be 5634). But it can also let you specify which units you care about, and then if you have anything higher, it will flow to that (using Interval.toDuration). This ultimately also create a nice string for you to use (5 days 6 hours 1 hour 8 minutes... if you specified those units).

Considering all this, I was firmly in Interval land, and then when I wanted a nice today boolean, I figured I'd just use what was already there :P But yes, now I know better, I'm doing directly on the dates now.

So at the end of the day, I guess I feel like these functions are part of the toolkit for developers - when you're dealing with an Interval, you might want X things from that calculation. And for me, one of the more important things is the ability to compare the param dates, or at least be aware "hey, this isn't the place to do that" :) Currently I feel like this might be a bit of a common mistake. Thanks!

[diesieben07]

It is indeed a common mistake to assume "Intervals are inclusive" (talking specifically about Interval in Luxon here). I have made this mistake myself before and had to fix a lot of code.
However the solution to this mistake is not to make intervals sometimes inclusive and sometimes not (although I might explore a more flexible Interval, where both ends can be either inclusive or exclusive, in the future). The solution would be to improve the documentation to make developers aware of how Interval works and how to use it correctly.

[sam-s4s]

Yep I'm totally happy for the code to remain the same and the docs updated for clarity. Thank you :)