Make hashmap keys required unless the key is an enum

[gustavo-shigueo]

Goal

Remove optional key from hashmaps unless the key type is an enum
Closes #442
Closes #374

Changes

Added new const to TS trait that determines whether or not a type should have a ? when used as a hashmap key

Checklist

• I have followed the steps listed in the Contributing guide.
• If necessary, I have added documentation related to the changes made.
• I have added or updated the tests related to the changes made.

[NyxCode]

Thanks for opening the PR!
I don't believe { [k in K]: V } is the correct binding for non-enum keys. That would require all values of K to be present in the map, no?
Wouldn't { [k: K]: V } as a default, with { [k in K]?: V } for enum keys, be the better choice here?

For context, here's the history of our bindings for hashmaps:

• We started with Record<K, V>
• We replaced that with { [k: K]: V } to allow for recursive types (Replace Record<K, V> with { [key: K]: V } #277)
• We replaced that with { [k in K]?: V } to allow for enum keys (Change HashMap to export mapped types #339)

[gustavo-shigueo]

in vs : doesn't make a difference for string or number which are the most likely uses of HashMap, and this implementation should catch all uses where K is a union of specific values (i.e. an enum), so it shouldn't make a difference:

That said, I do like key: K better and it's a pretty easy change to make

[NyxCode]

I thought about this more deeply in the meantime:

• I think we should go ahead with this.
• We should instruct users to use noUncheckedIndexedAccess if they want to keep their indexing checked for missing keys, which we forced onto them in Question to recent change Records<K, V> to { [key: K]: V } #338.
• I apologize for me previous criticism. I think we should stick with { [k in K]: V }. As you said, it should be equivalent.
• For enums, we have to keep using { [k in K]?: V }.
  • for being able to create empty hashmaps
  • without it, indexing always gives a value even with noUncheckedIndexedAccess
• I am very concerned about the churn we create for our users by making these changes too often.
  • Maybe we should make it configurable through environment variables, or at least offer a flag to keep the current behavior.

[timephy]

Maybe we should make it configurable through environment variables

I propose configuration like this to be done with feature flags 🙏🏽