List Resources: improve null checks on dates

[victorlin]

1

from https://github.com/nextstrain/nextstrain.org/pull/874#discussion_r1626765546

https://github.com/nextstrain/nextstrain.org/blob/aae874c06cd6d17616710c501ad73104a7c5e051/static-site/src/components/ListResources/Modal.tsx#L134-L140

@ivan-aksamentov's recommendation:

Why is this bang needed? the preceding line ensures that there is at lease one element in d.

Compiler is not smart enough to figure this out from this kind of the conditional. Connecting .length to .at() is tough for a type system.

What it can easily figure out is null checks. So a cleaner way would be to check and throw after non-fallible array access:

const t0 = d.at(0)?.getTime()
if(isNil(t0)) { // careful to not exclude the legit value `0`
    throw ...
}
// `t0` is guaranteed to be `number` in this branch

I would go further. Finding first and last value is a common enough "algorithm" that I would introduce a utility function:

export function firstLastOrThrow<T>(a: T[]): [T, T] {
    // TODO: use .at() or lodash head() and tail() along with null checks 
}

This would make the code very pretty and safe, with all dirty details hidden:

const [t0, t1] = firstLastOrThrow(dates) // guaranteed to return a pair of numbers or to  throw

2

from https://github.com/nextstrain/nextstrain.org/pull/874#discussion_r1626767197

https://github.com/nextstrain/nextstrain.org/blob/aae874c06cd6d17616710c501ad73104a7c5e051/static-site/src/components/ListResources/Modal.tsx#L148-L156

https://github.com/nextstrain/nextstrain.org/blob/aae874c06cd6d17616710c501ad73104a7c5e051/static-site/src/components/ListResources/Modal.tsx#L180-L182

@ivan-aksamentov's recommendation:

The problem with comments like this:

// presence of dates on resource has already been checked so this assertion is safe

is that 3 months from now a fresh intern comes and inserts new code between the check and the assertion (not knowing that this assertion exists - it's hard to find even when looking for it specifically) and then :boom:. Though, to be fair, it did not happen with js previously (due to lack of interns?)

Again, a small wrapper would make it safe and clean:

export function at<T>(a: T[], index: number): T {}

If you can find a fallback value, then something like this might work:

flatData[0]?.date ?? new Date()

This cannot fail and requires no hacks.

Continuing with the wrapper (though not directly applicable here sadly):

export function at<T>(a: T[], index: number, fallback?: T): T {}

Another option, although more involved, is to write a custom array type which always returns value or throws and never returns undefined. There might be libraries implementing non-empty arrays.

[ivan-aksamentov]

To shorten a bit:

• Try to narrow down the checks to null checks
• Make wrappers for common operations to avoid scattering the checks everywhere (there might be libs for that)

(Sadly, compilers are not yet smart enough to take array length checks into account. The length is only known at compile time, so in some cases it is plain impossible to decide at build time).