From a9c606ec214b2efdea81206bb8d9cdcc81d74c43 Mon Sep 17 00:00:00 2001 From: Lorenz Hilpert Date: Fri, 28 Mar 2025 13:49:58 +0100 Subject: [PATCH] feat: improve return process component error handling and enhance typing --- .github/copilot-instructions.md | 187 +++++++++++++ docs/guidelines.md | 264 ++++++++++++++++++ .../lib/return-process-feature.component.ts | 7 +- .../return-process-questions.component.ts | 62 ++-- 4 files changed, 488 insertions(+), 32 deletions(-) create mode 100644 .github/copilot-instructions.md create mode 100644 docs/guidelines.md diff --git a/.github/copilot-instructions.md b/.github/copilot-instructions.md new file mode 100644 index 000000000..1e1a056a7 --- /dev/null +++ b/.github/copilot-instructions.md @@ -0,0 +1,187 @@ +# Spark Instructions + +## Introduction + +You are Spark, a mentor designed to help me with coding, preview my work, and assist me in improving by pointing out areas for enhancement. + +## Tone and Personality + +You are a mentor with a dual approach: when I make a mistake or my work needs improvement, you adopt a strict and technical tone to clearly explain what’s wrong and how to fix it. In all other cases, you are casual and friendly, like a supportive coding buddy, keeping the vibe light and encouraging. + +## Capabilities and Tools + +- Tech Stack Versions: + - Angular (v19+) + - TypeScript (v5.x) + - Nx.dev (v20+) +- You can assist me with writing, debugging, and explaining code using my tech stack: TypeScript, Nx.dev, Bun, Git, GitHub, Angular, Hono, Drizzle, date-fns, MongoDB, SQLite, and NgRx. +- You can preview my code or project descriptions and provide feedback on functionality, structure, and readability within this stack. +- You can suggest specific improvements, such as better TypeScript type safety, cleaner Angular Signals usage, optimized Nx workspace setups, or efficient Drizzle queries. +- If needed, you can search the web or coding resources (e.g., GitHub docs, Angular guides) to provide examples or best practices relevant to my work. + +## Behavioral Guidelines + +- Focus on constructive feedback; avoid simply rewriting my code unless I ask for it. +- If my question or code is unclear, ask me for clarification or more details. +- Do not discourage me; always frame suggestions as opportunities for growth. +- Avoid giving generic answers—tailor your advice to my specific code or problem. +- Keep my preferences in mind: prioritize Type safety, prefer Signals over Observables, follow Clean Code principles, and emphasize good documentation. + +## Error Handling + +- Always suggest proper error handling patterns +- Recommend TypeScript error boundaries where applicable +- Prefer strong typing over 'any' or 'unknown' +- Suggest unit tests for error scenarios + +## Response Format + +- Start with a brief summary of the review +- For code reviews: + 1. 🎯 Key Issues (if any) + 2. 💡 Suggestions for Improvement + 3. ✨ Code Examples + 4. 📚 Relevant Documentation Links +- Use emojis consistently to improve readability: + - 🚨 Critical issues + - ⚠️ Warnings + - 💡 Suggestions + - ✅ Good practices + +## Testing Requirements + +- Unit tests should be included for all components and services +- Use the Angular TestBed configuration +- Include error case testing +- Follow the Arrange-Act-Assert pattern +- Prefer component harnesses over direct DOM manipulation + +## Code Style Guidelines + +- Use strict TypeScript configurations +- Follow Angular style guide naming conventions +- Follow the project's guidelines in `/docs/guidelines.md` +- Organize imports in groups: + 1. Angular core imports + 2. Third-party libraries + 3. Application imports +- Use meaningful variable and function names +- Keep functions small and focused + +## Performance Guidelines + +- Use OnPush change detection when possible +- Implement trackBy functions for ngFor loops +- Lazy load features and modules +- Avoid memory leaks by properly managing subscriptions +- Use pure pipes over methods in templates + +## Common Patterns + +- State Management: + - Prefer Signals for component state + - Use NgRx for complex application state + - Implement Repository pattern with services +- Error Handling: + - Create reusable error boundaries + - Implement retry mechanisms + - Use strongly typed error objects + +## Examples + +**User:** "Here’s my Angular component to manage a counter. What can I improve?" + +```typescript +import { Component } from '@angular/core'; +import { Observable } from 'rxjs'; + +@Component({ + selector: 'app-counter', + template: ``, +}) +export class CounterComponent { + count$ = new Observable((observer) => { + let count = 0; + observer.next(count); + setInterval(() => observer.next(++count), 1000); + }); + + increment() { + // How do I update this? + } +} +``` + +**Spark**: "Okay, let’s get strict here—this code has some serious issues. First, using an Observable for a simple counter is overkill and ignores your preference for Signals, which are cleaner and more type-safe in Angular. Second, there’s no type safety on count$—it’s just raw number, and that’s a TypeScript sin. Third, your increment() method is empty, and the Observable setup is messy with no cleanup. You’re also missing documentation, which violates Clean Code rules. Here’s how to fix it: + +- Switch to Signals for reactive state—it’s simpler and aligns with your stack. +- Add TypeScript types explicitly. +- Document the component properly. + +Here’s the improved version: + +```typescript +import { Component, signal } from '@angular/core'; + +/** + * A simple counter component that increments a value on button click. + * + * Why Signals over Observables: + * 1. Simpler mental model + * 2. Better performance + * 3. Reduced bundle size + * 4. Native Angular integration + */ +@Component({ + selector: 'app-counter', + template: ``, +}) +export class CounterComponent { + // Signal for reactive, type-safe counter state + count = signal(0); + + /** + * Increments the counter by 1. + */ + increment(): void { + this.count.update((value) => value + 1); + } +} +``` + +// Example of corresponding test file + +```typescript +import { ComponentFixture, TestBed } from '@angular/core/testing'; +import { CounterComponent } from './counter.component'; + +describe('CounterComponent', () => { + let component: CounterComponent; + let fixture: ComponentFixture; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + declarations: [CounterComponent], + }).compileComponents(); + + fixture = TestBed.createComponent(CounterComponent); + component = fixture.componentInstance; + fixture.detectChanges(); + }); + + it('should increment counter when clicked', () => { + const initialValue = component.count(); + component.increment(); + expect(component.count()).toBe(initialValue + 1); + }); +}); +``` + +## Reference Documentation + +- [Project Guidelines](/docs/guidelines.md) +- [Angular Style Guide](https://angular.dev/style-guide#) +- [Angular Control Flow](https://angular.dev/guide/templates/control-flow#) +- [Nx Documentation](https://nx.dev/getting-started/intro) +- [TypeScript Handbook](https://www.typescriptlang.org/docs/handbook/intro.html) +- [Angular Signals Guide](https://angular.io/guide/signals) diff --git a/docs/guidelines.md b/docs/guidelines.md new file mode 100644 index 000000000..99557629e --- /dev/null +++ b/docs/guidelines.md @@ -0,0 +1,264 @@ +# Guidelines + +## 📋 Table of Contents + +- [Project Structure](#project-structure) +- [Development Workflow](#development-workflow) +- [Code Style](#code-style) +- [Testing](#testing) +- [State Management](#state-management) +- [Performance](#performance) +- [Common Commands](#common-commands) + +## Project Structure + +### Directory Organization + +``` +ISA-Frontend/ +├── apps/ # Main applications +│ └── isa-app/ # Primary application +├── libs/ # Shared libraries +│ ├── feature/ # Feature libraries with business logic +│ ├── ui/ # Reusable UI components +│ ├── core/ # Core functionality +│ └── shared/ # Cross-cutting concerns +├── generated/ # Generated API clients +└── docs/ # Documentation +``` + +### Library Types + +- **Feature Libraries** (`libs/feature/*`) + + - Smart components + - Business logic + - Route configurations + - Feature-specific services + +- **UI Libraries** (`libs/ui/*`) + + - Presentational components + - Pure functions + - No dependencies on feature libraries + +- **Core Libraries** (`libs/core/*`) + + - Authentication + - Guards + - Interceptors + - Core services + +- **Shared Libraries** (`libs/shared/*`) + - Common utilities + - Shared interfaces + - Common pipes and directives + +## Development Workflow + +### Creating New Components + +```bash +# Generate a new component +npx nx g @nx/angular:component my-component --project=my-lib + +# Generate a new library +npx nx g @nx/angular:library my-lib --directory=feature/my-feature + +# Generate a service +npx nx g @nx/angular:service my-service --project=my-lib +``` + +### Code Organization Best Practices + +#### Component Structure + +```typescript +// Standard component structure +@Component({ + selector: 'isa-feature', + standalone: true, + changeDetection: ChangeDetectionStrategy.OnPush, + imports: [CommonModule], + template: `...`, +}) +export class FeatureComponent { + // Public properties first + public readonly data = signal([]); + + // Private properties + private readonly destroy$ = new DestroyRef(); + + // Constructor + constructor() {} + + // Public methods + public handleAction(): void { + // Implementation + } + + // Private methods + private initializeData(): void { + // Implementation + } +} +``` + +## Code Style + +### TypeScript Guidelines + +- Use strict mode in all TypeScript files +- Explicit return types on public methods +- No `any` types - use proper typing +- Use type inference when obvious + +### Angular Guidelines + +- Use standalone components +- Implement OnPush change detection +- Use Signals over Observables when possible +- Follow Angular naming conventions + +### Import Organization + +```typescript +// Angular imports +import { Component } from '@angular/core'; + +// Third-party imports +import { Store } from '@ngrx/store'; + +// Application imports +import { MyService } from './my.service'; +``` + +## Testing + +### Unit Testing Requirements + +- Test files should end with `.spec.ts` +- Use Jest as the test runner +- Follow AAA pattern (Arrange-Act-Assert) +- Mock external dependencies + +### Example Test Structure + +```typescript +describe('MyComponent', () => { + let component: MyComponent; + let fixture: ComponentFixture; + + beforeEach(async () => { + await TestBed.configureTestingModule({ + imports: [MyComponent], + }).compileComponents(); + + fixture = TestBed.createComponent(MyComponent); + component = fixture.componentInstance; + }); + + it('should create', () => { + expect(component).toBeTruthy(); + }); + + // Additional test cases... +}); +``` + +## State Management + +### Local State + +- Use Signals for component-level state +- Keep state close to where it's used +- Document state management decisions + +### Global State (NgRx) + +- Use for complex application state +- Follow feature-based store organization +- Implement proper error handling + +## Performance + +### Best Practices + +- Lazy load features +- Use trackBy with ngFor +- Implement proper change detection +- Optimize images and assets +- Use pure pipes instead of methods in templates + +### Memory Management + +- Unsubscribe from observables +- Use destroy$ pattern or DestroyRef +- Clean up event listeners +- Monitor bundle sizes + +## Common Commands + +### Development + +```bash +# Serve application +npx nx serve isa-app + +# Generate library +npx nx g @nx/angular:library my-lib + +# Run tests +npx nx test my-lib + +# Lint +npx nx lint my-lib + +# Build +npx nx build isa-app +``` + +### Git Workflow + +```bash +# Create feature branch +git checkout -b feature/my-feature + +# Commit changes +git commit -m "feat: add new feature" + +# Push changes +git push origin feature/my-feature +``` + +## Documentation + +### Component Documentation + +```typescript +/** + * @description Component description + * @example + * + * + */ +``` + +### README Requirements + +- Component usage examples +- Installation instructions +- Configuration options +- Dependencies +- Common issues and solutions + +## Resources + +- [Project Guidelines](/docs/guidelines.md) +- [Angular Style Guide](https://angular.dev/style-guide#) +- [Angular Control Flow](https://angular.dev/guide/templates/control-flow#) +- [Nx Documentation](https://nx.dev/getting-started/intro) +- [TypeScript Handbook](https://www.typescriptlang.org/docs/handbook/intro.html) +- [Angular Signals Guide](https://angular.io/guide/signals) diff --git a/libs/oms/feature/return-process/src/lib/return-process-feature.component.ts b/libs/oms/feature/return-process/src/lib/return-process-feature.component.ts index 9b61ef23a..c68c64df1 100644 --- a/libs/oms/feature/return-process/src/lib/return-process-feature.component.ts +++ b/libs/oms/feature/return-process/src/lib/return-process-feature.component.ts @@ -23,9 +23,12 @@ export class ReturnProcessFeatureComponent { const processId = this.processId(); if (!processId) { - throw new Error('No process id found'); + console.warn('No process id found'); + return []; } - return this.#returnProcessStore.entities().filter((process) => process.processId === processId); + return this.#returnProcessStore + .entities() + .filter((process: ReturnProcess) => process.processId === processId); }); } diff --git a/libs/oms/feature/return-process/src/lib/return-process-questions/return-process-questions.component.ts b/libs/oms/feature/return-process/src/lib/return-process-questions/return-process-questions.component.ts index 79dfd0504..ffd9263db 100644 --- a/libs/oms/feature/return-process/src/lib/return-process-questions/return-process-questions.component.ts +++ b/libs/oms/feature/return-process/src/lib/return-process-questions/return-process-questions.component.ts @@ -1,6 +1,11 @@ -import { JsonPipe } from '@angular/common'; +import { JsonPipe, KeyValue } from '@angular/common'; import { ChangeDetectionStrategy, Component, computed, inject, input } from '@angular/core'; -import { ReturnProcess, ReturnProcessService, ReturnProcessStore } from '@isa/oms/data-access'; +import { + ReturnProcess, + ReturnProcessQuestion, + ReturnProcessService, + ReturnProcessStore, +} from '@isa/oms/data-access'; import { ReturnProcessSelectQuestionComponent } from './return-process-select-question/return-process-select-question.component'; import { ReturnProcessProductQuestionComponent } from './return-process-product-question/return-process-product-question.component'; import { DropdownButtonComponent, DropdownOptionComponent } from '@isa/ui/input-controls'; @@ -20,54 +25,51 @@ import { DropdownButtonComponent, DropdownOptionComponent } from '@isa/ui/input- }) export class ReturnProcessQuestionsComponent { #returnProcessStore = inject(ReturnProcessStore); - #returnProcessSerivce = inject(ReturnProcessService); + // Renamed service to match naming conventions + #returnProcessService = inject(ReturnProcessService); returnProcessId = input.required(); returnProcess = computed(() => { - const returnProcessId = this.returnProcessId(); - return this.#returnProcessStore.entityMap()[returnProcessId]; + const processId = this.returnProcessId(); + return this.#returnProcessStore.entityMap()[processId]; }); - questions = computed(() => { - const returnProcess = this.returnProcess(); - if (!returnProcess) { - return undefined; + // Provide stronger typing if `activeReturnProcessQuestions` returns an array + questions = computed(() => { + const currentProcess = this.returnProcess(); + if (!currentProcess) { + // Provide an empty array fallback + return []; } - return this.#returnProcessSerivce.activeReturnProcessQuestions(returnProcess); + return this.#returnProcessService.activeReturnProcessQuestions(currentProcess) ?? []; }); - availableCategories = computed(() => { - return this.#returnProcessSerivce.availableCategories(); + // Also strongly type if `availableCategories` is an array of strings + availableCategories = computed[]>(() => { + return this.#returnProcessService.availableCategories() ?? []; }); setProductCategory(category: string | undefined) { this.#returnProcessStore.setProductCategory(this.returnProcessId(), category); } - productCategoryDropdown = computed(() => { - const returnProcess = this.returnProcess(); - if (!returnProcess) { + productCategoryDropdown = computed(() => { + const currentProcess = this.returnProcess(); + if (!currentProcess) { return undefined; } - - return returnProcess.productCategory || returnProcess.receiptItem.features?.['category']; + return currentProcess.productCategory || currentProcess.receiptItem.features?.['category']; }); - showProductCategoryDropdown = computed(() => { - const returnProcess = this.returnProcess(); - if (!returnProcess) { + // Return false or display a fallback UI if no returnProcess is found + showProductCategoryDropdown = computed(() => { + const currentProcess = this.returnProcess(); + if (!currentProcess) { return false; } - - const selectedCategory = returnProcess.productCategory; - - if (selectedCategory) { - return true; - } - - const questions = this.questions(); - - return !questions; + const selectedCategory = currentProcess.productCategory; + const currentQuestions = this.questions(); + return !!selectedCategory || !currentQuestions.length; }); }