Holistic refactor: modularize core, recycle shared helpers, generate the CSS light twin#47
Merged
Merged
Conversation
core.ts (1,470 lines) carried five jobs: schema derivation, the core control
constructors, the panel factory, markup enhancement, and the feedback chrome.
Pure code moves, no behavior change — core.ts stays the public entry and
re-exports tweaks/enhance/types, so both build outputs are unchanged:
schema.ts — metaFor/TYPED_META + the [data-tw] DATA_VALUE parsers
controls/basic.ts — slider, toggle, radiogrid, select, buttons, text,
number, folder + createControl
panel.ts — tweaks() and the panel API
enhance.ts — enhance() + the on-load auto-run
feedback.ts — toast, hint tooltip, toolbar button factories, copyText
icons.ts — the shared inline SVG icons
lazy.ts — the dynamic-import map + schema scan
…nto shared helpers The slider and interval had drifted-in-parallel copies of the same machinery; tokenize it once in shared.ts and point every surface at it: - normalizeRange() — the non-finite/inverted/degenerate range guard the slider and interval each hand-rolled (interval keeps its tuple-derived fallbacks via the defMin/defMax parameters) - rangeStep() — one keyboard model (arrows, shift-coarse, Page, Home/End) for the slider track, both interval handles, and the colour picker's alpha strip - overlapsText() — the label/value dodge overlap test, shared per handle - EASE_SPRING / EASE_GLIDE — the JS-side motion tokens; EASE_SPRING is the literal of the CSS --tw-ease-spring, previously duplicated inline at four call sites (slider glides, park transition, pill stretch) - chevronIcon(cls) — the folder and select chevrons were the same glyph declared twice - interval now rides the shared dragGesture() instead of re-implementing pointer capture, the single-pointer guard, and the four end paths - monitor.ts: strokeSeries() — the FPS graph and the numeric monitor shared the ring-buffer sparkline pen in two hand-rolled variants
The [data-tw-scheme="light"] block was a manual copy of the light media block, policed by a ~40-line drift checker that failed the build when the two diverged. Author the palette once: build.mjs now derives the attribute-forced twin from the @media (prefers-color-scheme: light) block — each :not([data-tw-scheme="dark"] …) :where() guard maps to its positive [data-tw-scheme="light"] … form, emitted right after the media block so the source-order precedence is unchanged. The generator keeps three guards: no hand-written forced-light rules may creep back into src, every rule inside the media block must carry a derivable :where() dark-guard, and the pair floor (≥3) still catches the twin system silently dropping out. dist/tweaks.css is byte-identical to the previous hand-maintained output.
Adding a control touches several hand-synced places (TYPED_META, DATA_VALUE, the registry, LAZY_IMPORT, single.ts). Two new guards pin them together: - TYPED_META is now typed Record<Exclude<SchemaObject["type"], "button">, …> and DATA_VALUE Partial<Record<SchemaObject["type"], …>>, so tsc itself fails the build when the tables and the public type union drift. - test/registry.test.mjs derives a meta from a minimal fixture of every declared type (bundled from src in split mode, so LAZY_IMPORT is populated) and asserts each resolves to a core-registered or lazily-loadable control, that no lazy chunk is orphaned, and that core/lazy registration don't overlap. test/state.test.mjs backfills the previously uncovered subsystems: persistence round-trip, presets (including the __proto__ key), undo/redo with redo-branch drop, toJSON/fromJSON with the JSON-pointer path escaping, setMany single-notify batching, and gradient value normalisation. CONTRIBUTING.md updated for the new module map, the generated CSS twin, and the registry test.
The refactor was cut before the review fixes landed, so rebasing it onto main moved core.ts's body into panel.ts / enhance.ts and left seven of them behind. Re-applied at their new addresses: - panel.ts: applySnapshot + applySet per-entry isolation, the header-drag `buttons === 0` bail, the undo history cap, doReset() + the RESET queue entry so reset() survives the lazy window, filter-needs-toolbar, the wireReset dblclick that reaches through the inline editor, and adopt() so a control's own destroy() reaches the panel's cleanups. - enhance.ts: writeDataValue + the copy-key dedup (needs schema.ts's hasOwn, which core.ts used to provide in-file). monitor.ts kept its stop() alongside the refactor's strokeSeries draw(). 61/61 tests, including all nine regression tests from the review — they are what proves the fixes came across intact rather than merely being present.
ryankiley
force-pushed
the
claude/codebase-refactoring-review-w7p8lt
branch
from
July 25, 2026 02:16
d78fa86 to
8c390af
Compare
ryankiley
marked this pull request as ready for review
July 25, 2026 02:17
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
1 participant
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.
Structural simplification with no behaviour change to the refactor itself. Rebased onto
main(post-#48) and extended with a fifth commit that carries the review fixes into the new module layout — see section 5, which is the part worth the closest read.Bundle sizes unchanged by this PR at ~19 KB split / ~34 KB single gzipped. Suite: 48 → 61 tests, all green.
1. Split
core.tsinto focused modulescore.ts(1,470 lines) carried five jobs. It's now the public entry (re-exports only), with the implementation split by responsibility:schema.ts(schema/[data-tw]→ control meta),controls/basic.ts(the always-loaded controls),panel.ts(tweaks()+ panel API),enhance.ts(markup path + auto-run),feedback.ts(toast/hint/toolbar),icons.ts,lazy.ts(dynamic-import map). Pure code moves.2. Recycle duplicated logic into shared helpers
The slider and interval had drifted-in-parallel copies of the same machinery, now written once in
shared.ts:normalizeRange()— the non-finite/inverted/degenerate range guardrangeStep()— one keyboard model (arrows / ⇧-coarse / Page / Home / End) for the slider, both interval handles, and the colour picker's alpha stripoverlapsText()— the label/value dodge overlap testEASE_SPRING/EASE_GLIDE— JS-side motion tokens (previously four inline copies of the CSS--tw-ease-springliteral)chevronIcon(cls)— the folder/select chevron declared oncedragGesture(); the FPS graph and numeric monitor share onestrokeSeries()sparkline pen3. Generate the forced-light CSS twin
The
[data-tw-scheme="light"]block was a hand-maintained copy of the light media block, policed by a ~40-line drift checker. The build now derives the twin from the media block (each:not([data-tw-scheme="dark"] …)guard maps to its positive form), so drift is impossible. Guards remain: no hand-written forced-light rules, every media-block rule must be derivable, and a pair floor catches the system dropping out. Output is byte-identical.4. Table-sync types + state-machinery tests
TYPED_META/DATA_VALUEare typed against the publicSchemaObject["type"]union — tsc fails the build if the tables andtypes.tsdrift.test/registry.test.mjscross-checksTYPED_META/DATA_VALUE/ the registry /LAZY_IMPORTvia minimal fixtures per control type.test/state.test.mjscovers previously untested subsystems: persistence round-trip, presets (including a__proto__key), undo/redo with redo-branch drop,toJSON/fromJSONpath escaping,setManysingle-notify batching, gradient normalisation.5. Carry the review fixes into the split modules
This branch was cut before #48 landed. Rebasing moved
core.ts's body intopanel.tsandenhance.ts, which left seven of #48's fixes behind — the rebase resolved cleanly in most files precisely because the code had moved rather than been edited in place, so nothing conflicted and the fixes silently didn't come across. Re-applied at their new addresses:panel.ts—applySnapshotandapplySetper-entry isolation; the header-dragbuttons === 0bail; the undo history cap;doReset()plus theRESETqueue entry soreset()survives the lazy window; filter-needs-toolbar; thewireResetdblclick that reaches through the inline editor; andadopt(), so a control's owndestroy()reaches the panel's cleanups.enhance.ts—writeDataValueand the copy-key dedup. This one neededhasOwnimported fromschema.ts, sincecore.tsused to provide it in-file; the build's tsc gate caught that.monitor.ts— kept itsstop()alongside the refactor'sstrokeSeriesdraw().All nine regression tests from #48 pass against the refactored modules. Those are the real check here: they were each written to fail against the pre-fix code, so they're what proves the fixes came across intact rather than merely appearing present to a grep.
Also verified in a browser against this build, not just jsdom: hostile-colour restore completes and notifies once (
a=5, z=9, notified=1), a buttonless hover leaves the panelinline,reset()in the lazy window applies, and the markup page enhances 9 controls with no[object Object]in anydata-valueand no console errors.Deliberately not done
enhance()auto-run to an opt-in entry (breaking change)strict/noImplicitAnyTypeScript (large churn; the table typing starts the ratchet)