jaykim88/code-refactoring

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.

1. 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

2. Name compound conditions

   • Any if with 2+ boolean expressions → extract to a named const before the if (examples in Anti-patterns)

3. 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)

4. 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

5. 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)

6. 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

7. 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

8. Split long functions/components

   • File > 500 LOC or function > 50 LOC → consider split
   • Single Responsibility per unit

9. 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)

10. 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

• [ ] tsc --noEmit clean
• [ ] ESLint warnings = 0
• [ ] All remaining any have justification comments
• [ ] Tests still pass
• [ ] Nesting depth ≤ 2 in refactored functions
• [ ] All Critical findings fixed; all Major findings scheduled

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

• Refactored code: one logical change per commit; commit format refactor(<scope>): <subject>
• PR description: bulleted list of major changes (extract / inline / rename / type-fix) with file references
• Verification block (paste into PR description):

  - tsc --noEmit: 0 errors
  - eslint .: 0 warnings
  - tests: N passing, 0 regressions
  - any usage: N (all justified with comments)
  

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.