1
0
mirror of https://github.com/bitwarden/browser synced 2025-12-06 00:13:28 +00:00

[PM-29235] Make a claude skill that modernizes angular code (#17445)

Experiment at creating a claude skill for modernizing Angular code.
This commit is contained in:
Oscar Hinton
2025-12-05 13:36:52 +01:00
committed by GitHub
parent ff7b625851
commit c036ffd775
3 changed files with 410 additions and 0 deletions

View File

@@ -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)

View File

@@ -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.*]`)

View File

@@ -52,6 +52,7 @@ jobs:
! -path "*/Cargo.lock" \ ! -path "*/Cargo.lock" \
! -path "./apps/desktop/macos/*" \ ! -path "./apps/desktop/macos/*" \
! -path "*/CLAUDE.md" \ ! -path "*/CLAUDE.md" \
! -path "*/SKILL.md" \
> tmp.txt > tmp.txt
diff <(sort .github/whitelist-capital-letters.txt) <(sort tmp.txt) diff <(sort .github/whitelist-capital-letters.txt) <(sort tmp.txt)