161 lines
5.3 KiB
Markdown
161 lines
5.3 KiB
Markdown
# ESLint Rule Analysis for RaceWithSOFViewModel.ts
|
|
|
|
## File Analyzed
|
|
`apps/website/lib/view-models/RaceWithSOFViewModel.ts`
|
|
|
|
## Violations Found
|
|
|
|
### 1. DTO Import (Line 1)
|
|
```typescript
|
|
import { RaceWithSOFDTO } from '@/lib/types/generated/RaceWithSOFDTO';
|
|
```
|
|
**Rule Violated**: `view-model-taxonomy.js`
|
|
**Reason**:
|
|
- Imports from DTO path (`lib/types/generated/`)
|
|
- Uses DTO naming convention (`RaceWithSOFDTO`)
|
|
|
|
### 2. Inline ViewData Interface (Lines 9-13)
|
|
```typescript
|
|
export interface RaceWithSOFViewData {
|
|
id: string;
|
|
track: string;
|
|
strengthOfField: number | null;
|
|
}
|
|
```
|
|
**Rule Violated**: `view-model-taxonomy.js`
|
|
**Reason**: Defines ViewData interface inline instead of importing from `lib/view-data/`
|
|
|
|
## Rule Gaps Identified
|
|
|
|
### Current Rule Issues
|
|
1. **Incomplete import checking**: Only reported if imported name contained "DTO", but should forbid ALL imports from disallowed paths
|
|
2. **No strict whitelist**: Didn't enforce that imports MUST be from allowed paths
|
|
3. **Poor relative import handling**: Couldn't properly resolve relative imports
|
|
4. **Missing strict import message**: No message for general import path violations
|
|
|
|
### Architectural Requirements
|
|
The project requires:
|
|
1. **Forbid "dto" in the whole directory** ✓ (covered)
|
|
2. **Imports only from contracts or view models/view data dir** ✗ (partially covered)
|
|
3. **No inline view data interfaces** ✓ (covered)
|
|
|
|
## Improvements Made
|
|
|
|
### 1. Updated `view-model-taxonomy.js`
|
|
**Changes**:
|
|
- Added `strictImport` message for general import path violations
|
|
- Changed import check to report for ANY import from disallowed paths (not just those with "DTO" in name)
|
|
- Added strict import path enforcement with whitelist
|
|
- Improved relative import handling
|
|
- Added null checks for `node.id` in interface/type checks
|
|
|
|
**New Behavior**:
|
|
- Forbids ALL imports from DTO/service paths (`lib/types/generated/`, `lib/dtos/`, `lib/api/`, `lib/services/`)
|
|
- Enforces strict whitelist: only allows imports from `@/lib/contracts/`, `@/lib/view-models/`, `@/lib/view-data/`
|
|
- Allows external imports (npm packages)
|
|
- Handles relative imports with heuristic pattern matching
|
|
|
|
### 2. Updated `test-view-model-taxonomy.js`
|
|
**Changes**:
|
|
- Added test for service layer imports
|
|
- Added test for strict import violations
|
|
- Updated test summary to include new test cases
|
|
|
|
## Test Results
|
|
|
|
### Before Improvements
|
|
- Test 1 (DTO import): ✓ PASS
|
|
- Test 2 (Inline ViewData): ✓ PASS
|
|
- Test 3 (Valid code): ✓ PASS
|
|
|
|
### After Improvements
|
|
- Test 1 (DTO import): ✓ PASS
|
|
- Test 2 (Inline ViewData): ✓ PASS
|
|
- Test 3 (Valid code): ✓ PASS
|
|
- Test 4 (Service import): ✓ PASS (new)
|
|
- Test 5 (Strict import): ✓ PASS (new)
|
|
|
|
## Recommended Refactoring for RaceWithSOFViewModel.ts
|
|
|
|
### Current Code (Violations)
|
|
```typescript
|
|
import { RaceWithSOFDTO } from '@/lib/types/generated/RaceWithSOFDTO';
|
|
import { ViewModel } from "../contracts/view-models/ViewModel";
|
|
|
|
export interface RaceWithSOFViewData {
|
|
id: string;
|
|
track: string;
|
|
strengthOfField: number | null;
|
|
}
|
|
|
|
export class RaceWithSOFViewModel extends ViewModel {
|
|
private readonly data: RaceWithSOFViewData;
|
|
|
|
constructor(data: RaceWithSOFViewData) {
|
|
super();
|
|
this.data = data;
|
|
}
|
|
|
|
get id(): string { return this.data.id; }
|
|
get track(): string { return this.data.track; }
|
|
get strengthOfField(): number | null { return this.data.strengthOfField; }
|
|
}
|
|
```
|
|
|
|
### Fixed Code (No Violations)
|
|
```typescript
|
|
import { ViewModel } from "../contracts/view-models/ViewModel";
|
|
import { RaceWithSOFViewData } from '@/lib/view-data/RaceWithSOFViewData';
|
|
|
|
export class RaceWithSOFViewModel extends ViewModel {
|
|
private readonly data: RaceWithSOFViewData;
|
|
|
|
constructor(data: RaceWithSOFViewData) {
|
|
super();
|
|
this.data = data;
|
|
}
|
|
|
|
get id(): string { return this.data.id; }
|
|
get track(): string { return this.data.track; }
|
|
get strengthOfField(): number | null { return this.data.strengthOfField; }
|
|
}
|
|
```
|
|
|
|
**Changes**:
|
|
1. Removed DTO import (`RaceWithSOFDTO`)
|
|
2. Moved ViewData interface to `lib/view-data/RaceWithSOFViewData.ts`
|
|
3. Imported ViewData from proper location
|
|
|
|
## Additional Recommendations
|
|
|
|
### 1. Consider Splitting the Rule
|
|
If the rule becomes too complex, consider splitting it into:
|
|
- `view-model-taxonomy.js`: Keep only DTO and ViewData definition checks
|
|
- `view-model-imports.js`: New rule for strict import path enforcement
|
|
|
|
### 2. Improve Relative Import Handling
|
|
The current heuristic for relative imports may have false positives/negatives. Consider:
|
|
- Using a path resolver
|
|
- Requiring absolute imports with `@/` prefix
|
|
- Adding configuration for allowed relative import patterns
|
|
|
|
### 3. Add More Tests
|
|
- Test with nested view model directories
|
|
- Test with type imports (`import type`)
|
|
- Test with external package imports
|
|
- Test with relative imports from different depths
|
|
|
|
### 4. Update Documentation
|
|
- Document the allowed import paths
|
|
- Provide examples of correct and incorrect usage
|
|
- Update the rule description to reflect the new strict import enforcement
|
|
|
|
## Conclusion
|
|
|
|
The updated `view-model-taxonomy.js` rule now properly enforces all three architectural requirements:
|
|
1. ✓ Forbids "DTO" in identifiers
|
|
2. ✓ Enforces strict import path whitelist
|
|
3. ✓ Forbids inline ViewData definitions
|
|
|
|
The rule is more robust and catches more violations while maintaining backward compatibility with existing valid code.
|