Incremental reactivity via @preact/signals-core + architecture decisions (part of #88)#89
Merged
Conversation
Add src/core/signal.js — a ~70-line signal()/effect()/batch() reactive core (100% covered) — and convert the tabs slice (state.tabs + state.activeTabId) to signals end-to-end to evaluate incremental, framework-free reactivity. - state.js: tabs/activeTabId are signals; activeTab()/tabsForSaved/pruneTabLinks read .value (insulating all 16 activeTab() callers from the change). - tabs.js: selectTab/newTab/loadIntoNewTab/closeTab just mutate the signals; refresh() is deleted. - app.js: one effect() reads the tab signals and repaints the strip + editor + results + Save button — the old refresh(), but self-triggering and impossible to forget. Result-data repaints still call renderResults directly. - Test churn: per-file gate stays green (1033 tests). tabs.test.js loses its "selectTab triggers repaint" assertions — that responsibility moved to the app-level effect — and asserts state transitions instead. Spike only (branch spike/signals); not wired beyond the tabs slice. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KQ6VFCwwUWnU3YgHQK2K5k
Second slice, chosen because sidePanel has NO accessor helper (unlike tabs' activeTab()), so every reader changes — the worst case for churn. - state.js: sidePanel is a signal. - saved-history.js: 6 reads → .value; switchTo() sets .value (filter cleared first, since the effect runs synchronously on assignment) and drops its manual renderSavedHistory() call. - app.js: an effect() repaints the side panel on sidePanel change; the history-record bridge read → .value. - Tests: saved-history.test.js (19), state.test.js (2), app.test.js (1) — all mechanical .value. Gate green (1033 tests). Finding: with no helper, churn is purely mechanical and concentrated in the panel's own test file; no behavioral test rewrites were needed (unlike tabs, where the repaint responsibility relocated). Confirms the pattern generalizes. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KQ6VFCwwUWnU3YgHQK2K5k
Records the reactivity decision, the measured option comparison (in-house signals ~0.45KB vs Preact ~7.3KB vs Solid/React), the two-slice spike evidence, and the per-slice + accessor-helper migration rule. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KQ6VFCwwUWnU3YgHQK2K5k
Drop-in swap on top of the two converted slices: @preact/signals-core has the same .value / signal / effect / batch API, so the only changes are import lines + deleting src/core/signal.js and its test. Suite green (1022 tests; -11 from removing the primitive's own tests). Adds @preact/signals-core ^1.14.3. Trade vs hand-rolled: +1.4 KB gzip artifact (~1%) and one deliberate dependency, in exchange for a battle-tested, glitch-free core with computed()/untracked() and no ~70 lines + 178 test lines of reactive code to own. NOTE if adopted: add @preact/signals-core to THIRD-PARTY-NOTICES.md and update ADR-0001 to record signals-core as the chosen primitive. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KQ6VFCwwUWnU3YgHQK2K5k
Update ADR-0001 (decision + measured 3-way comparison: signals-core +1.4KB vs
hand-rolled +0.45KB vs Preact +7.3KB), add the @preact/signals-core MIT notice
to THIRD-PARTY-NOTICES.md (two→three inlined deps), and fix the build.mjs
dependency comments. CLAUDE.md rule 4 ('two deps') to be updated on adoption.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01KQ6VFCwwUWnU3YgHQK2K5k
resultView/running become signals; the results pane and Run button now repaint via effects in createApp instead of manual setRunBtn/renderResults calls in the run flow. The tab effect drops renderResults — a new results effect reads activeTabId + resultView + running (and, via activeTab(), tabs). Run-start writes are batched, and the run bookkeeping (runT0, elapsed_ns) is set *before* the run signals flip, since the effects fire synchronously and read it. The format-error path keeps its explicit renderResults (an in-place tab.result write with no signal change). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FLVYgQpkbEbHKwEAA4aawZ
…o signals The header title (name + unsaved-changes dot) repaints via a libraryName/ libraryDirty effect in createApp instead of manual updateLibraryTitle/ renderLibraryTitle calls scattered across saved-history.js, file-menu.js, and the save popover. Removed the now-obsolete app.updateLibraryTitle seam. The editingLibrary-driven renders stay explicit (editingLibrary is not a signal); finish() now leaves edit mode before renameLibrary so the title effect repaints the button view rather than a transient input. The data-driven calls (saveJsonAction, afterLibraryChange, favorite/delete/rename) are dropped — equality-gated signals only skip a repaint when the title is already correct. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FLVYgQpkbEbHKwEAA4aawZ
…ADR-0001 Accepted Bump CLAUDE.md rule 4 from two to three bundled runtime deps (adds @preact/signals-core, pointing at ADR-0001). Move ADR-0001 to Accepted and reconcile its "behind accessor helpers" wording with the validated helper-free `.value` reality: the accessor helper is now a guideline (for slices with many scattered readers), not a rule — the sidePanel, resultView/running, and libraryName/libraryDirty slices all converted fine without one. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FLVYgQpkbEbHKwEAA4aawZ
…dapters Record the architecture decision from #88 + the Preact spike (ADR-0001): state reactivity via @preact/signals-core, no React/Preact/Solid, and the hard third-party/high-frequency-pointer surfaces (editor, graphs, Chart, result grid) stay imperative behind injected seams. CodeMirror 6 pre-approved as the next dep behind an EditorPort seam (#21, enabling #84). Shared UI primitives are extracted on a second consumer, not built speculatively. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FLVYgQpkbEbHKwEAA4aawZ
Record the spike outcome in ADR-0001: the component model removes the schema panel's in-place-mutation anti-pattern and hits the 100% coverage gate, but at +6.8 KB gzip (measured) and a second render paradigm + icon/h/columns integration seams paid app-wide for one panel. Recommendation: do not adopt Preact now — stay signals-core; keep the schema panel a documented imperative exception (or convert it with replaced Set/Map signals). Revisit only when several complex rich-local-state panels are actually on the roadmap. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FLVYgQpkbEbHKwEAA4aawZ
…ep count Add a Working-discipline section — surface out-of-scope findings as `inbox` issues; reconcile forward work (roadmap #68, the issue's Goal/Acceptance, the ADR addendum, CHANGELOG [Unreleased], close obsoleted via Closes #N) in the same commit; convert friction into memory. Also reconcile the intro line with rule 4 (two → three bundled runtime deps, now that @preact/signals-core is in). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FLVYgQpkbEbHKwEAA4aawZ
…eleased] Per the new working-discipline rule: record the @preact/signals-core adoption (ADR-0001 / #88) and the rejected Preact spike as an [Unreleased] entry. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FLVYgQpkbEbHKwEAA4aawZ
Implement the forward-work-tracking discipline: a PR checklist item to reconcile affected tracked work (roadmap #68 / issue body / ADR / CHANGELOG), and a "Roadmap item" issue template (Goal / scope / key implementation / acceptance / re-evaluation trigger / tracking) for structured roadmap issues. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01FLVYgQpkbEbHKwEAA4aawZ
…ditor harness
tests/e2e/editor.html loads /src as raw ESM (no bundler), but signals adoption
added a bare `import { signal } from '@preact/signals-core'` to src/state.js
that the browser can't resolve → state.js fails to load → the harness never sets
window.__ready → every editor-insert / editor-alignment spec times out on
page.waitForFunction. Add an import map pointing the bare specifier at the local
ESM build (the e2e server serves the repo root, so /node_modules is live),
mirroring how pipeline.html imports dagre. Unit tests + the bundle were
unaffected (node resolves bare specifiers; esbuild inlines them).
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01FLVYgQpkbEbHKwEAA4aawZ
# Conflicts: # CHANGELOG.md
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.
What
Incremental adoption of
@preact/signals-corefor state reactivity (ADR-0001 / #88), plus the architecture decisions and contributor discipline that came out of the spike.Code — Part A signal slices (full suite + per-file coverage gate green)
resultView+running→ signals; the results pane and Run button repaint viaeffect()s (manualsetRunBtn/renderResultsremoved; run-start writesbatch()ed, bookkeeping set before the flip so the synchronous effects read current values).libraryName+libraryDirty→ signals; the header title repaints via an effect; the now-unusedupdateLibraryTitleseam removed.tabs/activeTabId/sidePanelwere already converted.)Docs / decisions
spike/preact-schema(not merged).EditorPortseam — Consider CodeMirror 6 for the SQL editor (schema-aware autocomplete) #21) and a Working-discipline section (inboxfindings; reconcile forward work; friction → memory).CHANGELOG.md [Unreleased], a PR reconcile checkbox, and a "Roadmap item" issue template.Not here (follow-ups — see the #68 build order)
The schema slice (Phase 1 remainder), CM6 /
EditorPort(#21), and the editor-intelligence + graph work.Part of #88 — the migration continues; this lands the scalar slices + the settled architecture.
🤖 Generated with Claude Code