moment / luxon

⏱ A library for working with dates and times in JS
https://moment.github.io/luxon
MIT License
15.23k stars 730 forks source link

Duration.fromISOTime('0:00') --> can't be parsed #1460

Open carstenfuchs opened 1 year ago

carstenfuchs commented 1 year ago

Describe the bug The attempt to create a Duration with fromISOTime() fails whenever the hours are not two digits.

(As a side note: the attempt to create a Duration from time 50:00 works. See example below.)

To Reproduce These are all invalid:

luxon.Duration.fromISOTime("0:00")
luxon.Duration.fromISOTime("1:00")
luxon.Duration.fromISOTime("222:00")

Observe that in contrast, this is valid:

luxon.Duration.fromISOTime("50:00")

Actual vs Expected behavior All of these should result in valid Durations:

luxon.Duration.fromISOTime("0:00")
luxon.Duration.fromISOTime("1:00")
luxon.Duration.fromISOTime("222:00")

(Alternatively, "50:00" should be invalid.)

Desktop (please complete the following information):

Additional context In my opinion, the root cause of this problem is that times and durations are not actually compatible:

While a time such as 25:00 does not make sense (and is not covered by ISO 8601), a duration of 25 hours does. Specifying durations with hours, minutes and seconds in the form HH:MM:SS.s also makes sense, but should not be limited to the limits of times given in the same form. (Strictly spoken, the method name fromISOTime() is an antagonism: it should possibly be named fromHours() or similarly.)

In summary, my proposal is to relax Duration.fromISOTime() to accept all of the examples above, that is, hours that have one, two or more digits.

diesieben07 commented 1 year ago

ISO refers to ISO 8601, which requires leading zeros. However you are correct in that fromISOTime should technically not accept invalid times of day (such as your example 50:00). It is debatable whether we should make fromISOTime produce an invalid Duration for invalid times of day and if that is worth the breaking change. ISO 8601 specifies a different format for actual durations (unrelated to time of day), which can be parsed by Duration.fromISO.

carstenfuchs commented 1 year ago

Yes, I think that there are two “clean” solutions for this:

  1. Either strictly apply ISO 8601, which would cause times such as 50:00 (that are valid today) to become invalid.
  2. Further relax (or, in a sense, forego) the ISO 8601 rules so that examples such as 0:00 or 123:45 become valid.

It would be ideal if there was a standards or normative reference for durations that are given in this form, but I've not be able to find any (maybe DIN 5008 has more than I have been able to dig up?) But even without such a normative reference, I think that the 2. option, relaxing the rules, would be the right choice for the following reasons: