-
Notifications
You must be signed in to change notification settings - Fork 61
Description
Problem
The useOverlayData()
function currently returns Record<string, OverlayItem>
, but since the key type is string
, accessing with an incorrect string key returns undefined
at runtime while TypeScript still infers the return type as OverlayItem
.
This leads to runtime errors that could be prevented at compile time.
const overlayData = useOverlayData()
const d = overlayData['wrongId'] // type: OverlayItem, runtime: undefined
It would be beneficial to make users aware at compile time that the value could be undefined
.
Current Issue with Simple Type Union
If we simply add undefined
to the value type, it achieves the initial goal of having the value type as OverlayItem | undefined
:
export function createOverlaySafeContext() {
const [OverlayContextProvider, useOverlayContext] = createSafeContext<OverlayData>('overlay-kit/OverlayContext');
// ...others
function useOverlayData(): Record<string, OverlayItem | undefined> {
return useOverlayContext().overlayData;
}
return { OverlayContextProvider, useCurrentOverlay, useOverlayData };
}
However, this creates an issue with Object.values(overlayData)
usage, where the return type becomes (OverlayItem | undefined)[]
even though undefined
never actually exists as an array element.
Suggestion
I believe a change to the useOverlayData
interface is necessary.
If we change the implementation from object literal ({}
) to Map
, this issue could be resolved:
map.get('id') // type: OverlayItem | undefined
[...map.values()] // type: OverlayItem[]
Proposal: Use Map
internally while exposing ReadonlyMap
externally.
If you have a better opinion, please let me know. Thank you always.