fix-angular-fixmes
Key rules
- Fix the underlying issue — never just delete the suppression comment and leave broken code.
- Remove the complete comment block: FIXME line (if any) + TODO: Skipped block (if any) + eslint-disable-next-line line.
- Both FIXME-paired and standalone suppressions are the same migration debt.
- For Angular migration rules, prefer CLI schematics over manual edits.
- Do NOT convert service observables to signals (ADR-0027).
- For OnPush and signals patterns, the
angular-modernizationskill is the authoritative source — this skill only owns the ESLint suppression mechanics.
Step 1: Discover all suppressions
Use the Grep tool to find suppressions in the target path:
- Pattern
eslint-disable— finds all eslint suppressions - Pattern
FIXME.*CL-— finds Angular FIXME-tracked ones specifically
Group results by rule name. Two forms appear in this codebase:
Form A — FIXME-paired (a FIXME tracking comment sits above the disable):
// FIXME(https://bitwarden.atlassian.net/browse/CL-764): Migrate to OnPush
// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection
Form B — Standalone (disable without a FIXME, or with a CLI skip comment):
// eslint-disable-next-line @angular-eslint/prefer-on-push-component-change-detection
// TODO: Skipped for signal migration because:
// Accessor inputs cannot be migrated as they are too complex.
// FIXME(https://bitwarden.atlassian.net/browse/CL-903): Migrate to Signals
// eslint-disable-next-line @angular-eslint/prefer-signals
Both forms must be fixed the same way.
Rule reference
| Category | Rule | Section below |
|---|---|---|
| Angular | @angular-eslint/prefer-on-push-component-change-detection |
OnPush |
| Angular | @angular-eslint/prefer-signals |
Signals |
| Angular | @angular-eslint/prefer-output-emitter-ref |
Signals |
| Angular template | @angular-eslint/template/button-has-type |
HTML rules |
| TypeScript | @typescript-eslint/no-floating-promises |
no-floating-promises |
| TypeScript | @typescript-eslint/no-unused-vars |
no-unused-vars |
| TypeScript | @typescript-eslint/no-unsafe-function-type |
no-unsafe-function-type |
| RxJS | rxjs/no-async-subscribe |
rxjs rules |
| RxJS | rxjs-angular/prefer-takeuntil |
rxjs rules |
| Bitwarden | @bitwarden/platform/no-enums |
no-enums |
| Bitwarden | @bitwarden/components/no-bwi-class-usage |
HTML rules |
| General | no-restricted-imports |
no-restricted-imports |
| General | no-console |
no-console |
| General | no-empty |
no-empty |
| General | bare // eslint-disable-next-line |
bare disable |
| Tailwind | tailwindcss/no-custom-classname |
HTML rules |
OnPush
Rule: @angular-eslint/prefer-on-push-component-change-detection
Follow the OnPush guidance in the angular-modernization skill (add changeDetection: ChangeDetectionStrategy.OnPush, remove ChangeDetectorRef if only used for detectChanges()). Then remove the FIXME + eslint-disable-next-line lines.
@Directivedoes not supportchangeDetection— skip OnPush for pure directives.
Signals
Rules: @angular-eslint/prefer-signals, @angular-eslint/prefer-output-emitter-ref
Applies to @Input(), @Output(), @ViewChild, @ContentChild.
Follow the Signal Inputs, Outputs, and Queries guidance in the angular-modernization skill (prefer CLI schematics, then manual conversion). After each migration, manually remove the FIXME and eslint-disable-next-line lines, and any // TODO: Skipped for signal migration because: comment blocks.
Do NOT convert service observables to signals — only component-local state and decorator bindings (ADR-0027).
no-floating-promises
Rule: @typescript-eslint/no-floating-promises
A returned Promise is not handled. Pick one fix:
// 1. Await it (preferred in async functions)
await this.router.navigate(["/login"]);
// 2. void — explicit fire-and-forget
void this.router.navigate(["/login"]);
// 3. Chain .catch() for explicit error handling
this.router.navigate(["/login"]).catch((err) => this.logService.error(err));
Use void for navigation or toast calls that genuinely don't need awaiting. Use await when the result matters or you're already in an async context.
no-unused-vars
Rule: @typescript-eslint/no-unused-vars
// Remove unused variable
const unused = computeSomething(); // delete this line
// Or prefix with _ if it must be declared (e.g. destructuring)
const [_first, second] = array;
// Or suppress a catch variable (TypeScript 4.0+)
try { ... } catch { ... } // omit the variable entirely
no-unsafe-function-type
Rule: @typescript-eslint/no-unsafe-function-type
Replace the generic Function type with a specific signature:
Before
private callback: Function;
After — use the actual signature
private callback: () => void;
// or for unknown signatures:
private callback: (...args: unknown[]) => unknown;
RxJS rules
Rules: rxjs/no-async-subscribe, rxjs-angular/prefer-takeuntil
rxjs/no-async-subscribe — async callback inside .subscribe() swallows errors:
Before
this.service.data$.subscribe(async (value) => {
await this.process(value);
});
After — move async work into the pipe
this.service.data$
.pipe(
switchMap((value) => this.process(value)),
takeUntilDestroyed(),
)
.subscribe();
rxjs-angular/prefer-takeuntil — subscription without cleanup:
Before
this.service.data$.subscribe((value) => {
this.data = value;
});
After — add takeUntilDestroyed() (call in constructor or use destroyRef)
constructor() {
this.service.data$
.pipe(takeUntilDestroyed())
.subscribe((value) => { this.data = value; });
}
no-enums
Rule: @bitwarden/platform/no-enums
Convert TypeScript enums to const objects with type aliases (ADR-0025):
Before
enum CipherType {
Login = 1,
SecureNote = 2,
}
After
export const CipherType = Object.freeze({ Login: 1, SecureNote: 2 } as const);
export type CipherType = (typeof CipherType)[keyof typeof CipherType];
Update all import sites — the usage (CipherType.Login) stays the same.
no-restricted-imports
Rule: no-restricted-imports
The import is from a path that the ESLint config forbids. Steps:
- Read the context around the import to understand what is being imported.
- Check
eslint.config.mjsat the repo root (or the nearest config) for theno-restricted-importsrule to find the allowed alternative path. - Replace the import with the allowed path and remove the suppression.
Common cases: importing platform-internal modules directly instead of through the public API, or test-only helpers in non-test files.
no-console
Rule: no-console
// Remove debug statements
console.log("debug"); // delete
// Replace with the application logging service
this.logService.error("Something failed", error);
In test files (*.spec.ts), a console.error or console.warn spy may be intentional — in that case, set up the spy properly rather than suppressing:
jest.spyOn(console, "error").mockImplementation(() => {});
no-empty
Rule: no-empty
Empty catch blocks silently swallow errors:
Before
try {
await something();
// eslint-disable-next-line no-empty
} catch {}
After — handle or log the error
try {
await something();
} catch (e) {
// Intentionally ignored — operation is best-effort
}
// Or log it
try {
await something();
} catch (e) {
this.logService.warning("Operation failed", e);
}
Bare disable
Rule: bare // eslint-disable-next-line (no rule specified)
This disables ALL rules for the next line, which is always wrong. Steps:
- Remove the suppression and run
npm run lint:fixto see which specific rule triggers. - Fix the underlying issue using the appropriate section above.
- If the violation truly cannot be fixed (rare), replace the bare disable with a specific named rule.
HTML rules
@angular-eslint/template/button-has-type — Add an explicit type to every <button>:
Before
<button (click)="save()">Save</button>
After
<button type="button" (click)="save()">Save</button>
<!-- or type="submit" inside a <form> -->
@bitwarden/components/no-bwi-class-usage — Replace raw bwi-* icon classes with the <bit-icon> component or the appropriate icon token.
tailwindcss/no-custom-classname — Use a Tailwind utility class with the tw- prefix, or register the class in the Tailwind safelist. Never use arbitrary custom class names.
Step 2: Cleanup checklist per fixed instance
-
// FIXME(…)line removed (if present) -
// TODO: Skipped for signal migration because: …block removed (all lines, if present) -
// eslint-disable-next-line …line removed - Unused imports removed; new imports added as needed
- All in-class usages updated (e.g. signal reads need
())
Step 3: Validate
npm run lint:fix
Fix any errors that remain. Run npm run test if behaviour-critical code was changed.