fix: 7 bugs and 6 minors from a code review

[ryankiley]

A review of the whole kit — a full read of src/ (~2,900 lines) plus build.mjs, site/, the tests and CI — then a second pass over my own findings. 7 confirmed findings and 6 minors, all fixed here. Tests go from 39 to 48.

Each new test was run against the source with the fix stashed, and kept only once it failed there with the symptom described. That run also hung the test runner for two minutes, which is itself the proof for finding 4 — the leaked interval keeps Node's event loop alive.

Crashes and lost state

1. parseColor read a color() space off a plain object (colour.ts)

color(constructor …) and color(__proto__ …) return an inherited member. It's truthy, so it passes the if (!space) guard and reaches the conversion maths as something that isn't a space:

p.set("c", "color(constructor 1 0 0)")
→ TypeError: Cannot read properties of undefined (reading '0')

Every other lookup table in the kit already guards this — TYPED_META and DATA_VALUE use hasOwn, the plot whitelist and presets bag are null-prototype, and expr.test.mjs asserts compileExpr("constructor(1)") === null. This one was missed. It's now null-prototype, so an unknown space falls to the existing default. No pollution and no code execution — just a crash where the kit should degrade.

2. applySnapshot and applySet didn't isolate per entry (core.ts)

One control that throws stopped the whole restore, and skipped the notify with it:

fromJSON({ values: { a: 5, c: <bad>, z: 9 } })
→ a = 5, z untouched, 0 listeners notified, exception escapes

The panel ends up half-restored with nothing told about it, and that partial state can be saved back over the good snapshot. Each entry is now wrapped, the same way createControl, metaFor, notify and applyConditionals already are. This is the more useful of the two fixes: it holds even if some other control throws later.

Stuck and leaked state

3. Header drag left dragId set, so the panel followed an unpressed cursor (core.ts)

No pointer is captured below the 4px drag threshold. So a release just off the header — a little drift onto .tw-body, or the page scrolling out from under a held button — never reaches endDrag. The next ordinary hover then crosses the threshold against the old origin:

before hover: { mode: "inline",   left: "(none)" }
after  hover: { mode: "floating", left: "276px", cls: "tw-panel is-grabbing is-dragging" }

Clicking once clears it. The slider, the interval and dragGesture all handle this with e.buttons === 0; the header didn't. That's the fix.

4. Monitor and FPS loops outlived destroy() on a panel that never mounted (monitor.ts)

Both loops idle through the "built but not appended yet" window on purpose, so they never see an unmount to stop on — and destroy() had no way to reach controls at all. The same happens for a panel mounted into a detached subtree.

mounted then destroyed   → extra timers: 0
never mounted, destroyed → extra timers: 1

Controls can now return a destroy(), which the panel adds to its cleanups.

5. Undo history had no cap

A deep clone of every value, committed every 350ms of editing, with nothing dropping off the end. Capped at 200 steps.

Things you couldn't reach

6. Double-click-to-reset on a slider was blocked by click-to-type (core.ts)

The readout is both the only reset target (.tw-slider-label is pointer-events: none) and the trigger for the 800ms hover editor. Resting the pointer there is what you'd do before double-clicking, so click #1 swapped the readout for the inline input and the dblclick went to the input instead:

	before	after
double-click, no hover first	resets ✅	resets ✅
hover 1s, then double-click	opens the editor, never resets ❌	resets ✅

The docs describe both on the same target: "hover the value and click to type (double-click resets)". Checked with real mouse input in Chrome; click-to-type still commits as before.

7. reset() during the lazy window did nothing

It went through resetBtn.click(), and HTMLElement.click() does nothing on a disabled control — the toolbar is disabled until assemble(). set, setMany and fromJSON all queued; reset didn't. It now queues with them and replays in call order.

8. opts.filter with toolbar: false built a filter you couldn't open

The search button lives in the toolbar, so nothing could set .is-searching. It now warns and skips instead.

Minors

• THEME_ALIASES, MODE_INTERP and INTERP_MODE made null-prototype — same shape as finding 1, harmless today.
• stepPrecision capped at toFixed's 100-digit limit. A step below 1e-100 threw a RangeError that dropped the whole control.
• Markup mode keyed its copy by label, so two controls with the same label overwrote each other. It also wrote object values onto data-value as "[object Object]" — the attribute its own parsers read back. It now uses data-key when present (suffixing real duplicates) and writes objects in the comma form, so a point round-trips.
• The plot's y = label moved from el()'s innerHTML argument to txt().
• npm audit fix for the esbuild and undici advisories. Both are dev-only; lockfile changes only, nothing ships.

What I checked and found fine

No XSS sink — every innerHTML and insertAdjacentHTML call takes a static icon constant, and host data goes through txt() / textContent. No prototype-pollution route via the schema, set(), dotted paths or presets, including a preset named __proto__. The plot evaluator's whitelist and length cap hold. CI is in good shape: actions pinned by SHA, least-privilege permissions, no pull_request_target, and tag/version verification before publish. The light-twin drift check in build.mjs is still a good invariant.

The second pass also changed my own conclusions. It moved the emphasis on finding 1 (the crash was the trigger, the missing isolation was the real problem), corrected my description of finding 3 (I first wrote it as "permanently undraggable", which is wrong), moved three findings down to minors, and dropped one I couldn't reproduce.

One thing to look at

Finding 3's fix changes how dragging behaves: a header press that ends without a proper release now ends the drag rather than leaving it open. That matches the other drag surfaces, but it's the only change here that affects an interaction rather than a failure path.

Verification

npm test green (48/48), build clean, sizes unchanged at ~19 KB split and ~34 KB single. CI green on Linux/Node 22. Interaction fixes checked with real mouse input in Chrome; docs site re-checked on the numbers, colour/gradient, monitors and markup pages with no console errors.