# Code Refactoring

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

- Skill: `jaykim88/code-refactoring` (Agent Skill)
- Install (CLI): `npx skillmds@latest add jaykim88/code-refactoring`
- Raw SKILL.md: https://api.skillmd.com/api/skills/jaykim88/code-refactoring/raw
- Safety review: pending
- Works with: Claude Code, Claude.ai, OpenAI Codex
- Category: Web & Frontend
- License: MIT
- Author: JayKim88 (https://skillmd.com/u/jaykim88)
- Updated: 2026-09-10
- Page: https://skillmd.com/skills/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.