WIP: Custom eslint rule to catch unsafe Record<string, T>

[dsernst]

See https://dev.to/sarioglu/avoiding-unintended-undefined-values-while-using-typescript-record-4igo for general explanation.

Specific Context That Led Here:

Hotfix https://github.com/dsernst/siv/commit/921b308da3c7560f6c0fde99531848d0b9753d69 was needed earlier today to fix a broken results page. Typescript could have warned about the fatal error but didn't because of the unsafe ballot_items_by_id: Record<string, Item> typing, which implies all string indices are defined.

Thus the front end crashed by trying to access undefined.type. Item does always has a .type, but the index was sometimes wrong, resulting in value undefined not Item.

Aside on slicing bug

Specifically, because of a different bug, item was getting sliced wrong, because of an extra digit Priorities_10 instead of Priorities_9 [had only tested with single digit number of approval voting items], led to item getting sliced as Priorities_ with an extra underscore.

This slicing bug was fixed in the next commit, https://github.com/dsernst/siv/commit/681b64265b6808b646c8f8d96291988f621423e5, which better type safety wouldn't have caught. But the type-safety would have kept the entire page from crashing, and in this case no possible user-visible issues, because the specific problem was really only applicable to IRV votes with 10 or more rankings, but those are limited to 3 at the moment anyway. If we did lift the 3 limit above 10, the type safety would have kept the page from crashing, so only rankings 10 and above would get screwed up, which is still not ideal but user-catchable and much more contained than crashing the entire Election Results page [would only effect the rather contrived edge case of an IRV vote where voters first 9 choices were all eliminated — can't think of any govt IRV elections that allow that many rankings. ]

Warning about type danger

The idea beginning to emerge here is to have a custom eslint rule that warns about directly referencing Record<string, SomeType>.

Better alternatives:

• Partial<Record<string, SomeType>> if you really don't know the indices. Eg user supplied data such as ballot columns.
  • A helper function like type SafeRecord<SomeType> = Partial<Record<string, SomeType>>, shorthand for the above.
  • Record<string, SomeType | undefined> another way to write the same thing
• Record<ExplicitListOfKeys, SomeType> when you do know the keys ahead of time.

Status — 100+ other potential cases?

The eslint rule is written and seems to be working now. Example:

/Users/dsernst/Documents/siv/src/want/IfYesForm.tsx
  13:64  error  Direct use of Record<string, T> is unsafe. Use Partial<Record<string, T>> or explicit indices  no-direct-record-string/no-direct-record-string

✖ 108 problems (108 errors, 0 warnings)

Pushing up this Work-in-Progress on it for now, although it might be a bit to really go through the 108 cases it already identified, fix their typing, and see if this is really the best solution.

[vercel]

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name	Status	Preview	Updated (UTC)
siv	❌ Failed (Inspect)		Apr 28, 2024 8:41am

[CLAassistant]

All committers have signed the CLA.