refactor: fold duplicated code into shared helpers, remove dead code - #18
Merged
Conversation
Every change here is behaviour-preserving; no output, exit code or public
export changes. The same test suite passes, and the smoke run produces the
same reports.
Shared helpers replacing copies:
- src/fs.ts — six modules each had their own try/stat/catch for "does this
exist / is it a file / is it a directory", plus two copies of toPosix.
- src/text.ts (was src/escape.ts) — escaping, plus the pluralising helper
that existed twice under two names and inline in five more places, the
markup-collapsing that existed three times, and the "WCAG x, EN 301 549 y"
reference built three times.
- audit/result.ts — pageUrl was written out three times, once with a comment
asking that it be kept in step with the others; the nodes-or-one-empty
placeholder three times; the all-outcomes list twice.
- audit/impact.ts — impactRank was four different sorters agreeing by hand
that an unclassified impact ranks worst; impactLabel four more.
- audit/issues.ts — the unevaluated-rule grouping and the coverage phrases
were written once per report.
- cli/report.ts — audit and baseline each spelled out the same engine
selection, the same BrowserUnavailableError handling and the same
write-or-stdout; every command had its own pc.red('error') plumbing.
- The HTML report now reads the JSON report's tally rather than recomputing
it, so the two cannot disagree about what a run found.
Dead code removed:
- vite: the info branch of the logger fallback was identical to its else.
- cli: isOutputFormat and isStatementFormat had no callers left.
- cli: parsePositive and parseConcurrency were the same function twice, and
five flag parsers were one shape written five times.
- astro: EaaKitIntegrationOptions restated IntegrationOptions field for
field; it is now an alias, so the two entries cannot drift.
- The four command actions rebuilt their options key by key against a
Record<string, unknown>, re-checking types commander had already parsed
and enforced. Commander omits an option nobody passed, so the parsed flags
are the command options.
One incidental improvement: unevaluated rules with the same page count now
tie-break by rule id rather than by encounter order, so two runs of one site
order that section identically.
Reuse misses the first pass left behind: - cli/init.ts kept its own stderr plumbing throughout; it now uses the same writers as every other command. - Four sites hand-rolled the same shape — a yellow line of advice with no `warning` prefix — which is distinct from a warning and now has its own writer. picocolors is down to the two files that still need it directly. - component.ts collapsed whitespace by hand after stripping tags. Altitude: cli/report.ts is renamed cli/command.ts. A report is a document everywhere else in this codebase, and that module holds neither — it is what every command does around the audit. Also documents the change in the changelog, including the one behavioural detail: unevaluated rules on equal page counts now tie-break by rule id. Verified: all four report formats are byte-identical to the pre-refactor build, the built bundle is the same size, and axe-core's reachability from the CLI entry is unchanged.
|
You have reached your Codex usage limits for code reviews. You can see your limits in the Codex usage dashboard. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
A behaviour-preserving cleanup pass over
src/. No report, exit code, error message or public export changes.src/goes from 8,599 to 8,378 lines (-221), with 911 lines deleted against 700 added — the new lines are mostly the shared helpers and their documentation.Duplication folded into shared helpers
try/stat/catchfor exists / is-file / is-directory, andtoPosixsrc/fs.tsWCAG x, EN 301 549 ysrc/text.ts(wassrc/escape.ts)pageUrl, the nodes-or-one-empty placeholder, the all-outcomes listaudit/result.tsunclassifiedlabelaudit/impact.tsaudit/issues.tsBrowserUnavailableErrorhandling, write-or-stdout,pc.red('error')plumbingcli/command.tsTwo of these were duplication the code already knew about:
pageUrlcarried the comment "Kept in step with the sequential runner's own URL derivation", and four separate sorters each re-derived by hand that an unclassified impact ranks with the most severe.The HTML report now reads the JSON report's tally rather than recomputing it, so the two cannot disagree about what a run found.
Dead code removed
infobranch of the logger fallback was character-for-character its ownelse.isOutputFormatandisStatementFormathad no callers;parsePositiveandparseConcurrencywere the same function twice; five flag parsers were one shape written five times.EaaKitIntegrationOptionsrestatedIntegrationOptionsfield for field. It is now an alias, so the Astro and Vite entries cannot drift apart.Record<string, unknown>, re-checking types commander had already parsed and enforced. Commander omits an option nobody passed, so the parsed flags are the command options — confirmed against commander directly, including--max-depth 0and--no-build.Verification
--per-page --manual), as is a written baseline file, modulo timestamps.playwright.test.ts) fails identically onmaster— this sandbox has Chromium at a path Playwright does not look in.pnpm lint,pnpm typecheckandpnpm smokeall clean.The one behavioural detail
Unevaluated rules appearing on the same number of pages now tie-break by rule id in the console report's "Not evaluated" section, rather than by the order they happened to be encountered. Two runs of one site now order that section identically — the guarantee the rest of the reports already make. It is noted in the changelog.
Reviewed
Ran
/simplify's four angles — reuse, simplification, efficiency, altitude — over the first commit. The second commit is what that found:cli/init.tshad been skipped by the writer pass; four sites shared an advisory shape distinct from a warning;component.tscollapsed whitespace by hand; andcli/report.tswas renamedcli/command.ts, since a report is a document everywhere else in this codebase and that module holds neither.🤖 Generated with Claude Code
https://claude.ai/code/session_01VvS86GmjhxyM8CYnkh3Xxg
Generated by Claude Code