Code Refactoring
Purpose
Improve readability and maintainability without changing behavior. Each refactor pass targets a small, scoped area and is verifiable by tests.
Universal — the refactoring principles (named conditions, guard clauses, magic literals → constants, extraction, immutability, parameter limits) are language-agnostic. The grep patterns and linter commands differ per stack.
Procedure
Precondition — establish a safety net. Refactoring preserves behavior only if behavior is observable. Check test coverage on the target area first; if it's low, add characterization tests before refactoring (see test-strategy). "Tests pass" proves nothing when there are no tests on the code you're changing.
Eliminate type escapes
- Find all type bypasses — including suppression comments (
@ts-ignore / @ts-expect-error), as any / <any> casts, and any hidden in generics (Record<string, any>). Suppression comments are the most common real-world escape hatch — audit them first.
- Replace each with a precise type,
unknown-equivalent + narrowing, or a generic. When a suppression is unavoidable, prefer @ts-expect-error (with a reason) over @ts-ignore — it self-removes when the underlying error is fixed.
- If truly justified, add a comment explaining why
Name compound conditions
- Any
if with 2+ boolean expressions → extract to a named const before the if (examples in Anti-patterns)
Apply guard clauses (early returns)
- Validate at the top, happy path flows down; nesting depth ≤ 2 (one guard per condition keeps the failing reason visible)
Extract magic literals to named constants
- No bare numeric or string literals with meaning
const MAX_RETRY_COUNT = 3 not if (retries === 3)
- Module-level constants:
SCREAMING_SNAKE_CASE
- Function-local:
camelCase
Extract duplicated logic
- Found in 2+ places → extract to
lib/ (utilities) or components/ (shared UI)
- One source of truth, no copy-paste
- For UI components, apply the
component-quality skill's extraction criteria (① props-expressible ② readability improves)
Limit function parameters to ≤ 2
- 3+ positional params = use an options object:
fn({ strict: true, async: false })
- Boolean flags signal multiple responsibilities — consider splitting into two functions
- Anti-pattern:
createUser(name, email, true, false, null) — unreadable at call site
Prefer immutability
- Class fields:
readonly where the value is set once
- Literal types:
as const for inferring narrow types (const config = { mode: 'strict' } as const)
- Arrays/objects you pass around:
ReadonlyArray<T>, Readonly<T> in type signatures
Split long functions/components
- File > 500 LOC or function > 50 LOC → consider split
- Single Responsibility per unit
Remove what-comments, keep why-comments
// increment counter — delete (the code says this)
// off-by-one because the API is 1-indexed — keep (explains a non-obvious why)
Verify (validation loop)
- Run
tsc --noEmit; if errors, fix them and re-run until clean before proceeding
- Run
eslint . --max-warnings 0; if violations, fix and re-run until clean
- Run the test suite; if any test regresses, restore behavior before claiming the refactor is done
- All
any that remain must have justification comments — if not, add or remove
Anti-patterns
| ❌ Anti-pattern |
✅ Correct |
if (user.role === 'admin' && !user.suspended && order.total > 0) |
Extract to named const: const canApprove = ...; if (canApprove) |
if (a) { if (b) { if (c) { return doIt() } } } |
Guard clauses, one per condition (keeps the failing reason visible): if (!a) return; if (!b) return; if (!c) return; return doIt() |
if (retries === 3) |
const MAX_RETRY = 3; if (retries === MAX_RETRY) |
function fetchUser(id: any) |
Use precise type or unknown + narrowing |
// increments counter (what-comment) |
Delete — code says this; only why-comments survive |
fn(true, false, null) (boolean trap) |
Options object: fn({ strict: true, async: false }) |
Severity tiers
| Tier |
Examples |
Action SLA |
| Critical |
any in security-critical paths (auth, payment, RLS); silent catch swallowing security errors |
Fix immediately |
| Major |
Nesting depth > 3; functions > 50 LOC with multiple responsibilities; magic literals in business logic |
Fix this sprint |
| Minor |
Stylistic naming inconsistencies; let where const would do |
Schedule within 2 sprints |
Completion Criteria
Stop & Ask (AI must pause for user approval)
- Before touching 5+ files in one refactor — large blast radius needs explicit sign-off
- Before changing exported function signatures (callers may break)
- Before deleting code that looks unused — verify usage with
grep -r '<symbol>' first; the user confirms it's actually dead
Output
Implementation
React + Next.js / TypeScript (default)
- Type-escape grep:
grep -rEn ': any($|[^a-zA-Z])|as any|<any>|@ts-(ignore|expect-error)' src/
- Type-check loop:
npx tsc --noEmit — if errors, fix and re-run until clean
- Lint loop:
npx eslint . --max-warnings 0 — if violations, fix and re-run
- Large mechanical refactors (rename / signature change across many files): use a codemod (
ts-morph, jscodeshift) or an autofixable ESLint rule (eslint --fix) rather than manual edits — safer, faster, and reviewable as one mechanical diff
- Immutability:
readonly modifier, as const, ReadonlyArray<T>, Readonly<T>
- Common React-specific anti-patterns: nested ternaries in JSX, mutating props,
useState derivation chains
Other stacks
- JavaScript (no TS): type escapes not applicable; use JSDoc +
tsc --checkJs; the rest applies as-is
- Python: type escape grep
# type: ignore; type check mypy . or pyright; immutability via Final[], frozen=True dataclasses, MappingProxyType
- Go: type escapes via
interface{} / any; go vet ./... + staticcheck; immutability via unexported fields + accessor methods
- Rust: type escapes via
unsafe; immutability is default (let is immutable, opt-in mut)
- Universal principles — guard clauses, named conditions, magic literals → constants, function param limits (≤ 2), extract duplication, why-comments-only — apply identically across all languages
Related skills
component-quality — for component-level extraction and cva/hook patterns
architecture-improvement — when refactoring crosses file/module boundaries
Reference
- Key insight encoded: Encapsulate conditionals into named functions/variables and use guard clauses to flatten nesting — every compound boolean in an
if is a missing named const. This is more readable AND easier to test than nested ternaries or chained &&.
- Caveats: clean-code-typescript adapts Robert C. Martin's Clean Code, whose stricter rules (≤ 2 params, aggressive function extraction) are context-dependent heuristics, not absolutes — apply judgment, especially in React where prop-rich components and longer composition functions are normal and fine.
1---2name: code-refactoring3description: Refactor TypeScript/React code for readability and maintainability — remove `any`, name compound conditions, apply guard clauses, extract magic literals to constants, deduplicate shared logic. Use during PR review, before a feature touches a complex area, or on a weekly cadence. Not for component-level extraction (use component-quality) or cross-file/module restructuring (use architecture-improvement).4license: MIT5---67# Code Refactoring89## Purpose10Improve readability and maintainability without changing behavior. Each refactor pass targets a small, scoped area and is verifiable by tests.1112**Universal** — the refactoring principles (named conditions, guard clauses, magic literals → constants, extraction, immutability, parameter limits) are language-agnostic. The grep patterns and linter commands differ per stack.1314## Procedure1516**Precondition — establish a safety net.** Refactoring preserves behavior only if behavior is observable. Check test coverage on the target area first; if it's low, add characterization tests *before* refactoring (see `test-strategy`). "Tests pass" proves nothing when there are no tests on the code you're changing.17181. **Eliminate type escapes**19 - Find all type bypasses — including suppression comments (`@ts-ignore` / `@ts-expect-error`), `as any` / `<any>` casts, and `any` hidden in generics (`Record<string, any>`). Suppression comments are the most common real-world escape hatch — audit them first.20 - Replace each with a precise type, `unknown`-equivalent + narrowing, or a generic. When a suppression is unavoidable, prefer `@ts-expect-error` (with a reason) over `@ts-ignore` — it self-removes when the underlying error is fixed.21 - If truly justified, add a comment explaining why22232. **Name compound conditions**24 - Any `if` with 2+ boolean expressions → extract to a named `const` before the `if` (examples in Anti-patterns)25263. **Apply guard clauses (early returns)**27 - Validate at the top, happy path flows down; nesting depth ≤ 2 (one guard per condition keeps the failing reason visible)28294. **Extract magic literals to named constants**30 - No bare numeric or string literals with meaning31 - `const MAX_RETRY_COUNT = 3` not `if (retries === 3)`32 - Module-level constants: `SCREAMING_SNAKE_CASE`33 - Function-local: `camelCase`34355. **Extract duplicated logic**36 - Found in 2+ places → extract to `lib/` (utilities) or `components/` (shared UI)37 - One source of truth, no copy-paste38 - For UI components, apply the `component-quality` skill's extraction criteria (① props-expressible ② readability improves)39406. **Limit function parameters to ≤ 2**41 - 3+ positional params = use an options object: `fn({ strict: true, async: false })`42 - Boolean flags signal multiple responsibilities — consider splitting into two functions43 - Anti-pattern: `createUser(name, email, true, false, null)` — unreadable at call site44457. **Prefer immutability**46 - Class fields: `readonly` where the value is set once47 - Literal types: `as const` for inferring narrow types (`const config = { mode: 'strict' } as const`)48 - Arrays/objects you pass around: `ReadonlyArray<T>`, `Readonly<T>` in type signatures49508. **Split long functions/components**51 - File > 500 LOC or function > 50 LOC → consider split52 - Single Responsibility per unit53549. **Remove what-comments, keep why-comments**55 - `// increment counter` — delete (the code says this)56 - `// off-by-one because the API is 1-indexed` — keep (explains a non-obvious why)575810. **Verify (validation loop)**59 - Run `tsc --noEmit`; if errors, fix them and re-run until clean before proceeding60 - Run `eslint . --max-warnings 0`; if violations, fix and re-run until clean61 - Run the test suite; if any test regresses, restore behavior before claiming the refactor is done62 - All `any` that remain must have justification comments — if not, add or remove6364## Anti-patterns6566| ❌ Anti-pattern | ✅ Correct |67|---|---|68| `if (user.role === 'admin' && !user.suspended && order.total > 0)` | Extract to named const: `const canApprove = ...; if (canApprove)` |69| `if (a) { if (b) { if (c) { return doIt() } } }` | Guard clauses, one per condition (keeps the failing reason visible): `if (!a) return; if (!b) return; if (!c) return; return doIt()` |70| `if (retries === 3)` | `const MAX_RETRY = 3; if (retries === MAX_RETRY)` |71| `function fetchUser(id: any)` | Use precise type or `unknown` + narrowing |72| `// increments counter` (what-comment) | Delete — code says this; only why-comments survive |73| `fn(true, false, null)` (boolean trap) | Options object: `fn({ strict: true, async: false })` |7475## Severity tiers7677| Tier | Examples | Action SLA |78|---|---|---|79| **Critical** | `any` in security-critical paths (auth, payment, RLS); silent catch swallowing security errors | Fix immediately |80| **Major** | Nesting depth > 3; functions > 50 LOC with multiple responsibilities; magic literals in business logic | Fix this sprint |81| **Minor** | Stylistic naming inconsistencies; `let` where `const` would do | Schedule within 2 sprints |8283## Completion Criteria84- [ ] `tsc --noEmit` clean85- [ ] ESLint warnings = 086- [ ] All remaining `any` have justification comments87- [ ] Tests still pass88- [ ] Nesting depth ≤ 2 in refactored functions89- [ ] All Critical findings fixed; all Major findings scheduled9091## Stop & Ask (AI must pause for user approval)9293- **Before touching 5+ files in one refactor** — large blast radius needs explicit sign-off94- **Before changing exported function signatures** (callers may break)95- **Before deleting code that *looks* unused** — verify usage with `grep -r '<symbol>'` first; the user confirms it's actually dead9697## Output98- **Refactored code**: one logical change per commit; commit format `refactor(<scope>): <subject>`99- **PR description**: bulleted list of major changes (extract / inline / rename / type-fix) with file references100- **Verification block** (paste into PR description):101 ```102 - tsc --noEmit: 0 errors103 - eslint .: 0 warnings104 - tests: N passing, 0 regressions105 - any usage: N (all justified with comments)106 ```107108## Implementation109110### React + Next.js / TypeScript (default)111- Type-escape grep: `grep -rEn ': any($|[^a-zA-Z])|as any|<any>|@ts-(ignore|expect-error)' src/`112- Type-check loop: `npx tsc --noEmit` — if errors, fix and re-run until clean113- Lint loop: `npx eslint . --max-warnings 0` — if violations, fix and re-run114- Large mechanical refactors (rename / signature change across many files): use a codemod (`ts-morph`, `jscodeshift`) or an autofixable ESLint rule (`eslint --fix`) rather than manual edits — safer, faster, and reviewable as one mechanical diff115- Immutability: `readonly` modifier, `as const`, `ReadonlyArray<T>`, `Readonly<T>`116- Common React-specific anti-patterns: nested ternaries in JSX, mutating props, `useState` derivation chains117118### Other stacks119- **JavaScript (no TS)**: type escapes not applicable; use JSDoc + `tsc --checkJs`; the rest applies as-is120- **Python**: type escape grep `# type: ignore`; type check `mypy .` or `pyright`; immutability via `Final[]`, `frozen=True` dataclasses, `MappingProxyType`121- **Go**: type escapes via `interface{}` / `any`; `go vet ./...` + `staticcheck`; immutability via unexported fields + accessor methods122- **Rust**: type escapes via `unsafe`; immutability is default (`let` is immutable, opt-in `mut`)123- **Universal principles** — guard clauses, named conditions, magic literals → constants, function param limits (≤ 2), extract duplication, why-comments-only — apply identically across all languages124125## Related skills126- `component-quality` — for component-level extraction and cva/hook patterns127- `architecture-improvement` — when refactoring crosses file/module boundaries128129## Reference130- **Key insight encoded**: Encapsulate conditionals into named functions/variables and use guard clauses to flatten nesting — every compound boolean in an `if` is a missing named const. This is more readable AND easier to test than nested ternaries or chained `&&`.131- **Caveats**: clean-code-typescript adapts Robert C. Martin's *Clean Code*, whose stricter rules (≤ 2 params, aggressive function extraction) are context-dependent heuristics, not absolutes — apply judgment, especially in React where prop-rich components and longer composition functions are normal and fine.