-
Notifications
You must be signed in to change notification settings - Fork 67
Open
Description
Problem Description
There's a naming issue in our overlay management system that could cause confusion. Currently, the OverlayData type contains a property also named overlayData, creating ambiguity and making the code less intuitive to understand.
Current structure:
export type OverlayData = {
current: OverlayId | null;
overlayOrderList: OverlayId[];
overlayData: Record<OverlayId, OverlayItem>; // Name duplication causes confusion
};Improvement Proposals
I suggest one of the following approaches to improve the naming:
Option 1: Change the Type Name
export type OverlayState = { // Changed from 'OverlayData' to 'OverlayState'
current: OverlayId | null;
overlayOrderList: OverlayId[];
overlayData: Record<OverlayId, OverlayItem>; // Property name remains unchanged
};This option better reflects the actual usage of this type in the codebase. Since it's used as the state parameter in overlayReducer, renaming it to OverlayState would make the relationship clearer:
export function overlayReducer(state: OverlayState, action: OverlayReducerAction): OverlayState {
// ...
}This would make it immediately obvious that we're dealing with state management for overlays, which aligns with the reducer pattern common in state management libraries.
Option 2: Change the Property Name
export type OverlayData = { // Type name remains unchanged
current: OverlayId | null;
overlayOrderList: OverlayId[];
overlays: Record<OverlayId, OverlayItem>; // Changed from 'overlayData' to 'overlays'
};jungpaeng
Metadata
Metadata
Assignees
Labels
No labels