From c036ffd775213f16a7278375e3aad2bd5ba46845 Mon Sep 17 00:00:00 2001
From: Oscar Hinton <Hinton@users.noreply.github.com>
Date: Fri, 5 Dec 2025 13:36:52 +0100
Subject: [PATCH] [PM-29235] Make a claude skill that modernizes angular code
 (#17445)

Experiment at creating a claude skill for modernizing Angular code.
---
 .claude/skills/angular-modernization/SKILL.md | 156 +++++++++++
 .../migration-patterns.md                     | 253 ++++++++++++++++++
 .github/workflows/lint.yml                    |   1 +
 3 files changed, 410 insertions(+)
 create mode 100644 .claude/skills/angular-modernization/SKILL.md
 create mode 100644 .claude/skills/angular-modernization/migration-patterns.md

diff --git a/.claude/skills/angular-modernization/SKILL.md b/.claude/skills/angular-modernization/SKILL.md
new file mode 100644
index 00000000000..187f715bde2
--- /dev/null
+++ b/.claude/skills/angular-modernization/SKILL.md
@@ -0,0 +1,156 @@
+---
+name: angular-modernization
+description: Modernizes Angular code such as components and directives to follow best practices using both automatic CLI migrations and Bitwarden-specific patterns. YOU must use this skill when someone requests modernizing Angular code. DO NOT invoke for general Angular discussions unrelated to modernization.
+allowed-tools: Read, Write, Glob, Bash(npx ng generate:*)
+---
+
+# Angular Modernization
+
+Transforms legacy Angular components to modern architecture using a two-step approach:
+
+1. **Automated migrations** - Angular CLI schematics for standalone, control flow, and signals
+2. **Bitwarden patterns** - ADR compliance, OnPush change detection, proper visibility, thin components
+
+## Workflow
+
+### Step 1: Run Angular CLI Migrations
+
+**⚠️ CRITICAL: ALWAYS use Angular CLI migrations when available. DO NOT manually migrate features that have CLI schematics.**
+
+Angular provides automated schematics that handle edge cases, update tests, and ensure correctness. Manual migration should ONLY be used for patterns not covered by CLI tools.
+
+**IMPORTANT:**
+
+- Always run the commands using `npx ng`.
+- All the commands must be run on directories and NOT files. Use the `--path` option to target directories.
+- Run migrations in order (some depend on others)
+
+#### 1. Standalone Components
+
+```bash
+npx ng generate @angular/core:standalone --path=<directory> --mode=convert-to-standalone
+```
+
+NgModule-based → standalone architecture
+
+#### 2. Control Flow Syntax
+
+```bash
+npx ng generate @angular/core:control-flow
+```
+
+`*ngIf`, `*ngFor`, `*ngSwitch` → `@if`, `@for`, `@switch`
+
+#### 3. Signal Inputs
+
+```bash
+npx ng generate @angular/core:signal-input-migration
+```
+
+`@Input()` → signal inputs
+
+#### 4. Signal Outputs
+
+```bash
+npx ng generate @angular/core:output-migration
+```
+
+`@Output()` → signal outputs
+
+#### 5. Signal Queries
+
+```bash
+npx ng generate @angular/core:signal-queries-migration
+```
+
+`@ViewChild`, `@ContentChild`, etc. → signal queries
+
+#### 6. inject() Function
+
+```bash
+npx ng generate @angular/core:inject-migration
+```
+
+Constructor injection → `inject()` function
+
+#### 7. Self-Closing Tag
+
+```bash
+npx ng generate @angular/core:self-closing-tag
+```
+
+Updates templates to self-closing syntax
+
+#### 8. Unused Imports
+
+```bash
+npx ng generate @angular/core:unused-imports
+```
+
+Removes unused imports
+
+### Step 2: Apply Bitwarden Patterns
+
+See [migration-patterns.md](migration-patterns.md) for detailed examples.
+
+1. Add OnPush change detection
+2. Apply visibility modifiers (`protected` for template access, `private` for internal)
+3. Convert local component state to signals
+4. Keep service observables (don't convert to signals)
+5. Extract business logic to services
+6. Organize class members correctly
+7. Update tests for standalone
+
+### Step 3: Validate
+
+- Fix linting and formatting using `npm run lint:fix`
+- Run tests using `npm run test`
+
+If any errors occur, fix them accordingly.
+
+## Key Decisions
+
+### Signals vs Observables
+
+- **Signals** - Component-local state only (ADR-0027)
+- **Observables** - Service state and cross-component communication (ADR-0003)
+- Use `toSignal()` to bridge observables into signal-based components
+
+### Visibility
+
+- `protected` - Template-accessible members
+- `private` - Internal implementation
+
+### Other Rules
+
+- Always add OnPush change detection
+- No TypeScript enums (use const objects with type aliases per ADR-0025)
+- No code regions (refactor instead)
+- Thin components (business logic in services)
+
+## Validation Checklist
+
+Before completing migration:
+
+- [ ] OnPush change detection added
+- [ ] Visibility modifiers applied (`protected`/`private`)
+- [ ] Signals for component state, observables for service state
+- [ ] Class members organized (see [migration-patterns.md](migration-patterns.md#class-member-organization))
+- [ ] Tests updated and passing
+- [ ] No new TypeScript enums
+- [ ] No code regions
+
+## References
+
+### Bitwarden ADRs
+
+- [ADR-0003: Observable Data Services](https://contributing.bitwarden.com/architecture/adr/observable-data-services)
+- [ADR-0025: No TypeScript Enums](https://contributing.bitwarden.com/architecture/adr/no-enums)
+- [ADR-0027: Angular Signals](https://contributing.bitwarden.com/architecture/adr/angular-signals)
+- [Bitwarden Angular Style Guide](https://contributing.bitwarden.com/contributing/code-style/web/angular)
+
+### Angular Resources
+
+- [Angular Style Guide](https://angular.dev/style-guide)
+- [Angular Migrations](https://angular.dev/reference/migrations)
+- [Angular CLI Schematics](https://angular.dev/tools/cli/schematics)
diff --git a/.claude/skills/angular-modernization/migration-patterns.md b/.claude/skills/angular-modernization/migration-patterns.md
new file mode 100644
index 00000000000..284f90a410f
--- /dev/null
+++ b/.claude/skills/angular-modernization/migration-patterns.md
@@ -0,0 +1,253 @@
+# Angular Migration Patterns Reference
+
+## Table of Contents
+
+- [Component Architecture](#component-architecture)
+- [Dependency Injection](#dependency-injection)
+- [Reactivity Patterns](#reactivity-patterns)
+- [Template Syntax](#template-syntax)
+- [Type Safety](#type-safety)
+
+## Component Architecture
+
+### Standalone Components
+
+Angular defaults to standalone components. Components should omit `standalone: true`, and any component specifying `standalone: false` SHALL be migrated to standalone.
+
+```typescript
+@Component({
+  selector: "app-user-profile",
+  imports: [CommonModule, ReactiveFormsModule, AsyncPipe],
+  templateUrl: "./user-profile.component.html",
+  changeDetection: ChangeDetectionStrategy.OnPush,
+})
+export class UserProfileComponent {}
+```
+
+### Class Member Organization
+
+```typescript
+@Component({...})
+export class MyComponent {
+  // 1. Inputs (public)
+  @Input() data: string;
+
+  // 2. Outputs (public)
+  @Output() valueChange = new EventEmitter<string>();
+
+  // 3. ViewChild/ContentChild
+  @ViewChild('template') template: TemplateRef<any>;
+
+  // 4. Injected dependencies (private/protected)
+  private userService = inject(UserService);
+  protected dialogService = inject(DialogService);
+
+  // 5. Public properties
+  public formGroup: FormGroup;
+
+  // 6. Protected properties (template-accessible)
+  protected isLoading = signal(false);
+  protected items$ = this.itemService.items$;
+
+  // 7. Private properties
+  private cache = new Map();
+
+  // 8. Lifecycle hooks
+  ngOnInit() {}
+
+  // 9. Public methods
+  public save() {}
+
+  // 10. Protected methods (template-accessible)
+  protected handleClick() {}
+
+  // 11. Private methods
+  private processData() {}
+}
+```
+
+## Dependency Injection
+
+### Modern inject() Function
+
+**Before:**
+
+```typescript
+constructor(
+  private userService: UserService,
+  private route: ActivatedRoute
+) {}
+```
+
+**After:**
+
+```typescript
+private userService = inject(UserService);
+private route = inject(ActivatedRoute);
+```
+
+## Reactivity Patterns
+
+### Signals for Component State (ADR-0027)
+
+```typescript
+// Local state
+protected selectedFolder = signal<Folder | null>(null);
+protected isLoading = signal(false);
+
+// Derived state
+protected hasSelection = computed(() => this.selectedFolder() !== null);
+```
+
+### Prefer computed() Over effect()
+
+Use `computed()` for derived values. Use `effect()` only for side effects (logging, analytics, DOM sync).
+
+**❌ Bad:**
+
+```typescript
+constructor() {
+  effect(() => {
+    const id = this.selectedId();
+    this.selectedItem.set(this.items().find(i => i.id === id) ?? null);
+  });
+}
+```
+
+**✅ Good:**
+
+```typescript
+selectedItem = computed(() => this.items().find((i) => i.id === this.selectedId()) ?? null);
+```
+
+### Observables for Service Communication (ADR-0003)
+
+```typescript
+// In component
+protected folders$ = this.folderService.folders$;
+
+// Template
+// <div *ngFor="let folder of folders$ | async">
+
+// For explicit subscriptions
+constructor() {
+  this.userService.user$
+    .pipe(takeUntilDestroyed())
+    .subscribe(user => this.handleUser(user));
+}
+```
+
+### Bridging Observables to Signals
+
+Use `toSignal()` to convert service observables to signals in components. Keep service state as observables (ADR-0003).
+
+**Before:**
+
+```typescript
+private destroy$ = new Subject<void>();
+users: User[] = [];
+
+ngOnInit() {
+  this.userService.users$.pipe(takeUntil(this.destroy$))
+    .subscribe(users => this.users = users);
+}
+
+ngOnDestroy() {
+  this.destroy$.next();
+  this.destroy$.complete();
+}
+```
+
+**After:**
+
+```typescript
+protected users = toSignal(this.userService.users$, { initialValue: [] });
+```
+
+## Template Syntax
+
+### New Control Flow
+
+**Before:**
+
+```html
+<div *ngIf="user$ | async as user; else loading">
+  <p *ngFor="let item of user.items">{{ item.name }}</p>
+</div>
+<ng-template #loading>Loading...</ng-template>
+```
+
+**After:**
+
+```html
+@if (user$ | async; as user) { @for (item of user.items; track item.id) {
+<p>{{ item.name }}</p>
+} } @else {
+<p>Loading...</p>
+}
+```
+
+### Prefer Class/Style Bindings Over ngClass/ngStyle
+
+Use `[class.*]` and `[style.*]` bindings instead of `ngClass`/`ngStyle`.
+
+**❌ Bad:**
+
+```html
+<div [ngClass]="{ 'active': isActive(), 'disabled': isDisabled() }">
+  <div [ngStyle]="{ 'width.px': width(), 'height.px': height() }"></div>
+</div>
+```
+
+**✅ Good:**
+
+```html
+<div [class.active]="isActive()" [class.disabled]="isDisabled()">
+  <div [style.width.px]="width()" [style.height.px]="height()"></div>
+</div>
+```
+
+## Type Safety
+
+### No TypeScript Enums (ADR-0025)
+
+**Before:**
+
+```typescript
+enum CipherType {
+  Login = 1,
+  SecureNote = 2,
+}
+```
+
+**After:**
+
+```typescript
+export const CipherType = Object.freeze({
+  Login: 1,
+  SecureNote: 2,
+} as const);
+export type CipherType = (typeof CipherType)[keyof typeof CipherType];
+```
+
+### Reactive Forms
+
+```typescript
+protected formGroup = new FormGroup({
+  name: new FormControl('', { nonNullable: true }),
+  email: new FormControl<string>('', { validators: [Validators.email] }),
+});
+```
+
+## Anti-Patterns to Avoid
+
+- ❌ Manually refactoring when CLI migrations exist
+- ❌ Manual subscriptions without `takeUntilDestroyed()`
+- ❌ TypeScript enums (use const objects per ADR-0025)
+- ❌ Mixing constructor injection with `inject()`
+- ❌ Signals in services shared with non-Angular code (ADR-0003)
+- ❌ Business logic in components
+- ❌ Code regions
+- ❌ Converting service observables to signals (ADR-0003)
+- ❌ Using `effect()` for derived state (use `computed()`)
+- ❌ Using `ngClass`/`ngStyle` (use `[class.*]`/`[style.*]`)
diff --git a/.github/workflows/lint.yml b/.github/workflows/lint.yml
index 48d3eca2f4e..f2e6db96b30 100644
--- a/.github/workflows/lint.yml
+++ b/.github/workflows/lint.yml
@@ -52,6 +52,7 @@ jobs:
             ! -path "*/Cargo.lock" \
             ! -path "./apps/desktop/macos/*" \
             ! -path "*/CLAUDE.md" \
+            ! -path "*/SKILL.md" \
             > tmp.txt
           diff <(sort .github/whitelist-capital-letters.txt) <(sort tmp.txt)