Fix: Improve typing for min/max

[bensaufley]

I think it's good to be clear that null can be returned from min/max, but TypeScript is powerful enough to be even smarter about this. This PR updates the types so that at least obvious scenarios where at least one date is passed don't then require ! or other code smells.

As with all types as they get more complex, it's easy to miss things or forget scenarios. Please let me know if I've missed anything.

I wouldn't actually consider this a "fix" so much as an improvement on existing types but I don't think it's a feature and since the prior revision was marked as a fix, I'm keeping in line with that change.

[iamkun]

Any live example of this type update, please?

[bensaufley]

@iamkun Happy to provide examples; is there a place in the codebase I should do it?

For me, the issue the previous PR introduced is this (example code):

const myDate = dayjs('2023-08-17');
const anotherDate = dayjs(userInput);

if (!anotherDate.isValidDate()) return null;

return dayjs.min(myDate, anotherDate).add(1, 'week');

With the prior change, TS tells me .add isn't safe because the result of .min(…) may be null (and thus that I should put a ? before .add). But it will never be null; TypeScript reasonably guarantees that the two arguments passed in are both dayjs objects.

The function only takes Dayjs objects, but may technically be called with an empty array, in which case it will return null. But that's the only case in which it will return null. So if TS knows it's receiving at least one argument, it can also know that the min/max functions will return a Dayjs object. The rules are, or seem to be:

• If min/max receives at least one Dayjs object, it will return a Dayjs object.
• If it receives zero objects, it will return null.
• It may be passed the array as separate args or as a single arg with an array inside it; this does not change the behavior

TypeScript doesn't always know whether an array has at least one value: Dayjs[] only says it's an array that can only contain Dayjs objects, but it can contain zero or more. In these cases, TS can't know whether or not min/max will return null or a Dayjs object and must assume either can be returned. So:

• If args are ...Dayjs[] or Dayjs[]: return null | Dayjs
• If args are an explicitly empty array (min(): ...never[] or min([]): never[]): return only null
• If args are an array with at least one Dayjs element (min(foo, bar, baz): ...[Dayjs, ...Dayjs[]]) or min([foo, bar, baz]): [Dayjs, ...Dayjs[]]): return only Dayjs

The types are sorted by specificity (match "has Dayjs" first, match "explicitly empty" second, match "not sure" third) because TS will apply the first match it finds, so you don't want "not sure" at the top or it will always match.

Looking at the code now it looks like the function may also accept an explicit null (and return null). I can add typing for that, if it's valuable. That would be:

min(val: null): null; // explicitly null
min(vals: null | Dayjs[]): Dayjs | null // nullable but not definitely null
max(val: null): null;
max(vals: null | Dayjs[]): Dayjs | null // nullable but not definitely null

It does not look like min([null]) would be valid, as the check for truthiness happens before extracting args-as-array (so no min(val: null[])), and .isValid() is called on each element in the array, which would throw an error on null.

I know some codebases have "type tests" or similar but I couldn't find any here. If you would like something like that can you point me in their direction? I didn't see any in the original PR.

[bensaufley]

@iamkun is there anything else I can tell you about this PR, anything else I should do?

[iamkun]

excellent TypeScript example, Thanks.

[koengommers]

@iamkun Could this be released? Would help us out a ton.