High Risk Refactor Guidelines

High-Risk Refactoring Guidelines

This document captures proven techniques for executing complex, high-risk refactors in this codebase. These guidelines were derived from the successful BlockEditor refactor (January 2026), which reduced a 1200+ line component to ~400 lines across multiple phases while maintaining full test coverage.

---

Core Principles

1. Risk stratification drives phase ordering

Not all extractions carry equal risk. Structure refactors into phases ordered by increasing risk:

Phase	Risk Level	What to Extract
First	Low	Pure utilities (no dependencies), simple hooks (minimal state), trivial components
Middle	Medium	Hooks with moderate interdependencies, components with clear boundaries
Last	High	Complex hooks with circular dependencies, components with extensive prop threading

Rationale: Low-risk extractions establish testing infrastructure and build confidence before tackling dangerous changes.

2. Investigation before implementation

Before writing any extraction code, conduct a thorough investigation phase:

• Search for all consumers of globals, shared state, or APIs you plan to change
• Document prop flows between parent and child components
• Map dependencies to understand what depends on what
• Identify circular dependencies that will require architectural solutions

This investigation phase should produce zero code changes but inform all subsequent design decisions.

3. Tests are safety rails, not refactoring targets

For newly extracted code:

• Write unit tests immediately after extraction
• Tests validate the extracted code works in isolation

For existing tests:

• Do NOT modify existing unit tests during the refactor
• Do NOT modify e2e tests at all
• Existing tests (especially e2e) are your contract validators
• If e2e tests pass, the refactor preserves behavior

Exception: Only modify existing tests if they test implementation details that necessarily changed, and only after the extraction is stable.

4. Atomic commits with explicit rollback points

Every extraction gets its own commit:

✅ "refactor: extract useModalManager hook"
✅ "refactor: extract BlockEditorFooter component"
❌ "refactor: extract useModalManager and useBlockSelection hooks"  // Too many changes

Backup branches: Create explicit backup branches before highest-risk phases:

git checkout -b feature-refactor-pre-phase-3
git checkout -  # Return to working branch

5. Hard stop on test failures

If e2e tests fail after an extraction:

1. STOP immediately — do not attempt inline fixes
2. Document the failure in a dedicated file with:
   • Which phase failed
   • Full error output
   • Analysis of likely cause
3. Consider reverting to the last passing checkpoint
4. Re-analyze before retrying

Heroic debugging during a refactor often makes things worse. A clean rollback is faster than a broken fix.

---

Extraction Patterns

Pattern A: Pure utility extraction

When to use: Logic that performs calculations or transformations without side effects.

Characteristics:

• Takes data in, returns data out
• No React hooks, no state, no DOM
• Easy to unit test in isolation

Example:

// Before: inline in component
const processedSteps = steps.map(step => { /* complex logic */ });

// After: extracted utility
import { processSteps } from './utils/step-processor';
const processedSteps = processSteps(steps);

Testing: Write comprehensive unit tests covering edge cases. This is low-risk and high-value test coverage.

Pattern B: Simple hook extraction

When to use: State management that doesn't depend on other custom hooks.

Characteristics:

• Uses only built-in React hooks (useState, useCallback, useMemo)
• Self-contained state with clear boundaries
• No external service dependencies

Example:

// Before: scattered state in component
const [isOpen, setIsOpen] = useState(false);
const [selectedId, setSelectedId] = useState<string | null>(null);

// After: consolidated in hook
const selection = useSelection();
// selection.isOpen, selection.selectedId, selection.open(), selection.close()

Testing: Use renderHook from @testing-library/react for isolated hook testing.

Pattern C: Complex hook with dependency injection

When to use: Hooks that need access to other hooks or services but shouldn't create them.

Characteristics:

• Receives dependencies via parameters
• Returns actions that operate on injected dependencies
• Avoids circular dependency by receiving callbacks

Example:

// Hook receives its dependencies
export function useRecordingActions(deps: {
  state: RecordingState;
  editor: EditorActions;
  onClear: () => void;  // Callback breaks circular dependency
}) {
  const { state, editor, onClear } = deps;
  
  const stopRecording = useCallback(() => {
    // Use injected dependencies
    editor.addBlocks(state.recordedSteps);
    onClear();  // Call injected callback
  }, [state, editor, onClear]);
  
  return { stopRecording };
}

Pattern D: Three-layer architecture for circular dependencies

When to use: When Hook A needs Hook B's clear function, but Hook B needs Hook A's state.

