Code Review Phase 2: Constants & Configuration

[jitu5]

Code Review Phase 2: Constants & Configuration

Purpose: Review all application constants — the "magic values" that control behavior across the app. These are referenced by nearly every other module.

Prerequisites: Phase 1 (Type System)
Estimated time: 1 hour
Files: 8

---

Files to Review (in order)

• src/constants/canvas.ts — Canvas sizing, positioning, and layout defaults
• src/constants/timing.ts — Debounce and delay values (auto-save: 500ms, validation: 300ms)
• src/constants/storage.ts — localStorage keys + safe accessor wrappers
• src/constants/events.ts — Custom window events + typed dispatch/subscribe helpers
• src/constants/dnd.ts — Drag-and-drop MIME types + helper functions
• src/constants/datasetTypes.ts — 75+ dataset type definitions + Kedro class mapping
• src/constants/fileTree.ts — File tree indentation constants
• src/constants/index.ts — Barrel export

---

Key Concepts

• as const assertions create readonly literal types from object literals
• Custom window events (CustomEvent) are used for cross-component communication outside of Redux
• safeGetItem/safeSetItem wrap localStorage with try-catch for environments where storage is unavailable

---

Focus Areas

• STORAGE_KEYS constants are now used consistently across the codebase (themeSlice, uiSlice, useAppInitialization, telemetry all reference STORAGE_KEYS). Verify there are no remaining hardcoded localStorage key strings.
• Does DATASET_TYPE_MAPPING in datasetTypes.ts cover every value in the DatasetType union from kedro.ts?
• Are debounce values sensible (500ms auto-save, 300ms validation)?
• Are event producer/consumer comments in events.ts accurate?
• Is the isConnectionId check in dnd.ts consistent with the one in ids.ts?

---

Next: Phase 4 (Redux Store & Slices)

[ravi-kumar-pilla]

I think this file should not be present in constants - https://github.com/kedro-org/kedro-builder/blob/main/src/constants/storage.test.ts

Is this intentional ? @jitu5

[ravi-kumar-pilla]

In preferencesMiddleware you’re not using the name StorageKey. You’re using the same type written inline. Can we instead do -

const persistOrWarn = (key: StorageKey, value: string) => { ??

[ravi-kumar-pilla]

• STORAGE_KEYS constants are now used consistently across the codebase (themeSlice, uiSlice, useAppInitialization, telemetry all reference STORAGE_KEYS). Verify there are no remaining hardcoded localStorage key strings.
• Does DATASET_TYPE_MAPPING in datasetTypes.ts cover every value in the DatasetType union from kedro.ts?
• Are debounce values sensible (500ms auto-save, 300ms validation)?
• Are event producer/consumer comments in events.ts accurate?
• Is the isConnectionId check in dnd.ts consistent with the one in ids.ts?

Done 👍

Overall looks good to me. Left minor comments above. Thank you @jitu5

[rashidakanchwala]

• STORAGE_KEYS constants are now used consistently across the codebase (themeSlice, uiSlice, useAppInitialization, telemetry all reference STORAGE_KEYS). Verify there are no remaining hardcoded localStorage key strings.
• Does DATASET_TYPE_MAPPING in datasetTypes.ts cover every value in the DatasetType union from kedro.ts?
• Are debounce values sensible (500ms auto-save, 300ms validation)?
• Are event producer/consumer comments in events.ts accurate?
• Is the isConnectionId check in dnd.ts consistent with the one in ids.ts?