| name | refactor-safely |
| description | Restructure code that other people depend on without breaking them. Use when planning a large refactor or rewrite, splitting a monolith into packages, renaming or moving public symbols, changing internal architecture behind a stable API, or when the user says "I want to rewrite this" or "this code needs to be cleaned up". Covers strangler-fig migration, compatibility shims, codemods, feature flags, sequencing a refactor into reviewable PRs, and deciding when not to rewrite at all. |
Refactoring Safely
In a private codebase a refactor is a Tuesday. In a public one it is a coordination
problem with strangers who did not agree to be coordinated.
First: should you?
The Second System Effect is real and rewrites kill projects. Before agreeing:
| Reason to refactor | Verdict |
|---|
| A specific bug class keeps recurring here | Yes — targeted refactor |
| New feature is genuinely blocked by the structure | Yes — refactor exactly enough to unblock |
| Performance ceiling hit, profiled and proven | Yes |
| Onboarding contributors repeatedly stall in this file | Yes |
| "The code is ugly" | No — write tests instead, and reconsider in a month |
| "I'd write it differently now" | No |
| "Let's move to " | Only with a user-facing reason |
| "Full rewrite, v2, from scratch" | Almost never — see below |
The full rewrite trap. A from-scratch v2 means: shipping nothing for months,
maintaining v1 anyway, re-discovering every edge case that the ugly code in v1 was
silently handling, and a migration your users may simply decline. Projects have died
here. If the user wants a rewrite, propose the strangler-fig alternative first, and
only lose that argument once.
Rewriting is genuinely correct when the original's core assumption is wrong —
single-threaded when it must be concurrent, synchronous when it must stream, a data
model that cannot express the domain. Incremental refactoring cannot fix an axiom.
Preconditions
Do not start until all of these hold:
- Characterization tests exist. Before changing anything, write tests that pin
current behavior — including behavior you think is wrong. Bugs get depended upon;
those tests tell you which ones.
- The public API surface is snapshotted and asserted in CI (see
api-design).
- The refactor is sequenced into reviewable PRs. Nobody can review 5,000 lines,
including you in three weeks.
main stays releasable at every commit. A long-lived refactor branch
accumulates conflicts and blocks everyone else's work.
make test
git tag pre-refactor-baseline
cargo public-api > api-baseline.txt
The strangler fig
The pattern that lets you replace a system while it stays in production.
- Put a seam around the old implementation — an interface, a facade, a module
boundary. This is a pure-mechanical, zero-behavior-change PR. Merge it alone.
- Build the new implementation behind the same seam. It does not have to be
complete; it only has to be correct for the slice it claims.
- Route a slice of traffic/calls to the new path, behind a flag or an env var.
- Verify equivalence. Run both and compare outputs where feasible.
- Migrate slices one at a time, each its own PR, each independently revertible.
- Delete the old path once nothing routes to it — and actually delete it. A dead
branch left "just in case" is the thing future contributors trip over.
Each step ships. Each step is revertible. At no point is main broken. This is slower
in wall-clock time than a rewrite and dramatically faster in time-to-working-software.
Moving and renaming public symbols
The rule: never break the old path in the same release that introduces the new one.
export { parseConfig } from './config/parse.js';
export function parse(opts: Options) {
warnOnce('parse() moved to parseConfig() in pkg/config. Removed in v4.0.0.');
return parseConfig(opts);
}
Sequence: v3.1 adds the new path and deprecates the old → v3.x keeps both,
warning once per process → v4.0 removes the old path, with a migration guide and a
codemod.
For renames inside a package, keep the change mechanical and separate:
git mv src/utils.ts src/text/normalize.ts
Mixing a rename with a behavior change produces a diff where the behavior change is
invisible. Reviewers will miss it. This is one of the most reliable ways to ship a bug.
Ship a codemod
If a migration requires more than about ten mechanical edits per user, write the
codemod. The adoption difference is enormous — a one-command migration gets run; a
20-step guide gets deferred until the user is on an unsupported version filing bugs.
npx jscodeshift -t ./codemods/v4-rename-parse.js src/
python -m libcst.tool codemod v4_rename src/
comby 'parse(:[args])' 'parseConfig(:[args])' -i -f .go
cargo fix --edition
Ship it in the repo under codemods/, test it against your own codebase first, and
link it from the migration guide and the deprecation warning message.
Sequencing into PRs
A good refactor PR series, in order:
- Tests only. Characterization tests for current behavior. Merges instantly.
- Mechanical moves. Renames, file moves, extractions. No logic changes. Verify
with
git diff -M --stat showing pure renames.
- Seam introduction. Interface/facade added; old code unchanged behind it.
- New implementation, unused or flag-gated.
- Switch the default. One line. Trivially revertible — this is the point.
- Delete the old path.
- Cleanup. Now that both paths are gone, simplify what remained.
Each PR states in its description: what changed, what did not, and how to verify. Label
the series (refactor/parser) and track it in a single meta-issue so contributors know
which files are moving and can avoid conflicts.
Communicating with users and contributors
- Announce before starting, in an issue: what, why, what will break, when. Give
people the chance to object before you have spent the effort.
- Freeze the affected area for other contributors, or you will create conflicts
that make their PRs unmergeable — which is a good way to lose them.
- Publish the migration guide with the release, not after (see
docs-architecture).
- Support the previous major for a stated window. Backport security fixes to it.
"We support the previous major for 12 months" is a sentence that buys enormous
goodwill and should live in your README.
Verifying you didn't break anything
cargo public-api diff pre-refactor-baseline
npx api-extractor run --local && git diff etc/
for f in fixtures/*; do diff <(old-bin "$f") <(new-bin "$f") || echo "DIFF: $f"; done
For widely-depended-upon libraries, test against real downstream consumers before
release. Rust's crater and Go's module proxy analysis are the industrial versions;
the manual version — cloning your five biggest dependents and running their tests —
catches most of it and takes an afternoon.
Anti-patterns
- The v2 branch that never merges. If it has been open six months, it is dead;
extract what is salvageable and close it.
- Refactor mixed with features. Reviewers cannot separate them, so they approve
neither carefully.
- Renaming and changing logic in one commit.
- Removing a deprecated API earlier than announced. You published a date; honor it.
- No deprecation period because "nobody uses that." You cannot know. Check
GitHub code search and package download stats before asserting it.
- Reformatting the whole repo inside a refactor PR. Do formatting in its own
commit, add it to
.git-blame-ignore-revs, and never do it again.
- Breaking changes in a minor release because "it was technically a bug." The
users' upgrade did not care about your reasoning.