Solution: Split into three layers where each layer only depends on the previous:

┌─────────────────────────────────────────────────────┐
│  Layer 1: State (no dependencies)                   │
│  const state = useRecordingState();                 │
├─────────────────────────────────────────────────────┤
│  Layer 2: Persistence (reads state)                 │
│  const persistence = usePersistence({               │
│    state: state,                                    │
│    onRestore: state.restore,                        │
│  });                                                │
├─────────────────────────────────────────────────────┤
│  Layer 3: Actions (receives state + clear callback) │
│  const actions = useActions({                       │
│    state: state,                                    │
│    onClear: persistence.clear,                      │
│  });                                                │
└─────────────────────────────────────────────────────┘

Key insight: The cycle is broken because Layer 3 receives persistence.clear as a callback, rather than importing the persistence hook directly.

Pattern E: Interface-first component extraction

When to use: Before extracting a component that receives many props.

Process:

1. Define the interface in types.ts first (zero runtime risk)
2. Document which operations belong together
3. Commit the interface
4. Extract the component using the interface
5. Update consumers to pass the consolidated object

Example:

// Step 1: Define interface (commit this alone)
export interface BlockOperations {
  onEdit: (block: Block) => void;
  onDelete: (id: string) => void;
  onMove: (from: number, to: number) => void;
  // ... 25 more operations grouped logically
}

// Step 2: Extract component using interface
interface BlockListProps {
  blocks: Block[];
  operations: BlockOperations;  // Single prop replaces 29 individual props
}

---

Verification Protocol

After every extraction checkpoint, run the full verification sequence:

# 1. Format code (prevents CI failures)
npm run prettier

# 2. Type checking
npm run typecheck

# 3. Linting
npm run lint

# 4. Unit tests
npm run test:ci

# 5. End-to-end tests (critical - validates behavior preservation)
npm run e2e

Do not proceed to the next phase until all checks pass.

---

Preserving External Contracts

Test IDs are API surface

Document all data-testid attributes that e2e tests depend on:

Test ID	Purpose	Must Preserve
data-testid="block-editor"	Main container	Yes
data-testid="block-palette"	Footer palette	Yes
data-testid="view-mode-toggle"	Preview toggle	Yes

When extracting components, ensure these IDs remain in the DOM at the same structural level.

Window globals and side effects

If the codebase uses window globals (a code smell, but reality):

1. Search for ALL consumers before removal
2. Replace with React Context if globals serve cross-component communication
3. If globals are internal to one component, simple state replacement suffices

---

Troubleshooting Common Issues

Symptom	Likely Cause	Solution
TypeScript: Property 'X' does not exist	Missing prop in interface	Add to interface
E2E: Cannot find element	Test ID not preserved	Check data-testid in extracted component
Circular dependency error	Hook A imports Hook B which imports Hook A	Use three-layer architecture
Modal doesn't open	Event handler not wired	Verify callback prop names match
State not clearing	Cleanup function missing dependency	Check useCallback dependency array
Infinite loop	Object/array in useEffect deps	Use useMemo or extract primitives

---

Planning Template

When planning a large refactor, structure your plan with:

Phase 0: Investigation (no code changes)

• Search for all consumers of code being changed
• Document current prop flows
• Identify circular dependencies
• List test IDs that must be preserved
• Create backup branch

Phase 1: Low-risk extractions

• Pure utilities with unit tests
• Simple hooks with unit tests
• Trivial components
• Checkpoint: all tests passing

Phase 2: Medium-risk extractions

• Hooks with moderate dependencies
• Components with clear prop boundaries
• Create backup branch before highest-risk item
• Checkpoint: all tests passing

Phase 3: High-risk extractions

• Complex interdependent hooks (use dependency injection)
• Components with extensive prop threading (use interface consolidation)
• Checkpoint: all tests passing

Phase 4: Cleanup

• Update exports
• Remove dead code
• Final verification

---

When NOT to Use These Guidelines

These guidelines are for high-risk, complex refactors. Skip the ceremony for:

• Renaming a single function
• Adding a new prop to an existing component
• Extracting a 20-line utility
• Any change where rollback is trivial

Rule of thumb: If the refactor touches more than 3 files or involves state/hook extraction, use these guidelines. Otherwise, just make the change.

---

References

• Original BlockEditor refactor plans (January 2026)
• React anti-patterns rule: .cursor/rules/react-antipatterns.mdc
• Testing strategy: .cursor/rules/testingStrategy.mdc