Typesafe dialogs (vibe-kanban) (#1304)
* Excellent! ✅ All tasks completed successfully. Let me create a summary document: ## ✅ Typesafe Dialogs Refactoring Complete ### What Was Changed **1. New Typesafe Modal System** ([lib/modals.ts](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/e9a3-typesafe-dialogs/frontend/src/lib/modals.ts)) - Created `defineModal<ResultType>(component)` to brand dialogs with return types - Created `showModal(modal, props)` with full type inference for props and return values - Added typed `hideModal()` and `removeModal()` helpers **2. Modals Registry** ([components/dialogs/index.ts](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/e9a3-typesafe-dialogs/frontend/src/components/dialogs/index.ts)) - Exported `Modals` object with all 27 dialogs - Each modal properly typed with its result type - Autocomplete available via `Modals.*` **3. ESLint Enforcement** ([frontend/.eslintrc.cjs](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/e9a3-typesafe-dialogs/frontend/.eslintrc.cjs)) - Blocks direct `NiceModal.show()` usage - Blocks `NiceModal.register()` usage - Restricts `NiceModal` imports to lib/modals.ts, App.tsx, and dialog files **4. Removed String Registrations** ([main.tsx](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/e9a3-typesafe-dialogs/frontend/src/main.tsx)) - Deleted 27 `NiceModal.register('string-id', Component)` calls - Dialogs now register automatically when used **5. Migrated All Call Sites** (14+ files) - Replaced `NiceModal.show('string-id', props)` → `showModal(Modals.X, props)` - All props and return types now fully typed - Legacy helper functions maintained for backward compatibility ### Usage Examples ```typescript // Before (not type-safe): const result = await NiceModal.show('onboarding'); // ❌ No autocomplete, no type checking, string can be misspelled // After (fully type-safe): const result = await showModal(Modals.Onboarding, {}); // ✅ result is typed as OnboardingResult // ✅ Props are checked at compile time // ✅ Autocomplete for modal names ``` ### Verification - ✅ **Type check passes**: `pnpm run check` - ✅ **Lint passes**: 0 errors (113 warnings, none related to this refactor) - ✅ **ESLint enforces pattern**: Prevents future regressions The refactoring is complete and all code is now fully typesafe! * Cleanup script changes for task attempt e9a36404-ba8e-4c84-9cca-1a1b7d68fdf7 * incremental * ## ✅ Component-Based Registration Complete! Successfully eliminated the centralized `Modals` registry and migrated to component-based dialog registration. ### What Changed **1. Each dialog now brands itself** (30 dialog files) - Added `defineModal<ResultType>()` at the source - Example: `export const FolderPickerDialog = defineModal<string | null>(FolderPickerDialogImpl)` **2. Removed centralized bottleneck** - Deleted `Modals` registry object from [components/dialogs/index.ts](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/e9a3-typesafe-dialogs/frontend/src/components/dialogs/index.ts) - Removed 100+ lines of duplication - index.ts now just re-exports for convenience **3. Fixed circular dependency** - Removed legacy helpers from [lib/modals.ts](file:///private/var/folders/m1/9q_ct1913z10v6wbnv54j25r0000gn/T/vibe-kanban/worktrees/e9a3-typesafe-dialogs/frontend/src/lib/modals.ts) - lib/modals.ts no longer imports from components **4. Updated all call sites** (24 files) - Changed from `showModal(Modals.X, props)` → `showModal(XDialog, props)` - Added direct component imports - Guided by TypeScript compiler errors ### Benefits Achieved ✅ **No duplication** - dialogs defined once in their own files ✅ **No circular deps** - clean import hierarchy ✅ **Easy to add dialogs** - just brand and use ✅ **Still fully typesafe** - props and return types inferred ✅ **Clear imports** - see exactly which dialogs are used ✅ **Compiler verified** - `pnpm run check` passes ✅ **Lint clean** - 0 errors * Cleanup script changes for task attempt e9a36404-ba8e-4c84-9cca-1a1b7d68fdf7 * Refactor dialog calling (vibe-kanban 8586934a) Instead of `showModal(CreatePRDialog)` it should be `CreatePRDialog.show()`, also we should add the same for `.hide()`. We should enforce the pattern using eslint, then use the lint errors to guide where to refactor. * reset merge conflicted dialogs to main * Fix dialog errors * fmt
This commit is contained in:
committed by
GitHub
parent
8fcc6f31b1
commit
a2df2334d0
@@ -34,6 +34,54 @@ module.exports = {
|
||||
],
|
||||
'@typescript-eslint/no-explicit-any': 'warn',
|
||||
'@typescript-eslint/switch-exhaustiveness-check': 'error',
|
||||
// Enforce typesafe modal pattern
|
||||
'no-restricted-imports': [
|
||||
'error',
|
||||
{
|
||||
paths: [
|
||||
{
|
||||
name: '@ebay/nice-modal-react',
|
||||
importNames: ['default'],
|
||||
message:
|
||||
'Import NiceModal only in lib/modals.ts or dialog component files. Use DialogName.show(props) instead.',
|
||||
},
|
||||
{
|
||||
name: '@/lib/modals',
|
||||
importNames: ['showModal', 'hideModal', 'removeModal'],
|
||||
message:
|
||||
'Do not import showModal/hideModal/removeModal. Use DialogName.show(props) and DialogName.hide() instead.',
|
||||
},
|
||||
],
|
||||
},
|
||||
],
|
||||
'no-restricted-syntax': [
|
||||
'error',
|
||||
{
|
||||
selector:
|
||||
'CallExpression[callee.object.name="NiceModal"][callee.property.name="show"]',
|
||||
message:
|
||||
'Do not use NiceModal.show() directly. Use DialogName.show(props) instead.',
|
||||
},
|
||||
{
|
||||
selector:
|
||||
'CallExpression[callee.object.name="NiceModal"][callee.property.name="register"]',
|
||||
message:
|
||||
'Do not use NiceModal.register(). Dialogs are registered automatically.',
|
||||
},
|
||||
{
|
||||
selector: 'CallExpression[callee.name="showModal"]',
|
||||
message:
|
||||
'Do not use showModal(). Use DialogName.show(props) instead.',
|
||||
},
|
||||
{
|
||||
selector: 'CallExpression[callee.name="hideModal"]',
|
||||
message: 'Do not use hideModal(). Use DialogName.hide() instead.',
|
||||
},
|
||||
{
|
||||
selector: 'CallExpression[callee.name="removeModal"]',
|
||||
message: 'Do not use removeModal(). Use DialogName.remove() instead.',
|
||||
},
|
||||
],
|
||||
// i18n rule - only active when LINT_I18N=true
|
||||
'i18next/no-literal-string': i18nCheck
|
||||
? [
|
||||
@@ -76,5 +124,13 @@ module.exports = {
|
||||
'@typescript-eslint/switch-exhaustiveness-check': 'off',
|
||||
},
|
||||
},
|
||||
{
|
||||
// Allow NiceModal usage in lib/modals.ts, App.tsx (for Provider), and dialog component files
|
||||
files: ['src/lib/modals.ts', 'src/App.tsx', 'src/components/dialogs/**/*.{ts,tsx}'],
|
||||
rules: {
|
||||
'no-restricted-imports': 'off',
|
||||
'no-restricted-syntax': 'off',
|
||||
},
|
||||
},
|
||||
],
|
||||
};
|
||||
|
||||
Reference in New Issue
Block a user