Enhance migration templates with critical omission prevention

Add comprehensive guidance to prevent common migration oversights:
- Remove unused notification imports
- Replace hardcoded timeout values with constants
- Remove legacy wrapper functions
- Extract long class attributes to computed properties
- Replace literal strings with constants

Based on lessons learned from ContactQRScanShowView.vue migration.
Includes validation commands and specific examples for each pattern.
This commit is contained in:
Matthew Raymer
2025-07-09 04:42:05 +00:00
parent 71e7eb4fb6
commit d8e7fc90e5
7 changed files with 753 additions and 242 deletions

View File

@@ -273,6 +273,192 @@ This approach provides:
- **Localization**: Ready for future i18n support
- **Testability**: Constants can be imported in tests
## Critical Migration Omissions to Avoid
### 1. Remove Unused Notification Imports
**❌ COMMON MISTAKE**: Importing notification constants that aren't actually used
```typescript
// ❌ BAD - Unused imports
import {
NOTIFY_CONTACT_ADDED, // Not used
NOTIFY_CONTACT_ADDED_SUCCESS, // Not used
NOTIFY_CONTACT_ERROR, // Actually used
NOTIFY_CONTACT_EXISTS, // Actually used
} from "@/constants/notifications";
// ✅ GOOD - Only import what's used
import {
NOTIFY_CONTACT_ERROR,
NOTIFY_CONTACT_EXISTS,
} from "@/constants/notifications";
```
**How to check**: Use IDE "Find Usages" or grep to verify each imported constant is actually used in the file.
### 2. Replace ALL Hardcoded Timeout Values
**❌ COMMON MISTAKE**: Converting `$notify()` calls but leaving hardcoded timeout values
```typescript
// ❌ BAD - Hardcoded timeout values
this.notify.error(NOTIFY_CONTACT_ERROR.message, 5000);
this.notify.success(NOTIFY_CONTACT_ADDED.message, 3000);
this.notify.warning(NOTIFY_CONTACT_EXISTS.message, 5000);
this.notify.toast(NOTIFY_URL_COPIED.message, 2000);
// ✅ GOOD - Use timeout constants
this.notify.error(NOTIFY_CONTACT_ERROR.message, QR_TIMEOUT_LONG);
this.notify.success(NOTIFY_CONTACT_ADDED.message, QR_TIMEOUT_STANDARD);
this.notify.warning(NOTIFY_CONTACT_EXISTS.message, QR_TIMEOUT_LONG);
this.notify.toast(NOTIFY_URL_COPIED.message, QR_TIMEOUT_MEDIUM);
```
**Add timeout constants to your constants file**:
```typescript
// Add to src/constants/notifications.ts
export const QR_TIMEOUT_SHORT = 1000; // Short operations
export const QR_TIMEOUT_MEDIUM = 2000; // Medium operations
export const QR_TIMEOUT_STANDARD = 3000; // Standard success messages
export const QR_TIMEOUT_LONG = 5000; // Error messages and warnings
```
### 3. Remove Legacy Wrapper Functions
**❌ COMMON MISTAKE**: Keeping legacy notification wrapper functions that are inconsistent with the new system
```typescript
// ❌ BAD - Legacy wrapper function
danger(message: string, title: string = "Error", timeout = 5000) {
this.notify.error(message, timeout);
}
// Usage (inconsistent with rest of system)
this.danger(result.error as string, "Error Setting Visibility");
// ✅ GOOD - Direct usage of notification system
this.notify.error(result.error as string, QR_TIMEOUT_LONG);
```
**Why remove legacy wrappers**:
- Creates inconsistency in the codebase
- Adds unnecessary abstraction layer
- Often have unused parameters (like `title` above)
- Bypasses the centralized notification system benefits
### 4. Extract Long Class Attributes to Computed Properties
**❌ COMMON MISTAKE**: Leaving long class strings in template instead of extracting to computed properties
```typescript
// ❌ BAD - Long class strings in template
<template>
<div class="bg-amber-200 text-amber-900 border-amber-500 border-dashed border text-center rounded-md overflow-hidden px-4 py-3 my-4">
<button class="inline-block text-md uppercase bg-gradient-to-b from-blue-400 to-blue-700 shadow-[inset_0_-1px_0_0_rgba(0,0,0,0.5)] text-white px-4 py-2 rounded-md">
Set Name
</button>
</div>
</template>
// ✅ GOOD - Extract to computed properties
<template>
<div :class="nameWarningClasses">
<button :class="setNameButtonClasses">
Set Name
</button>
</div>
</template>
// Class methods
get nameWarningClasses(): string {
return "bg-amber-200 text-amber-900 border-amber-500 border-dashed border text-center rounded-md overflow-hidden px-4 py-3 my-4";
}
get setNameButtonClasses(): string {
return "inline-block text-md uppercase bg-gradient-to-b from-blue-400 to-blue-700 shadow-[inset_0_-1px_0_0_rgba(0,0,0,0.5)] text-white px-4 py-2 rounded-md";
}
```
**Benefits of extracting long classes**:
- Improves template readability
- Enables reusability of styles
- Makes testing easier
- Allows for dynamic class computation
### 5. Ensure ALL Literal Strings Use Constants
**❌ COMMON MISTAKE**: Converting `$notify()` calls to helpers but not replacing literal strings with constants
```typescript
// ❌ BAD - Literal strings in notification calls
this.notify.error("This QR code does not contain valid contact information.");
this.notify.warning("The contact DID is missing.");
this.notify.success("Registration submitted...");
// ✅ GOOD - Use constants for all static messages
this.notify.error(NOTIFY_QR_INVALID_QR_CODE.message);
this.notify.warning(NOTIFY_QR_MISSING_DID.message);
this.notify.success(NOTIFY_QR_REGISTRATION_SUBMITTED.message);
```
**Add constants for ALL static messages**:
```typescript
// Add to src/constants/notifications.ts
export const NOTIFY_QR_INVALID_QR_CODE = {
message: "This QR code does not contain valid contact information.",
};
export const NOTIFY_QR_MISSING_DID = {
message: "The contact DID is missing.",
};
export const NOTIFY_QR_REGISTRATION_SUBMITTED = {
message: "Registration submitted...",
};
```
### 6. Validation Checklist for Omissions
**Before marking migration complete, verify these items**:
```bash
# Check for unused imports
grep -n "import.*NOTIFY_" src/views/YourComponent.vue
# Then verify each imported constant is actually used in the file
# Check for hardcoded timeouts
grep -n "notify\.[a-z]*(" src/views/YourComponent.vue | grep -E "[0-9]{3,4}"
# Check for legacy wrapper functions
grep -n "danger\|success\|warning\|info.*(" src/views/YourComponent.vue | grep -v "notify\."
# Check for long class attributes (>50 chars)
grep -n "class=\"[^\"]\{50,\}" src/views/YourComponent.vue
# Check for literal strings in notifications
grep -n "notify\.[a-z]*(" src/views/YourComponent.vue | grep -v "NOTIFY_\|message"
```
### 7. Post-Migration Cleanup Commands
**Run these commands after migration to catch omissions**:
```bash
# Check TypeScript compilation
npm run lint-fix
# Run validation scripts
scripts/validate-migration.sh
scripts/validate-notification-completeness.sh
# Check for any remaining databaseUtil references
grep -r "databaseUtil" src/views/YourComponent.vue
# Check for any remaining $notify calls
grep -r "\$notify(" src/views/YourComponent.vue
```
## Template Logic Streamlining
### Move Complex Template Logic to Class
@@ -432,6 +618,8 @@ get itemCoordinates() {
- [ ] **Hardcoded timeouts replaced with `TIMEOUTS` constants**
- [ ] **Static messages use notification constants from `@/constants/notifications`**
- [ ] **Dynamic messages use literal strings appropriately**
- [ ] **Unused notification constants removed from imports but these can mean that notifications have been overlooked**
- [ ] **Legacy wrapper functions removed (e.g., `danger()`, `success()`, etc.)**
### Final Validation
- [ ] Error handling includes component name context