From 5a0375970f87abb5cfb43a46f5fb981b9689ca5a Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Mon, 29 Jun 2026 23:02:28 +0200 Subject: [PATCH 01/14] spike: reactive signals primitive + convert the tabs state-slice MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01KQ6VFCwwUWnU3YgHQK2K5k --- src/core/signal.js | 95 ++++++++++++++++++++ src/main.js | 2 +- src/state.js | 14 +-- src/ui/app.js | 16 +++- src/ui/tabs.js | 51 +++++------ tests/unit/app.test.js | 8 +- tests/unit/main.test.js | 27 +++--- tests/unit/signal.test.js | 178 ++++++++++++++++++++++++++++++++++++++ tests/unit/state.test.js | 60 ++++++------- tests/unit/tabs.test.js | 51 +++++------ 10 files changed, 392 insertions(+), 110 deletions(-) create mode 100644 src/core/signal.js create mode 100644 tests/unit/signal.test.js diff --git a/src/core/signal.js b/src/core/signal.js new file mode 100644 index 0000000..6dc48dc --- /dev/null +++ b/src/core/signal.js @@ -0,0 +1,95 @@ +// A ~50-line reactive core. `signal()` holds a value and remembers which effects +// read it; `effect()` runs a function and re-runs it whenever a signal it read +// changes. This is the whole mechanism behind "the UI repaints itself" — enough +// to retire manual `renderX(app)` invalidation without adopting a framework. +// +// Pure by construction: no DOM, no globals beyond the module-local tracking +// pointer, so it lives in core/ and tests with plain stubs. + +// The effect currently running — set while an effect's body executes so that any +// signal read during it knows who to subscribe. null when no effect is active +// (e.g. a read from ordinary code), in which case the read just returns a value. +let activeEffect = null; +// While > 0, signal writes queue their subscribers instead of running them, so a +// multi-signal update (e.g. set the tab list AND the active id) repaints once. +let batchDepth = 0; +const queued = new Set(); + +/** + * A reactive box around `initial`. Reading `.value` *inside* an effect subscribes + * that effect to this signal; assigning a different `.value` re-runs every + * subscribed effect. Assigning an equal value is a no-op (no needless re-render). + */ +export function signal(initial) { + let value = initial; + const subs = new Set(); + return { + get value() { + if (activeEffect) { + subs.add(activeEffect); + activeEffect.deps.add(subs); + } + return value; + }, + set value(next) { + if (Object.is(next, value)) return; + value = next; + // Copy first: an effect re-subscribes as it re-runs, mutating `subs` + // mid-iteration. Inside a batch, queue instead (deduped by the Set). + if (batchDepth > 0) { + for (const eff of subs) queued.add(eff); + return; + } + for (const eff of [...subs]) eff.run(); + }, + }; +} + +/** + * Run `fn` immediately, and again whenever any signal it read changes. Each run + * re-derives the dependency set from what `fn` actually reads, so conditional + * dependencies are tracked correctly (a signal no longer read stops triggering + * re-runs). Returns a `dispose()` that unsubscribes the effect. + */ +export function effect(fn) { + const eff = { + deps: new Set(), + run() { + cleanup(eff); + const prev = activeEffect; + activeEffect = eff; + try { + fn(); + } finally { + activeEffect = prev; + } + }, + }; + eff.run(); + return () => cleanup(eff); +} + +/** + * Run `fn`, deferring every effect triggered by signal writes inside it until + * `fn` returns, then run each affected effect once. Nesting is depth-counted, so + * only the outermost batch flushes. Returns `fn`'s result. + */ +export function batch(fn) { + batchDepth++; + try { + return fn(); + } finally { + if (--batchDepth === 0) { + const run = [...queued]; + queued.clear(); + for (const eff of run) eff.run(); + } + } +} + +// Drop this effect from every signal it was subscribed to, and forget them, so +// the next run (or a dispose) starts from a clean dependency set. +function cleanup(eff) { + for (const subs of eff.deps) subs.delete(eff); + eff.deps.clear(); +} diff --git a/src/main.js b/src/main.js index 2dd0a13..27cc73d 100644 --- a/src/main.js +++ b/src/main.js @@ -61,7 +61,7 @@ export async function bootstrap(app, env) { catch { shared = { sql: '', chart: null }; } } if (shared.sql) { - const t0 = app.state.tabs[0]; + const t0 = app.state.tabs.value[0]; t0.sql = shared.sql; t0.name = 'Shared query'; if (shared.chart && shared.chart.cfg) { diff --git a/src/state.js b/src/state.js index 0aca88b..3a337d4 100644 --- a/src/state.js +++ b/src/state.js @@ -6,6 +6,7 @@ import { clamp } from './core/format.js'; import { mergeSaved } from './core/saved-io.js'; import { cloneChartCfg } from './core/chart-data.js'; import { loadJSON, saveJSON, loadStr, saveStr } from './core/storage.js'; +import { signal } from './core/signal.js'; /** A tab's chart state as a persistable payload `{ cfg, key }`, or null. */ export function tabChart(tab) { @@ -48,8 +49,11 @@ export function createState(read = { loadJSON, loadStr }) { sidebarPx: clamp(parseInt(read.loadStr(KEYS.sidebarPx, '248'), 10), 180, 420), editorPct: num(KEYS.editorPct, 45, 15, 85), sideSplitPct: num(KEYS.sideSplitPct, 58, 25, 85), - tabs: [newTabObj('t1')], - activeTabId: 't1', + // Reactive (signals): renderTabs + the editor/results/save-button repaint via + // an effect that reads these, so mutating them is all a caller does — no + // manual refresh() list to keep in sync. Read/write through `.value`. + tabs: signal([newTabObj('t1')]), + activeTabId: signal('t1'), schema: null, schemaError: null, schemaFilter: '', @@ -80,7 +84,7 @@ export function createState(read = { loadJSON, loadStr }) { /** The currently-active tab object (falls back to the first tab). */ export function activeTab(state) { - return state.tabs.find((t) => t.id === state.activeTabId) || state.tabs[0]; + return state.tabs.value.find((t) => t.id === state.activeTabId.value) || state.tabs.value[0]; } /** Allocate a new tab id ('t2', 't3', ...). */ @@ -90,7 +94,7 @@ export function allocTabId(state) { const rnd = () => Math.random().toString(36).slice(2, 6); const makeId = (prefix, now) => prefix + now + rnd(); -const tabsForSaved = (state, id) => state.tabs.filter((t) => t.savedId === id); +const tabsForSaved = (state, id) => state.tabs.value.filter((t) => t.savedId === id); /** The saved query a tab is linked to (via tab.savedId), or null. */ export function savedForTab(state, tab) { @@ -219,7 +223,7 @@ export function deleteSaved(state, id, save = saveJSON) { * kept tab doesn't show "Saved" against a query that's gone. */ function pruneTabLinks(state) { const ids = new Set(state.savedQueries.map((q) => q.id)); - for (const t of state.tabs) if (t.savedId && !ids.has(t.savedId)) t.savedId = null; + for (const t of state.tabs.value) if (t.savedId && !ids.has(t.savedId)) t.savedId = null; } /** Rename the library (blank → the default name). Marks dirty; persists name. */ diff --git a/src/ui/app.js b/src/ui/app.js index 0f5bacf..c2a923f 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -26,6 +26,7 @@ import * as oauth from '../net/oauth.js'; import * as ch from '../net/ch-client.js'; import { mountEditor, insertAtCursor, replaceEditor, SCHEMA_GRAPH_MIME } from './editor.js'; import { renderTabs, selectTab, newTab, closeTab, loadIntoNewTab } from './tabs.js'; +import { effect } from '../core/signal.js'; import { renderSchema } from './schema.js'; import { renderResults } from './results.js'; import { openSchemaView } from './explain-graph.js'; @@ -938,11 +939,20 @@ export function renderApp(app, helpers) { app.root.replaceChildren(header, app.dom.banner, h('div', { class: 'main-row' }, sidebar, sideHandle, workbench)); mountEditor(app, app.dom.editorRegion); - renderTabs(app); - renderResults(app); + // Reactive repaint of the tab-dependent surface — replaces the old tabs.js + // refresh(): re-runs whenever the tab list or active tab changes, so tab ops + // just mutate the signals. (Result-data repaints still call renderResults + // directly from the run flow; those aren't tab changes.) + effect(() => { + app.state.tabs.value; + app.state.activeTabId.value; + renderTabs(app); + if (app.dom.editorSync) app.dom.editorSync(); + renderResults(app); + app.updateSaveBtn(); + }); renderSchema(app); renderSavedHistory(app); - app.updateSaveBtn(); app.loadVersion(); app.loadSchema(); app.loadReference(); diff --git a/src/ui/tabs.js b/src/ui/tabs.js index 9a1fc1a..8361847 100644 --- a/src/ui/tabs.js +++ b/src/ui/tabs.js @@ -5,17 +5,18 @@ import { h } from './dom.js'; import { Icon } from './icons.js'; import { activeTab, allocTabId, newTabObj } from '../state.js'; import { cloneChartCfg } from '../core/chart-data.js'; +import { batch } from '../core/signal.js'; /** Paint the tab strip into app.dom.qtabsInner. */ export function renderTabs(app) { const host = app.dom.qtabsInner; if (!host) return; - host.replaceChildren(...app.state.tabs.map((t) => { - const isActive = t.id === app.state.activeTabId; + host.replaceChildren(...app.state.tabs.value.map((t) => { + const isActive = t.id === app.state.activeTabId.value; return h('div', { class: 'qtab' + (isActive ? ' active' : ''), onclick: () => selectTab(app, t.id) }, h('span', { class: 'name' }, t.name), t.dirty ? h('span', { class: 'dirty' }) : null, - app.state.tabs.length > 1 + app.state.tabs.value.length > 1 ? h('button', { class: 'close', onclick: (e) => { e.stopPropagation(); closeTab(app, t.id); }, @@ -25,26 +26,24 @@ export function renderTabs(app) { })); } -function refresh(app) { - renderTabs(app); - if (app.dom.editorSync) app.dom.editorSync(); - app.actions.rerenderResults(); - app.actions.updateSaveBtn(); -} +// No refresh() any more: an effect wired in createApp() reads `tabs`/`activeTabId` +// and repaints the strip + editor + results + Save button, so these operations +// just mutate the signals. `batch()` coalesces the two-signal updates (list + +// active) into a single repaint. /** Switch the active tab (no-op if already active). */ export function selectTab(app, id) { - if (id === app.state.activeTabId) return; - app.state.activeTabId = id; - refresh(app); + if (id === app.state.activeTabId.value) return; + app.state.activeTabId.value = id; } /** Open a new blank tab and focus the editor. */ export function newTab(app) { const id = allocTabId(app.state); - app.state.tabs.push(newTabObj(id)); - app.state.activeTabId = id; - refresh(app); + batch(() => { + app.state.tabs.value = [...app.state.tabs.value, newTabObj(id)]; + app.state.activeTabId.value = id; + }); if (app.dom.editorTextarea) app.dom.editorTextarea.focus(); } @@ -65,21 +64,23 @@ export function loadIntoNewTab(app, name, sql, savedId = null, chart = null) { tab.chartCfg = cloneChartCfg(chart.cfg); tab.chartKey = chart.key ?? null; } - app.state.tabs.push(tab); - app.state.activeTabId = id; - refresh(app); + batch(() => { + app.state.tabs.value = [...app.state.tabs.value, tab]; + app.state.activeTabId.value = id; + }); if (app.dom.editorTextarea) app.dom.editorTextarea.focus(); } /** Close a tab (never the last one), re-selecting a neighbour if needed. */ export function closeTab(app, id) { - if (app.state.tabs.length <= 1) return; - const idx = app.state.tabs.findIndex((t) => t.id === id); - app.state.tabs.splice(idx, 1); - if (id === app.state.activeTabId) { - app.state.activeTabId = app.state.tabs[Math.max(0, idx - 1)].id; - } - refresh(app); + if (app.state.tabs.value.length <= 1) return; + const idx = app.state.tabs.value.findIndex((t) => t.id === id); + batch(() => { + app.state.tabs.value = app.state.tabs.value.filter((t) => t.id !== id); + if (id === app.state.activeTabId.value) { + app.state.activeTabId.value = app.state.tabs.value[Math.max(0, idx - 1)].id; + } + }); } export { activeTab }; diff --git a/tests/unit/app.test.js b/tests/unit/app.test.js index 4aed1bd..d36fd34 100644 --- a/tests/unit/app.test.js +++ b/tests/unit/app.test.js @@ -874,7 +874,7 @@ describe('exhaustive controller coverage', () => { document.querySelector('.save-popover .sp-input').value = 'Q'; document.querySelector('.save-popover .sp-save').dispatchEvent(new Event('click')); // commit app.dom.shareBtn.dispatchEvent(new Event('click')); // share - expect(app.state.tabs.length).toBeGreaterThan(1); + expect(app.state.tabs.value.length).toBeGreaterThan(1); expect(app.state.savedQueries.length).toBe(1); }); @@ -986,7 +986,7 @@ describe('exhaustive controller coverage', () => { app.renderApp(); app.dom.runBtn.dispatchEvent(new Event('click')); // run wrapper (empty sql → no-op) app.actions.newTab(); - app.state.tabs.push({ id: 'tx', name: 'X', sql: '', dirty: false, result: null, savedId: null }); + app.state.tabs.value.push({ id: 'tx', name: 'X', sql: '', dirty: false, result: null, savedId: null }); app.actions.selectTab('tx'); app.actions.insertAtCursor('zz'); app.actions.replaceEditor('SELECT 9'); @@ -994,8 +994,8 @@ describe('exhaustive controller coverage', () => { app.actions.rerenderTabs(); app.actions.rerenderResults(); app.actions.updateSaveBtn(); - app.actions.closeTab(app.state.activeTabId); - expect(app.state.tabs.length).toBeGreaterThan(0); + app.actions.closeTab(app.state.activeTabId.value); + expect(app.state.tabs.value.length).toBeGreaterThan(0); }); it('share / toggleSaved tolerate empty SQL; share with no navigator at all', () => { diff --git a/tests/unit/main.test.js b/tests/unit/main.test.js index ed66f48..2d94fe5 100644 --- a/tests/unit/main.test.js +++ b/tests/unit/main.test.js @@ -1,5 +1,6 @@ import { describe, it, expect, vi } from 'vitest'; import { bootstrap } from '../../src/main.js'; +import { signal } from '../../src/core/signal.js'; function jwt(payload) { const b = (o) => Buffer.from(JSON.stringify(o)).toString('base64url'); @@ -10,7 +11,7 @@ const valid = jwt({ email: 'me@x.com', exp: Math.floor(Date.now() / 1000) + 3600 function fakeApp(over = {}) { return { token: null, - state: { tabs: [{ id: 't1', sql: '', name: 'Untitled' }] }, + state: { tabs: signal([{ id: 't1', sql: '', name: 'Untitled' }]) }, loadConfig: vi.fn(async () => ({ clientId: 'c', tokenUri: 'https://t', clientSecret: '' })), ensureConfig: vi.fn(async () => ({})), setTokens: vi.fn(function (id) { this.token = id; }), @@ -138,9 +139,9 @@ describe('bootstrap', () => { const hash = '#' + btoa(unescape(encodeURIComponent(sql))); const env = fakeEnv({ location: { href: 'https://ch/sql' + hash, origin: 'https://ch', pathname: '/sql', search: '', hash } }); await bootstrap(app, env); - expect(app.state.tabs[0].sql).toBe('SELECT 1'); - expect(app.state.tabs[0].name).toBe('Shared query'); - expect(app.state.tabs[0].chartCfg).toBeFalsy(); // legacy hash carries no chart + expect(app.state.tabs.value[0].sql).toBe('SELECT 1'); + expect(app.state.tabs.value[0].name).toBe('Shared query'); + expect(app.state.tabs.value[0].chartCfg).toBeFalsy(); // legacy hash carries no chart expect(JSON.parse(env.sessionStorage.getItem('oauth_shared'))).toEqual({ sql: 'SELECT 1', chart: null }); // survives a login redirect }); @@ -150,10 +151,10 @@ describe('bootstrap', () => { const hash = '#' + btoa(unescape(encodeURIComponent(JSON.stringify({ __asb: 1, sql: 'SELECT a, b FROM t', chart })))); const env = fakeEnv({ location: { href: 'https://ch/sql' + hash, origin: 'https://ch', pathname: '/sql', search: '', hash } }); await bootstrap(app, env); - expect(app.state.tabs[0].sql).toBe('SELECT a, b FROM t'); - expect(app.state.tabs[0].chartCfg).toEqual(chart.cfg); - expect(app.state.tabs[0].chartCfg).not.toBe(chart.cfg); // cloned, not aliased - expect(app.state.tabs[0].chartKey).toBe(chart.key); + expect(app.state.tabs.value[0].sql).toBe('SELECT a, b FROM t'); + expect(app.state.tabs.value[0].chartCfg).toEqual(chart.cfg); + expect(app.state.tabs.value[0].chartCfg).not.toBe(chart.cfg); // cloned, not aliased + expect(app.state.tabs.value[0].chartKey).toBe(chart.key); }); it('restores a shared query (SQL + chart) from sessionStorage after the OAuth round-trip', async () => { @@ -163,9 +164,9 @@ describe('bootstrap', () => { const chart = { cfg: { type: 'bar', x: 0, y: [1], series: null }, key: 'k' }; env.sessionStorage.setItem('oauth_shared', JSON.stringify({ sql: 'SELECT 42', chart })); await bootstrap(app, env); - expect(app.state.tabs[0].sql).toBe('SELECT 42'); - expect(app.state.tabs[0].name).toBe('Shared query'); - expect(app.state.tabs[0].chartCfg).toEqual(chart.cfg); + expect(app.state.tabs.value[0].sql).toBe('SELECT 42'); + expect(app.state.tabs.value[0].name).toBe('Shared query'); + expect(app.state.tabs.value[0].chartCfg).toEqual(chart.cfg); expect(app.renderApp).toHaveBeenCalled(); expect(env.sessionStorage.getItem('oauth_shared')).toBeNull(); // consumed on render }); @@ -175,8 +176,8 @@ describe('bootstrap', () => { const env = fakeEnv({ location: { href: 'https://ch/sql', origin: 'https://ch', pathname: '/sql', search: '', hash: '' } }); env.sessionStorage.setItem('oauth_shared', '{not json'); await bootstrap(app, env); - expect(app.state.tabs[0].sql).toBe(''); - expect(app.state.tabs[0].name).toBe('Untitled'); + expect(app.state.tabs.value[0].sql).toBe(''); + expect(app.state.tabs.value[0].name).toBe('Untitled'); }); it('preserves extra query params while stripping oauth ones', async () => { diff --git a/tests/unit/signal.test.js b/tests/unit/signal.test.js new file mode 100644 index 0000000..0e5154d --- /dev/null +++ b/tests/unit/signal.test.js @@ -0,0 +1,178 @@ +import { describe, it, expect, vi } from 'vitest'; +import { signal, effect, batch } from '../../src/core/signal.js'; + +describe('signal / effect — the reactive core', () => { + it('reads the current value outside any effect (no subscription)', () => { + const s = signal(1); + expect(s.value).toBe(1); // exercises the `if (activeEffect)` false branch + }); + + it('runs an effect once immediately', () => { + const s = signal('a'); + const spy = vi.fn(() => s.value); + effect(spy); + expect(spy).toHaveBeenCalledTimes(1); + }); + + it('re-runs the effect when a read signal changes', () => { + const s = signal(1); + const seen = []; + effect(() => seen.push(s.value)); + s.value = 2; + s.value = 3; + expect(seen).toEqual([1, 2, 3]); + }); + + it('does NOT re-run when the value is unchanged (Object.is)', () => { + const s = signal(1); + const spy = vi.fn(() => s.value); + effect(spy); + s.value = 1; // equal → no-op + expect(spy).toHaveBeenCalledTimes(1); + }); + + it('only notifies effects that actually read the signal', () => { + const a = signal(0); + const b = signal(0); + const aSpy = vi.fn(() => a.value); + const bSpy = vi.fn(() => b.value); + effect(aSpy); + effect(bSpy); + a.value = 1; + expect(aSpy).toHaveBeenCalledTimes(2); + expect(bSpy).toHaveBeenCalledTimes(1); // b untouched + }); + + it('tracks conditional dependencies — a no-longer-read signal stops triggering', () => { + const useB = signal(true); + const b = signal('x'); + const spy = vi.fn(() => { + if (useB.value) return b.value; + return 'const'; + }); + effect(spy); + expect(spy).toHaveBeenCalledTimes(1); + + b.value = 'y'; // still read → re-runs + expect(spy).toHaveBeenCalledTimes(2); + + useB.value = false; // effect re-runs and stops reading b + expect(spy).toHaveBeenCalledTimes(3); + + b.value = 'z'; // b no longer a dependency → no re-run + expect(spy).toHaveBeenCalledTimes(3); + }); + + it('supports nested effects (restores the previous active effect)', () => { + const outer = signal(0); + const inner = signal(0); + const innerSpy = vi.fn(() => inner.value); + const outerSpy = vi.fn(() => { + outer.value; + effect(innerSpy); // nested: activeEffect must restore to outer after this + }); + effect(outerSpy); + expect(outerSpy).toHaveBeenCalledTimes(1); + expect(innerSpy).toHaveBeenCalledTimes(1); + + inner.value = 1; // only the inner effect depends on inner + expect(innerSpy).toHaveBeenCalledTimes(2); + expect(outerSpy).toHaveBeenCalledTimes(1); + }); + + it('batch() coalesces multiple signal writes into one re-run', () => { + const a = signal(0); + const b = signal(0); + const spy = vi.fn(() => a.value + b.value); // depends on both + effect(spy); + expect(spy).toHaveBeenCalledTimes(1); + batch(() => { + a.value = 1; // queued, not run + b.value = 2; // queued (same effect → deduped by the Set) + }); + expect(spy).toHaveBeenCalledTimes(2); // one flush, not two + expect(a.value + b.value).toBe(3); + }); + + it('batch() returns the callback result and flushes only at the outermost level', () => { + const s = signal(0); + const spy = vi.fn(() => s.value); + effect(spy); + const out = batch(() => { + s.value = 1; + batch(() => { s.value = 2; }); // nested → does NOT flush here + expect(spy).toHaveBeenCalledTimes(1); // still deferred mid-batch + return 'done'; + }); + expect(out).toBe('done'); + expect(spy).toHaveBeenCalledTimes(2); // single flush after the outer batch + }); + + it('dispose() unsubscribes the effect', () => { + const s = signal(1); + const spy = vi.fn(() => s.value); + const dispose = effect(spy); + s.value = 2; + expect(spy).toHaveBeenCalledTimes(2); + dispose(); + s.value = 3; // no longer subscribed + expect(spy).toHaveBeenCalledTimes(2); + }); +}); + +// A focused demonstration: the same tab lifecycle as src/ui/tabs.js, but the +// "render" runs itself. Note there is no refresh() that has to remember to call +// renderTabs + rerenderResults + updateSaveBtn — each view subscribes to what it +// reads, and mutating state is all the caller does. +describe('demonstration: tabs.js lifecycle without manual refresh()', () => { + function makeTabsModel() { + const tabs = signal([{ id: 't1', name: 'Untitled' }]); + const activeId = signal('t1'); + let nextId = 2; + return { + tabs, + activeId, + selectTab: (id) => { activeId.value = id; }, + newTab: () => { + const id = 't' + nextId++; + tabs.value = [...tabs.value, { id, name: 'Untitled' }]; + activeId.value = id; // both reads update; no manual repaint call + }, + closeTab: (id) => { + tabs.value = tabs.value.filter((t) => t.id !== id); + if (activeId.value === id) activeId.value = tabs.value[tabs.value.length - 1].id; + }, + }; + } + + it('repaints the strip + the active-tab-dependent views automatically', () => { + const m = makeTabsModel(); + + // Three independent "views", each declaring what it reads — exactly the + // three things tabs.js's refresh() repaints by hand today. + const strip = vi.fn(() => m.tabs.value.map((t) => t.id)); + const saveBtn = vi.fn(() => m.activeId.value); // "Save" cares about active tab + const results = vi.fn(() => m.activeId.value); + + effect(strip); + effect(saveBtn); + effect(results); + expect([strip, saveBtn, results].map((f) => f.mock.calls.length)).toEqual([1, 1, 1]); + + m.newTab(); // strip changes (new tab) AND active changes + expect(strip).toHaveBeenCalledTimes(2); + expect(saveBtn).toHaveBeenCalledTimes(2); + expect(results).toHaveBeenCalledTimes(2); + + m.selectTab('t1'); // only active changes → strip is NOT repainted + expect(strip).toHaveBeenCalledTimes(2); // unchanged — surgical + expect(saveBtn).toHaveBeenCalledTimes(3); + expect(results).toHaveBeenCalledTimes(3); + + m.closeTab('t2'); // t2 doesn't exist anymore? it's 't2' from newTab → close it + // (newTab created 't2' and selected it; selectTab('t1') made t1 active) + expect(m.tabs.value.map((t) => t.id)).toEqual(['t1']); + expect(strip).toHaveBeenCalledTimes(3); // strip repainted (list shrank) + expect(saveBtn).toHaveBeenCalledTimes(3); // active was already t1 → no change + }); +}); diff --git a/tests/unit/state.test.js b/tests/unit/state.test.js index 331afe2..bc8edef 100644 --- a/tests/unit/state.test.js +++ b/tests/unit/state.test.js @@ -31,7 +31,7 @@ describe('createState', () => { expect(s.sidebarPx).toBe(248); expect(s.editorPct).toBe(45); expect(s.sideSplitPct).toBe(58); - expect(s.tabs).toHaveLength(1); + expect(s.tabs.value).toHaveLength(1); expect(s.savedQueries).toEqual([]); expect(s.expandedTables).toBeInstanceOf(Set); expect(s.libraryName).toBe(DEFAULT_LIBRARY_NAME); @@ -60,7 +60,7 @@ describe('createState', () => { it('defaults the reader to storage helpers', () => { vi.stubGlobal('localStorage', memStore({ [KEYS.theme]: 'light' })); const s = createState(); - expect(s.tabs[0].id).toBe('t1'); + expect(s.tabs.value[0].id).toBe('t1'); expect(s.theme).toBe('light'); }); }); @@ -69,7 +69,7 @@ describe('activeTab / allocTabId', () => { it('returns the active tab, falling back to the first', () => { const s = createState(reader()); expect(activeTab(s).id).toBe('t1'); - s.activeTabId = 'gone'; + s.activeTabId.value = 'gone'; expect(activeTab(s).id).toBe('t1'); }); it('allocates incrementing ids', () => { @@ -83,16 +83,16 @@ describe('saved queries', () => { it('saveQuery is a no-op for empty SQL or empty name', () => { const s = createState(reader()); const save = vi.fn(); - s.tabs[0].sql = ''; - expect(saveQuery(s, s.tabs[0], 'name', '', save)).toBeNull(); - s.tabs[0].sql = 'SELECT 1'; - expect(saveQuery(s, s.tabs[0], ' ', '', save)).toBeNull(); + s.tabs.value[0].sql = ''; + expect(saveQuery(s, s.tabs.value[0], 'name', '', save)).toBeNull(); + s.tabs.value[0].sql = 'SELECT 1'; + expect(saveQuery(s, s.tabs.value[0], ' ', '', save)).toBeNull(); expect(save).not.toHaveBeenCalled(); }); it('saveQuery creates + links the tab, then updates in place on re-save', () => { const s = createState(reader()); const save = vi.fn(); - const tab = s.tabs[0]; + const tab = s.tabs.value[0]; tab.sql = 'SELECT 1'; const e1 = saveQuery(s, tab, 'My query', '', save, 100); expect(e1).toMatchObject({ name: 'My query', sql: 'SELECT 1', favorite: false }); @@ -111,7 +111,7 @@ describe('saved queries', () => { it('saveQuery stores/updates/clears an optional description', () => { const s = createState(reader()); const save = vi.fn(); - const tab = s.tabs[0]; + const tab = s.tabs.value[0]; tab.sql = 'SELECT 1'; const e = saveQuery(s, tab, 'Q', ' what it does ', save, 100); // trimmed expect(e.description).toBe('what it does'); @@ -120,27 +120,27 @@ describe('saved queries', () => { saveQuery(s, tab, 'Q', ' ', save, 300); // blank → dropped expect('description' in s.savedQueries[0]).toBe(false); // create with no description arg → no description field - const t2 = newTabObj('t2'); t2.sql = 'SELECT 2'; s.tabs.push(t2); + const t2 = newTabObj('t2'); t2.sql = 'SELECT 2'; s.tabs.value.push(t2); const e2 = saveQuery(s, t2, 'Q2', undefined, save, 400); expect('description' in e2).toBe(false); }); it('savedForTab resolves the linked entry (or null)', () => { const s = createState(reader()); s.savedQueries = [{ id: 's1', sql: 'x', name: 'n', favorite: false }]; - s.tabs[0].savedId = 's1'; - expect(savedForTab(s, s.tabs[0])).toMatchObject({ id: 's1' }); - s.tabs[0].savedId = 'gone'; - expect(savedForTab(s, s.tabs[0])).toBeNull(); + s.tabs.value[0].savedId = 's1'; + expect(savedForTab(s, s.tabs.value[0])).toMatchObject({ id: 's1' }); + s.tabs.value[0].savedId = 'gone'; + expect(savedForTab(s, s.tabs.value[0])).toBeNull(); expect(savedForTab(s, { savedId: null })).toBeNull(); }); it('renameSaved updates the entry + any linked tab name', () => { const s = createState(reader()); s.savedQueries = [{ id: 's1', sql: 'x', name: 'old', favorite: false }]; - s.tabs[0].savedId = 's1'; + s.tabs.value[0].savedId = 's1'; const save = vi.fn(); renameSaved(s, 's1', ' new ', undefined, save); expect(s.savedQueries[0].name).toBe('new'); - expect(s.tabs[0].name).toBe('new'); + expect(s.tabs.value[0].name).toBe('new'); renameSaved(s, 's1', ' ', undefined, save); // blank ignored expect(s.savedQueries[0].name).toBe('new'); renameSaved(s, 'missing', 'x', undefined, save); // unknown id ignored @@ -225,7 +225,7 @@ describe('saved queries', () => { it('saveQuery persists, updates, and clears the chart config alongside the SQL', () => { const s = createState(reader()); const save = vi.fn(); - const tab = s.tabs[0]; + const tab = s.tabs.value[0]; tab.sql = 'SELECT a, b'; tab.chartCfg = { type: 'pie', x: 0, y: [1], series: null }; tab.chartKey = 'a:String|b:UInt64'; @@ -244,7 +244,7 @@ describe('saved queries', () => { it('saveQuery persists the result view (Table/JSON/Chart), updates it, and ignores the transient raw view', () => { const s = createState(reader()); const save = vi.fn(); - const tab = s.tabs[0]; + const tab = s.tabs.value[0]; tab.sql = 'SELECT 1'; s.resultView = 'chart'; const e = saveQuery(s, tab, 'V', '', save, 100); @@ -261,11 +261,11 @@ describe('saved queries', () => { it('deleteSaved removes + clears tab pointers', () => { const s = createState(reader()); s.savedQueries = [{ id: 's1', sql: 'x', name: 'n' }]; - s.tabs[0].savedId = 's1'; + s.tabs.value[0].savedId = 's1'; const save = vi.fn(); deleteSaved(s, 's1', save); expect(s.savedQueries).toHaveLength(0); - expect(s.tabs[0].savedId).toBeNull(); + expect(s.tabs.value[0].savedId).toBeNull(); expect(save).toHaveBeenCalledWith(KEYS.saved, []); }); }); @@ -273,7 +273,7 @@ describe('saved queries', () => { describe('library document', () => { it('dirty flag: saved-query mutations set it; markLibrarySaved clears it', () => { const s = createState(reader()); - const tab = s.tabs[0]; tab.sql = 'SELECT 1'; + const tab = s.tabs.value[0]; tab.sql = 'SELECT 1'; expect(s.libraryDirty).toBe(false); saveQuery(s, tab, 'Q', '', vi.fn()); expect(s.libraryDirty).toBe(true); @@ -307,14 +307,14 @@ describe('library document', () => { const s = createState(reader()); s.savedQueries = [{ id: 's1', name: 'A', sql: '1', favorite: false }]; s.libraryName = 'Old'; s.libraryDirty = true; - s.tabs[0].savedId = 's1'; // dangling after clear → pruned - s.tabs.push(newTabObj('t2')); // no savedId → skipped by prune + s.tabs.value[0].savedId = 's1'; // dangling after clear → pruned + s.tabs.value.push(newTabObj('t2')); // no savedId → skipped by prune const save = vi.fn(), saveName = vi.fn(); newLibrary(s, save, saveName); expect(s.savedQueries).toEqual([]); expect(s.libraryName).toBe(DEFAULT_LIBRARY_NAME); expect(s.libraryDirty).toBe(false); - expect(s.tabs[0].savedId).toBeNull(); + expect(s.tabs.value[0].savedId).toBeNull(); expect(save).toHaveBeenCalledWith(KEYS.saved, []); expect(saveName).toHaveBeenCalledWith(KEYS.libraryName, DEFAULT_LIBRARY_NAME); }); @@ -322,7 +322,7 @@ describe('library document', () => { it('replaceLibrary keeps ids (mints for id-less), carries metadata, adopts the base name, clears dirty, prunes links', () => { const s = createState(reader()); s.savedQueries = [{ id: 'old', name: 'X', sql: 'x', favorite: false }]; - s.tabs[0].savedId = 'old'; // becomes dangling → pruned + s.tabs.value[0].savedId = 'old'; // becomes dangling → pruned s.libraryDirty = true; const chart = { cfg: { type: 'bar' }, key: 'k' }; const incoming = [ @@ -336,7 +336,7 @@ describe('library document', () => { expect(s.savedQueries[0]).toMatchObject({ name: 'A', sql: '1', favorite: true, description: 'd', chart, view: 'json' }); expect(s.libraryName).toBe('My Library'); // extension stripped expect(s.libraryDirty).toBe(false); - expect(s.tabs[0].savedId).toBeNull(); + expect(s.tabs.value[0].savedId).toBeNull(); expect(save).toHaveBeenCalledWith(KEYS.saved, s.savedQueries); expect(saveName).toHaveBeenCalledWith(KEYS.libraryName, 'My Library'); }); @@ -381,8 +381,8 @@ describe('library document', () => { it('library ops default their persistence seams (real storage helpers)', () => { vi.stubGlobal('localStorage', memStore()); const s = createState(reader()); - s.tabs[0].sql = 'SELECT 1'; - const e = saveQuery(s, s.tabs[0], 'Q'); // default save/now/description + s.tabs.value[0].sql = 'SELECT 1'; + const e = saveQuery(s, s.tabs.value[0], 'Q'); // default save/now/description renameLibrary(s, 'Lib'); // default saveName replaceLibrary(s, [{ id: e.id, name: 'Q', sql: 'SELECT 1' }], 'f.json'); // default seams newLibrary(s); // default seams @@ -447,8 +447,8 @@ describe('default persistence', () => { it('saveQuery/renameSaved/toggleFavorite/deleteSaved/recordHistory/clearHistory persist via storage by default', () => { const s = createState(reader()); // Exercises the default saveJSON path (writes to happy-dom localStorage). - s.tabs[0].sql = 'SELECT 9'; - const e = saveQuery(s, s.tabs[0], 'nine'); + s.tabs.value[0].sql = 'SELECT 9'; + const e = saveQuery(s, s.tabs.value[0], 'nine'); renameSaved(s, e.id, 'nine!'); toggleFavorite(s, e.id); recordHistory(s, { sql: 'SELECT 9', result: { rawText: null, rows: [], progress: { elapsed_ns: 0 } } }); diff --git a/tests/unit/tabs.test.js b/tests/unit/tabs.test.js index afd8657..7673c12 100644 --- a/tests/unit/tabs.test.js +++ b/tests/unit/tabs.test.js @@ -10,11 +10,11 @@ describe('renderTabs', () => { }); it('marks the active tab, shows dirty dot, and a close button only with >1 tab', () => { const app = makeApp(); - app.state.tabs = [ + app.state.tabs.value = [ { id: 't1', name: 'A', dirty: true }, { id: 't2', name: 'B', dirty: false }, ]; - app.state.activeTabId = 't1'; + app.state.activeTabId.value = 't1'; renderTabs(app); const tabs = app.dom.qtabsInner.querySelectorAll('.qtab'); expect(tabs).toHaveLength(2); @@ -29,38 +29,31 @@ describe('renderTabs', () => { }); it('clicking a tab selects it; clicking close closes it', () => { const app = makeApp(); - app.state.tabs = [{ id: 't1', name: 'A' }, { id: 't2', name: 'B' }]; + app.state.tabs.value = [{ id: 't1', name: 'A' }, { id: 't2', name: 'B' }]; renderTabs(app); const second = app.dom.qtabsInner.querySelectorAll('.qtab')[1]; second.dispatchEvent(new Event('click')); - expect(app.state.activeTabId).toBe('t2'); + expect(app.state.activeTabId.value).toBe('t2'); const close = app.dom.qtabsInner.querySelectorAll('.qtab')[0].querySelector('.close'); close.dispatchEvent(new Event('click', { bubbles: true })); - expect(app.state.tabs.map((t) => t.id)).toEqual(['t2']); + expect(app.state.tabs.value.map((t) => t.id)).toEqual(['t2']); }); }); +// tabs.js is now pure state-mutation over the tab signals; the repaint on a tab +// change (renderTabs + editorSync + results + Save button) is the createApp() +// effect's job and is covered in app.test.js — not here. describe('selectTab', () => { - it('switches active tab and refreshes', () => { + it('switches the active tab', () => { const app = makeApp(); - app.state.tabs.push({ id: 't2', name: 'B' }); + app.state.tabs.value = [...app.state.tabs.value, { id: 't2', name: 'B' }]; selectTab(app, 't2'); - expect(app.state.activeTabId).toBe('t2'); - expect(app.actions.rerenderResults).toHaveBeenCalled(); - expect(app.actions.updateSaveBtn).toHaveBeenCalled(); + expect(app.state.activeTabId.value).toBe('t2'); }); - it('no-ops if already active', () => { + it('no-ops if already active (early-return guard)', () => { const app = makeApp(); selectTab(app, 't1'); - expect(app.actions.rerenderResults).not.toHaveBeenCalled(); - }); - it('calls editorSync + focuses the textarea when present', () => { - const app = makeApp(); - app.dom.editorSync = vi.fn(); - app.dom.editorTextarea = { focus: vi.fn() }; - app.state.tabs.push({ id: 't2', name: 'B' }); - selectTab(app, 't2'); - expect(app.dom.editorSync).toHaveBeenCalled(); + expect(app.state.activeTabId.value).toBe('t1'); }); }); @@ -69,7 +62,7 @@ describe('newTab / loadIntoNewTab', () => { const app = makeApp(); app.dom.editorTextarea = { focus: vi.fn() }; newTab(app); - expect(app.state.tabs).toHaveLength(2); + expect(app.state.tabs.value).toHaveLength(2); expect(app.activeTab().name).toBe('Untitled'); expect(app.dom.editorTextarea.focus).toHaveBeenCalled(); }); @@ -102,21 +95,21 @@ describe('closeTab', () => { it('refuses to close the last tab', () => { const app = makeApp(); closeTab(app, 't1'); - expect(app.state.tabs).toHaveLength(1); + expect(app.state.tabs.value).toHaveLength(1); }); it('closes a non-active tab', () => { const app = makeApp(); - app.state.tabs = [{ id: 't1', name: 'A' }, { id: 't2', name: 'B' }]; - app.state.activeTabId = 't1'; + app.state.tabs.value = [{ id: 't1', name: 'A' }, { id: 't2', name: 'B' }]; + app.state.activeTabId.value = 't1'; closeTab(app, 't2'); - expect(app.state.tabs.map((t) => t.id)).toEqual(['t1']); - expect(app.state.activeTabId).toBe('t1'); + expect(app.state.tabs.value.map((t) => t.id)).toEqual(['t1']); + expect(app.state.activeTabId.value).toBe('t1'); }); it('closing the active tab re-selects the previous neighbour', () => { const app = makeApp(); - app.state.tabs = [{ id: 't1', name: 'A' }, { id: 't2', name: 'B' }, { id: 't3', name: 'C' }]; - app.state.activeTabId = 't2'; + app.state.tabs.value = [{ id: 't1', name: 'A' }, { id: 't2', name: 'B' }, { id: 't3', name: 'C' }]; + app.state.activeTabId.value = 't2'; closeTab(app, 't2'); - expect(app.state.activeTabId).toBe('t1'); + expect(app.state.activeTabId.value).toBe('t1'); }); }); From eacda90ea1774e023f2c177b5d820530e55548fa Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Mon, 29 Jun 2026 23:09:47 +0200 Subject: [PATCH 02/14] spike: convert the sidePanel slice (worst-case: no accessor helper) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01KQ6VFCwwUWnU3YgHQK2K5k --- src/state.js | 2 +- src/ui/app.js | 10 +++++++-- src/ui/saved-history.js | 18 ++++++++------- tests/unit/app.test.js | 2 +- tests/unit/saved-history.test.js | 38 ++++++++++++++++---------------- tests/unit/state.test.js | 2 +- 6 files changed, 40 insertions(+), 32 deletions(-) diff --git a/src/state.js b/src/state.js index 3a337d4..b02570f 100644 --- a/src/state.js +++ b/src/state.js @@ -67,7 +67,7 @@ export function createState(read = { loadJSON, loadStr }) { // derived per-run from the typed statement / clicked tab, not stored here. forceExplain: false, resultSort: { col: null, dir: 'asc' }, - sidePanel: read.loadStr(KEYS.sidePanel, 'saved'), + sidePanel: signal(read.loadStr(KEYS.sidePanel, 'saved')), savedQueries: read.loadJSON(KEYS.saved, []), history: read.loadJSON(KEYS.history, []), // The saved-query collection treated as a named document ("the Library"). diff --git a/src/ui/app.js b/src/ui/app.js index c2a923f..81182b1 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -640,7 +640,7 @@ export function createApp(env = {}) { // --- saved / history bridges ------------------------------------------ app.recordHistory = (tab) => { recordHistory(app.state, tab, saveJSON); - if (app.state.sidePanel === 'history') renderSavedHistory(app); + if (app.state.sidePanel.value === 'history') renderSavedHistory(app); }; // --- share + star ------------------------------------------------------ @@ -952,7 +952,13 @@ export function renderApp(app, helpers) { app.updateSaveBtn(); }); renderSchema(app); - renderSavedHistory(app); + // Reactive repaint of the side panel: re-runs when the active panel changes + // (Library ↔ History). Data-driven repaints (savedQueries/history mutations) + // still call renderSavedHistory directly until those slices are signals too. + effect(() => { + app.state.sidePanel.value; + renderSavedHistory(app); + }); app.loadVersion(); app.loadSchema(); app.loadReference(); diff --git a/src/ui/saved-history.js b/src/ui/saved-history.js index 76e6076..32d6d39 100644 --- a/src/ui/saved-history.js +++ b/src/ui/saved-history.js @@ -25,22 +25,24 @@ export function renderSavedHistory(app) { const state = app.state; const count = state.savedQueries.length; - // Switching panes clears the search so each tab starts unfiltered. + // Switching panes clears the search so each tab starts unfiltered. Clear the + // (plain) filter first, then set the sidePanel signal — its render effect runs + // synchronously on assignment and must see the cleared filter. No manual + // re-render call: the effect in createApp() repaints. const switchTo = (panel) => { - state.sidePanel = panel; state.libraryFilter = ''; app.savePref('sidePanel', panel); - renderSavedHistory(app); + state.sidePanel.value = panel; }; tabsRow.replaceChildren( h('button', { - class: 'side-tab' + (state.sidePanel === 'saved' ? ' active' : ''), + class: 'side-tab' + (state.sidePanel.value === 'saved' ? ' active' : ''), onclick: () => switchTo('saved'), }, Icon.layers(), h('span', null, 'Library'), count ? h('span', { class: 'side-count' }, '· ' + count) : null), h('button', { - class: 'side-tab' + (state.sidePanel === 'history' ? ' active' : ''), + class: 'side-tab' + (state.sidePanel.value === 'history' ? ' active' : ''), onclick: () => switchTo('history'), }, Icon.history(), h('span', null, 'History')), ); @@ -54,7 +56,7 @@ export function renderSavedHistory(app) { function renderList(app) { const list = app.dom.savedList; list.replaceChildren(); - if (app.state.sidePanel === 'saved') renderSaved(app, list); + if (app.state.sidePanel.value === 'saved') renderSaved(app, list); else renderHistory(app, list); } @@ -67,13 +69,13 @@ function renderSearch(app) { const box = app.dom.savedSearch; if (!box) return; const state = app.state; - const hasItems = state.sidePanel === 'saved' ? state.savedQueries.length > 0 : state.history.length > 0; + const hasItems = state.sidePanel.value === 'saved' ? state.savedQueries.length > 0 : state.history.length > 0; box.replaceChildren(); if (!hasItems) return; const input = h('input', { class: 'sv-search-input', type: 'text', - placeholder: state.sidePanel === 'saved' ? 'Search saved queries…' : 'Search history…', + placeholder: state.sidePanel.value === 'saved' ? 'Search saved queries…' : 'Search history…', value: state.libraryFilter, }); const clear = h('button', { class: 'sv-search-clear', title: 'Clear' }, Icon.close()); diff --git a/tests/unit/app.test.js b/tests/unit/app.test.js index d36fd34..5b60200 100644 --- a/tests/unit/app.test.js +++ b/tests/unit/app.test.js @@ -1197,7 +1197,7 @@ describe('exhaustive controller coverage', () => { }); const app = createApp(e); app.renderApp(); - app.state.sidePanel = 'history'; + app.state.sidePanel.value = 'history'; app.activeTab().sql = 'SELECT 1'; await app.actions.run(); expect(app.state.history.length).toBe(1); diff --git a/tests/unit/saved-history.test.js b/tests/unit/saved-history.test.js index c61660e..e289ad6 100644 --- a/tests/unit/saved-history.test.js +++ b/tests/unit/saved-history.test.js @@ -20,7 +20,7 @@ describe('renderSavedHistory', () => { it('saved: empty state', () => { const app = makeApp(); - app.state.sidePanel = 'saved'; + app.state.sidePanel.value = 'saved'; renderSavedHistory(app); expect(app.dom.savedList.textContent).toContain('No saved queries yet.'); }); @@ -29,7 +29,7 @@ describe('renderSavedHistory', () => { it('saved: lists rows, loads on click, deletes via trash + refreshes Save button', () => { const app = makeApp(); - app.state.sidePanel = 'saved'; + app.state.sidePanel.value = 'saved'; const chart = { cfg: { type: 'pie', x: 0, y: [1], series: null }, key: 'k' }; app.state.savedQueries = [{ id: 's1', name: 'Q1', sql: 'SELECT 1\n-- more', favorite: false, chart, view: 'chart' }]; renderSavedHistory(app); @@ -46,7 +46,7 @@ describe('renderSavedHistory', () => { it('saved: live count + star toggles favorite and re-sorts favorites first', () => { const app = makeApp(); - app.state.sidePanel = 'saved'; + app.state.sidePanel.value = 'saved'; app.state.savedQueries = [ { id: 'a', name: 'A', sql: '1', favorite: false }, { id: 'b', name: 'B', sql: '2', favorite: false }, @@ -63,7 +63,7 @@ describe('renderSavedHistory', () => { it('saved: pencil opens the edit form; Name(Enter)+Description commit via renameSaved; double-fire is guarded', () => { const app = makeApp(); - app.state.sidePanel = 'saved'; + app.state.sidePanel.value = 'saved'; app.state.savedQueries = [{ id: 's1', name: 'Old', sql: '1', favorite: false }]; renderSavedHistory(app); byTitle(app.dom.savedList, 'Edit name & description').dispatchEvent(new Event('click', { bubbles: true })); @@ -92,7 +92,7 @@ describe('renderSavedHistory', () => { }); it('saved: edit form — description prefilled; ⌘/Ctrl+Enter + Save commit, Escape/Cancel + empty name revert', () => { const app = makeApp(); - app.state.sidePanel = 'saved'; + app.state.sidePanel.value = 'saved'; app.state.savedQueries = [{ id: 's1', name: 'Old', sql: '1', favorite: false, description: 'd0' }]; renderSavedHistory(app); const open = () => byTitle(app.dom.savedList, 'Edit name & description').dispatchEvent(new Event('click', { bubbles: true })); @@ -130,7 +130,7 @@ describe('renderSavedHistory', () => { }); it('saved: renders a 2-line description preview when present, omits it otherwise', () => { const app = makeApp(); - app.state.sidePanel = 'saved'; + app.state.sidePanel.value = 'saved'; app.state.savedQueries = [ { id: 's1', name: 'A', sql: '1', favorite: false, description: 'explains A' }, { id: 's2', name: 'B', sql: '2', favorite: false }, @@ -143,7 +143,7 @@ describe('renderSavedHistory', () => { it('saved: the tab is labelled "Library" with a live count and no Export/Import row', () => { const app = makeApp(); - app.state.sidePanel = 'saved'; + app.state.sidePanel.value = 'saved'; app.state.savedQueries = [{ id: 's1', name: 'A', sql: '1', favorite: false }]; renderSavedHistory(app); const savedTab = app.dom.savedTabsRow.querySelectorAll('.side-tab')[0]; @@ -156,14 +156,14 @@ describe('renderSavedHistory', () => { }); it('history: empty state', () => { const app = makeApp(); - app.state.sidePanel = 'history'; + app.state.sidePanel.value = 'history'; renderSavedHistory(app); expect(app.dom.savedList.textContent).toContain('No history yet.'); }); it('history: lists rows (with + without row count) and loads on click', () => { const app = makeApp(); - app.state.sidePanel = 'history'; + app.state.sidePanel.value = 'history'; app.state.history = [ { id: 'h1', sql: 'SELECT 1', ts: Date.now(), rows: 3, ms: 4 }, { id: 'h2', sql: 'INSERT …', ts: Date.now(), rows: null, ms: 1 }, @@ -180,7 +180,7 @@ describe('renderSavedHistory', () => { it('history: per-row delete removes just that entry without loading it', () => { const app = makeApp(); - app.state.sidePanel = 'history'; + app.state.sidePanel.value = 'history'; app.state.history = [ { id: 'h1', sql: 'SELECT 1', ts: Date.now(), rows: 3, ms: 4 }, { id: 'h2', sql: 'SELECT 2', ts: Date.now(), rows: 1, ms: 2 }, @@ -194,14 +194,14 @@ describe('renderSavedHistory', () => { it('switching panels persists the choice', () => { const app = makeApp(); - app.state.sidePanel = 'saved'; + app.state.sidePanel.value = 'saved'; renderSavedHistory(app); const [savedBtn, histBtn] = app.dom.savedTabsRow.querySelectorAll('.side-tab'); click(histBtn); - expect(app.state.sidePanel).toBe('history'); + expect(app.state.sidePanel.value).toBe('history'); expect(app.savePref).toHaveBeenCalledWith('sidePanel', 'history'); click(savedBtn); - expect(app.state.sidePanel).toBe('saved'); + expect(app.state.sidePanel.value).toBe('saved'); expect(app.savePref).toHaveBeenCalledWith('sidePanel', 'saved'); }); }); @@ -209,7 +209,7 @@ describe('renderSavedHistory', () => { describe('renderSavedHistory — search/filter', () => { const savedApp = () => { const app = makeApp(); - app.state.sidePanel = 'saved'; + app.state.sidePanel.value = 'saved'; app.state.savedQueries = [ { id: 's1', name: 'Carrier delays', sql: 'SELECT carrier FROM flights', favorite: false, description: 'worst delays' }, { id: 's2', name: 'Busiest airports', sql: 'SELECT origin, count() FROM flights', favorite: false }, @@ -230,7 +230,7 @@ describe('renderSavedHistory — search/filter', () => { it('collapses the search box when the active list is empty', () => { const app = makeApp(); - app.state.sidePanel = 'saved'; + app.state.sidePanel.value = 'saved'; renderSavedHistory(app); expect(app.dom.savedSearch.children.length).toBe(0); // :empty → hidden via CSS expect(input(app)).toBeNull(); @@ -239,7 +239,7 @@ describe('renderSavedHistory — search/filter', () => { it('shows the box with a per-tab placeholder when items exist', () => { const app = savedApp(); expect(input(app).placeholder).toBe('Search saved queries…'); - app.state.sidePanel = 'history'; + app.state.sidePanel.value = 'history'; app.state.history = [{ id: 'h1', sql: 'SELECT 1', ts: Date.now(), rows: 1, ms: 1 }]; renderSavedHistory(app); expect(input(app).placeholder).toBe('Search history…'); @@ -274,7 +274,7 @@ describe('renderSavedHistory — search/filter', () => { it('filters history by sql with its own no-match message', () => { const app = makeApp(); - app.state.sidePanel = 'history'; + app.state.sidePanel.value = 'history'; app.state.history = [ { id: 'h1', sql: 'SELECT 1', ts: Date.now(), rows: 1, ms: 1 }, { id: 'h2', sql: 'INSERT INTO t', ts: Date.now(), rows: null, ms: 1 }, @@ -300,7 +300,7 @@ describe('renderSavedHistory — search/filter', () => { describe('drag a row into the editor', () => { it('a saved row is draggable and carries its SQL as a subquery payload', () => { const app = makeApp(); - app.state.sidePanel = 'saved'; + app.state.sidePanel.value = 'saved'; app.state.savedQueries = [{ id: 's1', name: 'Q1', sql: 'SELECT 1\n-- more', favorite: false }]; renderSavedHistory(app); const row = app.dom.savedList.querySelector('.saved-row'); @@ -310,7 +310,7 @@ describe('drag a row into the editor', () => { }); it('a history row is draggable and carries its SQL as a subquery payload', () => { const app = makeApp(); - app.state.sidePanel = 'history'; + app.state.sidePanel.value = 'history'; app.state.history = [{ id: 'h1', sql: 'SELECT 2', ts: Date.now(), rows: 1, ms: 1 }]; renderSavedHistory(app); const row = app.dom.savedList.querySelector('.history-row'); diff --git a/tests/unit/state.test.js b/tests/unit/state.test.js index bc8edef..dd8c393 100644 --- a/tests/unit/state.test.js +++ b/tests/unit/state.test.js @@ -53,7 +53,7 @@ describe('createState', () => { expect(s.sidebarPx).toBe(420); expect(s.editorPct).toBe(15); expect(s.sideSplitPct).toBe(85); - expect(s.sidePanel).toBe('history'); + expect(s.sidePanel.value).toBe('history'); expect(s.savedQueries).toHaveLength(1); expect(s.history).toHaveLength(1); }); From 7631005ed2958b732064252aae0b5405a42989e6 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Mon, 29 Jun 2026 23:18:10 +0200 Subject: [PATCH 03/14] =?UTF-8?q?docs:=20ADR-0001=20=E2=80=94=20incrementa?= =?UTF-8?q?l=20signals=20over=20a=20UI=20framework?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01KQ6VFCwwUWnU3YgHQK2K5k --- docs/ADR-0001-reactivity.md | 68 +++++++++++++++++++++++++++++++++++++ 1 file changed, 68 insertions(+) create mode 100644 docs/ADR-0001-reactivity.md diff --git a/docs/ADR-0001-reactivity.md b/docs/ADR-0001-reactivity.md new file mode 100644 index 0000000..a46fbec --- /dev/null +++ b/docs/ADR-0001-reactivity.md @@ -0,0 +1,68 @@ +# ADR-0001: Reactivity — incremental signals, not a framework + +- **Status:** Proposed +- **Date:** 2026-06-29 +- **Context tracking:** roadmap #68 +- **Evidence:** branch `spike/signals` (primitive + two converted slices) + +## Context + +The app is a framework-free ES-module SPA: pure logic in `src/core/` (100% +covered), an injected-seam controller (`createApp(env)`), and a hand-rolled +hyperscript render layer in `src/ui/`. As feature count grows (16+ open issues), +the recurring pain is **manual render invalidation**: state is mutated and then +the dependent views are repainted by hand (e.g. `tabs.js`'s old `refresh()` +called `renderTabs` + editorSync + `renderResults` + `updateSaveBtn`). Forgetting +a dependent → stale UI. This is an *absence-of-reactivity* problem, distinct from +a *component-model* problem. + +Three hard constraints frame any solution: a single self-contained HTML artifact +with zero third-party requests (CLAUDE.md rule 4 — deliberate deps only), the +per-file 100/100/100/100 coverage gate (rule 1), and the injected-seam layering +(rule 2). + +## Decision + +Adopt a **~70-line in-house reactive primitive** (`src/core/signal.js`: +`signal()` / `effect()` / `batch()`) and migrate state **one slice at a time**, +behind accessor helpers. **Do not** adopt a UI framework (React/Preact/Solid). + +## Options considered + +| Option | gzip cost | Verdict | +|---|---|---| +| **In-house `signal`/`effect`** | ~0.45 KB (measured) | **Chosen** — retires manual invalidation; zero deps; proven 100%-coverable | +| Preact + @preact/signals | ~7.3 KB (measured, `spike/preact`) | Rejected for now — only added value over the primitive is auto DOM-diffing of large lists, which we don't need | +| Solid.js | ~7 KB | Rejected — compiler/plugin + ecosystem cost; same "big-list" benefit we don't need | +| React (proper) | ~45 KB | Rejected — heaviest dep, vDOM-on-big-tables liability, mismatch with the single-file ethos | + +Decisive factor: we do **not** need a virtualized / large-list renderer (no +10k-row grid requirement), which is the one thing a vDOM framework buys over the +primitive. The pain is invalidation, which the primitive solves directly. + +## Evidence (spike) + +Two slices converted end-to-end with the full suite + coverage gate green +(1033 tests; `signal.js` 100/100/100/100; real `npm run build` succeeds): + +- **tabs** (`tabs` + `activeTabId`) — *has* an `activeTab()` accessor → 16 callers + insulated; low mechanical churn, but one behavioral test relocation (the + repaint responsibility moved from `tabs.js` to a `createApp()` effect). +- **sidePanel** — *no* accessor → every reader changed, but the churn was purely + mechanical `.value` and concentrated in the panel's own test file; no assertion + rewrites. + +Cumulative spike cost: 12 files, +432/−142, most of which is the reusable +primitive (95) + its tests (178). + +## Consequences + +- Mutating a converted slice is just `state.x.value = …`; an `effect()` repaints. + Manual `refresh()` lists are deleted. +- **Migration is monotonic**: a slice keeps some direct render calls until its + co-dependent slices are also signals. No slice un-converts another. +- Per-slice cost is mostly mechanical `.value` edits in that slice's tests. +- **Rule:** give each converted slice an accessor helper (like `activeTab()`) to + contain reader churn and localize behavior. +- Re-evaluate Preact/Solid only if the UI later grows many interdependent + components with rich local state, or a genuine large-list render need appears. From d9872ed27bbfe0e198f0f52e0b8c134f67e8b48a Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Mon, 29 Jun 2026 23:38:20 +0200 Subject: [PATCH 04/14] spike: use @preact/signals-core instead of the hand-rolled primitive 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) Claude-Session: https://claude.ai/code/session_01KQ6VFCwwUWnU3YgHQK2K5k --- package.json | 1 + src/core/signal.js | 95 -------------------- src/state.js | 2 +- src/ui/app.js | 2 +- src/ui/tabs.js | 2 +- tests/unit/main.test.js | 2 +- tests/unit/signal.test.js | 178 -------------------------------------- 7 files changed, 5 insertions(+), 277 deletions(-) delete mode 100644 src/core/signal.js delete mode 100644 tests/unit/signal.test.js diff --git a/package.json b/package.json index da54d92..37e3f20 100644 --- a/package.json +++ b/package.json @@ -22,6 +22,7 @@ }, "dependencies": { "@dagrejs/dagre": "^3.0.0", + "@preact/signals-core": "^1.14.3", "chart.js": "^4.5.1" } } diff --git a/src/core/signal.js b/src/core/signal.js deleted file mode 100644 index 6dc48dc..0000000 --- a/src/core/signal.js +++ /dev/null @@ -1,95 +0,0 @@ -// A ~50-line reactive core. `signal()` holds a value and remembers which effects -// read it; `effect()` runs a function and re-runs it whenever a signal it read -// changes. This is the whole mechanism behind "the UI repaints itself" — enough -// to retire manual `renderX(app)` invalidation without adopting a framework. -// -// Pure by construction: no DOM, no globals beyond the module-local tracking -// pointer, so it lives in core/ and tests with plain stubs. - -// The effect currently running — set while an effect's body executes so that any -// signal read during it knows who to subscribe. null when no effect is active -// (e.g. a read from ordinary code), in which case the read just returns a value. -let activeEffect = null; -// While > 0, signal writes queue their subscribers instead of running them, so a -// multi-signal update (e.g. set the tab list AND the active id) repaints once. -let batchDepth = 0; -const queued = new Set(); - -/** - * A reactive box around `initial`. Reading `.value` *inside* an effect subscribes - * that effect to this signal; assigning a different `.value` re-runs every - * subscribed effect. Assigning an equal value is a no-op (no needless re-render). - */ -export function signal(initial) { - let value = initial; - const subs = new Set(); - return { - get value() { - if (activeEffect) { - subs.add(activeEffect); - activeEffect.deps.add(subs); - } - return value; - }, - set value(next) { - if (Object.is(next, value)) return; - value = next; - // Copy first: an effect re-subscribes as it re-runs, mutating `subs` - // mid-iteration. Inside a batch, queue instead (deduped by the Set). - if (batchDepth > 0) { - for (const eff of subs) queued.add(eff); - return; - } - for (const eff of [...subs]) eff.run(); - }, - }; -} - -/** - * Run `fn` immediately, and again whenever any signal it read changes. Each run - * re-derives the dependency set from what `fn` actually reads, so conditional - * dependencies are tracked correctly (a signal no longer read stops triggering - * re-runs). Returns a `dispose()` that unsubscribes the effect. - */ -export function effect(fn) { - const eff = { - deps: new Set(), - run() { - cleanup(eff); - const prev = activeEffect; - activeEffect = eff; - try { - fn(); - } finally { - activeEffect = prev; - } - }, - }; - eff.run(); - return () => cleanup(eff); -} - -/** - * Run `fn`, deferring every effect triggered by signal writes inside it until - * `fn` returns, then run each affected effect once. Nesting is depth-counted, so - * only the outermost batch flushes. Returns `fn`'s result. - */ -export function batch(fn) { - batchDepth++; - try { - return fn(); - } finally { - if (--batchDepth === 0) { - const run = [...queued]; - queued.clear(); - for (const eff of run) eff.run(); - } - } -} - -// Drop this effect from every signal it was subscribed to, and forget them, so -// the next run (or a dispose) starts from a clean dependency set. -function cleanup(eff) { - for (const subs of eff.deps) subs.delete(eff); - eff.deps.clear(); -} diff --git a/src/state.js b/src/state.js index b02570f..9b0c5d9 100644 --- a/src/state.js +++ b/src/state.js @@ -6,7 +6,7 @@ import { clamp } from './core/format.js'; import { mergeSaved } from './core/saved-io.js'; import { cloneChartCfg } from './core/chart-data.js'; import { loadJSON, saveJSON, loadStr, saveStr } from './core/storage.js'; -import { signal } from './core/signal.js'; +import { signal } from '@preact/signals-core'; /** A tab's chart state as a persistable payload `{ cfg, key }`, or null. */ export function tabChart(tab) { diff --git a/src/ui/app.js b/src/ui/app.js index 81182b1..ef09ed3 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -26,7 +26,7 @@ import * as oauth from '../net/oauth.js'; import * as ch from '../net/ch-client.js'; import { mountEditor, insertAtCursor, replaceEditor, SCHEMA_GRAPH_MIME } from './editor.js'; import { renderTabs, selectTab, newTab, closeTab, loadIntoNewTab } from './tabs.js'; -import { effect } from '../core/signal.js'; +import { effect } from '@preact/signals-core'; import { renderSchema } from './schema.js'; import { renderResults } from './results.js'; import { openSchemaView } from './explain-graph.js'; diff --git a/src/ui/tabs.js b/src/ui/tabs.js index 8361847..d0e94ec 100644 --- a/src/ui/tabs.js +++ b/src/ui/tabs.js @@ -5,7 +5,7 @@ import { h } from './dom.js'; import { Icon } from './icons.js'; import { activeTab, allocTabId, newTabObj } from '../state.js'; import { cloneChartCfg } from '../core/chart-data.js'; -import { batch } from '../core/signal.js'; +import { batch } from '@preact/signals-core'; /** Paint the tab strip into app.dom.qtabsInner. */ export function renderTabs(app) { diff --git a/tests/unit/main.test.js b/tests/unit/main.test.js index 2d94fe5..7223753 100644 --- a/tests/unit/main.test.js +++ b/tests/unit/main.test.js @@ -1,6 +1,6 @@ import { describe, it, expect, vi } from 'vitest'; import { bootstrap } from '../../src/main.js'; -import { signal } from '../../src/core/signal.js'; +import { signal } from '@preact/signals-core'; function jwt(payload) { const b = (o) => Buffer.from(JSON.stringify(o)).toString('base64url'); diff --git a/tests/unit/signal.test.js b/tests/unit/signal.test.js deleted file mode 100644 index 0e5154d..0000000 --- a/tests/unit/signal.test.js +++ /dev/null @@ -1,178 +0,0 @@ -import { describe, it, expect, vi } from 'vitest'; -import { signal, effect, batch } from '../../src/core/signal.js'; - -describe('signal / effect — the reactive core', () => { - it('reads the current value outside any effect (no subscription)', () => { - const s = signal(1); - expect(s.value).toBe(1); // exercises the `if (activeEffect)` false branch - }); - - it('runs an effect once immediately', () => { - const s = signal('a'); - const spy = vi.fn(() => s.value); - effect(spy); - expect(spy).toHaveBeenCalledTimes(1); - }); - - it('re-runs the effect when a read signal changes', () => { - const s = signal(1); - const seen = []; - effect(() => seen.push(s.value)); - s.value = 2; - s.value = 3; - expect(seen).toEqual([1, 2, 3]); - }); - - it('does NOT re-run when the value is unchanged (Object.is)', () => { - const s = signal(1); - const spy = vi.fn(() => s.value); - effect(spy); - s.value = 1; // equal → no-op - expect(spy).toHaveBeenCalledTimes(1); - }); - - it('only notifies effects that actually read the signal', () => { - const a = signal(0); - const b = signal(0); - const aSpy = vi.fn(() => a.value); - const bSpy = vi.fn(() => b.value); - effect(aSpy); - effect(bSpy); - a.value = 1; - expect(aSpy).toHaveBeenCalledTimes(2); - expect(bSpy).toHaveBeenCalledTimes(1); // b untouched - }); - - it('tracks conditional dependencies — a no-longer-read signal stops triggering', () => { - const useB = signal(true); - const b = signal('x'); - const spy = vi.fn(() => { - if (useB.value) return b.value; - return 'const'; - }); - effect(spy); - expect(spy).toHaveBeenCalledTimes(1); - - b.value = 'y'; // still read → re-runs - expect(spy).toHaveBeenCalledTimes(2); - - useB.value = false; // effect re-runs and stops reading b - expect(spy).toHaveBeenCalledTimes(3); - - b.value = 'z'; // b no longer a dependency → no re-run - expect(spy).toHaveBeenCalledTimes(3); - }); - - it('supports nested effects (restores the previous active effect)', () => { - const outer = signal(0); - const inner = signal(0); - const innerSpy = vi.fn(() => inner.value); - const outerSpy = vi.fn(() => { - outer.value; - effect(innerSpy); // nested: activeEffect must restore to outer after this - }); - effect(outerSpy); - expect(outerSpy).toHaveBeenCalledTimes(1); - expect(innerSpy).toHaveBeenCalledTimes(1); - - inner.value = 1; // only the inner effect depends on inner - expect(innerSpy).toHaveBeenCalledTimes(2); - expect(outerSpy).toHaveBeenCalledTimes(1); - }); - - it('batch() coalesces multiple signal writes into one re-run', () => { - const a = signal(0); - const b = signal(0); - const spy = vi.fn(() => a.value + b.value); // depends on both - effect(spy); - expect(spy).toHaveBeenCalledTimes(1); - batch(() => { - a.value = 1; // queued, not run - b.value = 2; // queued (same effect → deduped by the Set) - }); - expect(spy).toHaveBeenCalledTimes(2); // one flush, not two - expect(a.value + b.value).toBe(3); - }); - - it('batch() returns the callback result and flushes only at the outermost level', () => { - const s = signal(0); - const spy = vi.fn(() => s.value); - effect(spy); - const out = batch(() => { - s.value = 1; - batch(() => { s.value = 2; }); // nested → does NOT flush here - expect(spy).toHaveBeenCalledTimes(1); // still deferred mid-batch - return 'done'; - }); - expect(out).toBe('done'); - expect(spy).toHaveBeenCalledTimes(2); // single flush after the outer batch - }); - - it('dispose() unsubscribes the effect', () => { - const s = signal(1); - const spy = vi.fn(() => s.value); - const dispose = effect(spy); - s.value = 2; - expect(spy).toHaveBeenCalledTimes(2); - dispose(); - s.value = 3; // no longer subscribed - expect(spy).toHaveBeenCalledTimes(2); - }); -}); - -// A focused demonstration: the same tab lifecycle as src/ui/tabs.js, but the -// "render" runs itself. Note there is no refresh() that has to remember to call -// renderTabs + rerenderResults + updateSaveBtn — each view subscribes to what it -// reads, and mutating state is all the caller does. -describe('demonstration: tabs.js lifecycle without manual refresh()', () => { - function makeTabsModel() { - const tabs = signal([{ id: 't1', name: 'Untitled' }]); - const activeId = signal('t1'); - let nextId = 2; - return { - tabs, - activeId, - selectTab: (id) => { activeId.value = id; }, - newTab: () => { - const id = 't' + nextId++; - tabs.value = [...tabs.value, { id, name: 'Untitled' }]; - activeId.value = id; // both reads update; no manual repaint call - }, - closeTab: (id) => { - tabs.value = tabs.value.filter((t) => t.id !== id); - if (activeId.value === id) activeId.value = tabs.value[tabs.value.length - 1].id; - }, - }; - } - - it('repaints the strip + the active-tab-dependent views automatically', () => { - const m = makeTabsModel(); - - // Three independent "views", each declaring what it reads — exactly the - // three things tabs.js's refresh() repaints by hand today. - const strip = vi.fn(() => m.tabs.value.map((t) => t.id)); - const saveBtn = vi.fn(() => m.activeId.value); // "Save" cares about active tab - const results = vi.fn(() => m.activeId.value); - - effect(strip); - effect(saveBtn); - effect(results); - expect([strip, saveBtn, results].map((f) => f.mock.calls.length)).toEqual([1, 1, 1]); - - m.newTab(); // strip changes (new tab) AND active changes - expect(strip).toHaveBeenCalledTimes(2); - expect(saveBtn).toHaveBeenCalledTimes(2); - expect(results).toHaveBeenCalledTimes(2); - - m.selectTab('t1'); // only active changes → strip is NOT repainted - expect(strip).toHaveBeenCalledTimes(2); // unchanged — surgical - expect(saveBtn).toHaveBeenCalledTimes(3); - expect(results).toHaveBeenCalledTimes(3); - - m.closeTab('t2'); // t2 doesn't exist anymore? it's 't2' from newTab → close it - // (newTab created 't2' and selected it; selectTab('t1') made t1 active) - expect(m.tabs.value.map((t) => t.id)).toEqual(['t1']); - expect(strip).toHaveBeenCalledTimes(3); // strip repainted (list shrank) - expect(saveBtn).toHaveBeenCalledTimes(3); // active was already t1 → no change - }); -}); From ce2f09b138e5aba1f35cde6c932cfa575e89fc6e Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Mon, 29 Jun 2026 23:41:35 +0200 Subject: [PATCH 05/14] docs: record @preact/signals-core as the chosen reactivity primitive MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01KQ6VFCwwUWnU3YgHQK2K5k --- THIRD-PARTY-NOTICES.md | 16 ++++++++++- build/build.mjs | 6 ++--- docs/ADR-0001-reactivity.md | 53 ++++++++++++++++++++++++++----------- 3 files changed, 55 insertions(+), 20 deletions(-) diff --git a/THIRD-PARTY-NOTICES.md b/THIRD-PARTY-NOTICES.md index 37cdacf..1b1f08f 100644 --- a/THIRD-PARTY-NOTICES.md +++ b/THIRD-PARTY-NOTICES.md @@ -1,7 +1,7 @@ # Third-party notices The Altinity SQL Browser is licensed under Apache-2.0 (see `LICENSE`). The built -single-file artifact (`dist/sql.html`) inlines the two runtime dependencies +single-file artifact (`dist/sql.html`) inlines the three runtime dependencies below; this file reproduces their MIT license texts as required, and the same notices are embedded as a comment at the top of the built artifact. @@ -32,3 +32,17 @@ Permission is hereby granted, free of charge, to any person obtaining a copy of The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. + +--- + +## @preact/signals-core — v1.14.3 + +The MIT License (MIT) + +Copyright (c) 2022-present Preact Team + +Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated documentation files (the "Software"), to deal in the Software without restriction, including without limitation the rights to use, copy, modify, merge, publish, distribute, sublicense, and/or sell copies of the Software, and to permit persons to whom the Software is furnished to do so, subject to the following conditions: + +The above copyright notice and this permission notice shall be included in all copies or substantial portions of the Software. + +THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER DEALINGS IN THE SOFTWARE. diff --git a/build/build.mjs b/build/build.mjs index c513761..1c0cafa 100644 --- a/build/build.mjs +++ b/build/build.mjs @@ -2,7 +2,7 @@ // is inlined (with the stylesheet) into build/template.html → dist/sql.html. // // esbuild is the only build-time tool; the bundled runtime dependencies are -// Chart.js and @dagrejs/dagre (inlined, not fetched). The output is a self-contained HTML file +// Chart.js, @dagrejs/dagre, and @preact/signals-core (inlined, not fetched). The output is a self-contained HTML file // that installs into any ClickHouse cluster's user_files and is served by an // static rule — it still makes zero third-party requests. @@ -52,8 +52,8 @@ async function main() { const styles = await readFile(resolve(root, 'src/styles.css'), 'utf8'); const template = await readFile(resolve(here, 'template.html'), 'utf8'); - // The two runtime deps (Chart.js, dagre) are MIT and inlined into the bundle, - // so the artifact must carry their notices. esbuild strips legal comments + // The runtime deps (Chart.js, dagre, @preact/signals-core) are MIT and inlined + // into the bundle, so the artifact must carry their notices. esbuild strips legal comments // (legalComments: 'none'), so embed THIRD-PARTY-NOTICES.md as a leading HTML // comment — sanitized so its text can't close the comment early. const notices = (await readFile(resolve(root, 'THIRD-PARTY-NOTICES.md'), 'utf8')).replace(/--+>?/g, '-'); diff --git a/docs/ADR-0001-reactivity.md b/docs/ADR-0001-reactivity.md index a46fbec..a4a0e7f 100644 --- a/docs/ADR-0001-reactivity.md +++ b/docs/ADR-0001-reactivity.md @@ -3,7 +3,8 @@ - **Status:** Proposed - **Date:** 2026-06-29 - **Context tracking:** roadmap #68 -- **Evidence:** branch `spike/signals` (primitive + two converted slices) +- **Evidence:** `spike/signals` (hand-rolled primitive + two converted slices), + `spike/signals-core` (same two slices on `@preact/signals-core`) ## Context @@ -23,27 +24,42 @@ per-file 100/100/100/100 coverage gate (rule 1), and the injected-seam layering ## Decision -Adopt a **~70-line in-house reactive primitive** (`src/core/signal.js`: -`signal()` / `effect()` / `batch()`) and migrate state **one slice at a time**, -behind accessor helpers. **Do not** adopt a UI framework (React/Preact/Solid). +Adopt **`@preact/signals-core`** (`signal()` / `effect()` / `computed()` / +`batch()` / `untracked()`) and migrate state **one slice at a time**, behind +accessor helpers. **Do not** adopt a UI framework (React/Preact/Solid). -## Options considered +A hand-rolled ~70-line primitive was prototyped first and works (`spike/signals`), +but `@preact/signals-core` was chosen over it: same `.value` API (so the +migration code is identical and the choice is reversible in ~5 lines), but +maintained, glitch-free (correct topological/diamond updates), and with a lazy +memoized `computed()` — none of which the naive synchronous prototype provides. +It costs one small, zero-transitive-dependency package and **+1.4 KB gzip** to +the artifact (vs +0.45 KB hand-rolled), in exchange for not owning ~70 lines + +~180 test lines of correctness-critical reactive code. Per CLAUDE.md rule 4 this +is a deliberate dependency; it is still inlined, so the artifact makes zero +third-party requests. -| Option | gzip cost | Verdict | +## Options considered (all measured) + +| Option | artifact Δ (gzip) | Verdict | |---|---|---| -| **In-house `signal`/`effect`** | ~0.45 KB (measured) | **Chosen** — retires manual invalidation; zero deps; proven 100%-coverable | -| Preact + @preact/signals | ~7.3 KB (measured, `spike/preact`) | Rejected for now — only added value over the primitive is auto DOM-diffing of large lists, which we don't need | -| Solid.js | ~7 KB | Rejected — compiler/plugin + ecosystem cost; same "big-list" benefit we don't need | -| React (proper) | ~45 KB | Rejected — heaviest dep, vDOM-on-big-tables liability, mismatch with the single-file ethos | +| **@preact/signals-core** | **+1.4 KB** | **Chosen** — maintained, glitch-free, `computed`; same `.value` API; we own no reactive code | +| Hand-rolled `signal.js` | +0.45 KB | Viable (`spike/signals`) — zero deps, but we own + 100%-cover it; naive (not glitch-free), no `computed` | +| Preact + @preact/signals | +7.3 KB (`spike/preact`) | Rejected — its only edge over signals-core is auto DOM-diffing of large lists, which we don't need | +| Solid.js | ~+7 KB | Rejected — compiler/plugin + ecosystem cost; same unneeded big-list benefit | +| React (proper) | ~+45 KB | Rejected — heaviest dep, vDOM-on-big-tables liability, mismatch with the single-file ethos | Decisive factor: we do **not** need a virtualized / large-list renderer (no -10k-row grid requirement), which is the one thing a vDOM framework buys over the -primitive. The pain is invalidation, which the primitive solves directly. +10k-row grid requirement), which is the one thing a vDOM framework buys over a +bare signals core. The pain is invalidation, which signals solve directly — and +between the two signals options, a maintained, glitch-free core beats owning the +primitive for ~1 KB. ## Evidence (spike) -Two slices converted end-to-end with the full suite + coverage gate green -(1033 tests; `signal.js` 100/100/100/100; real `npm run build` succeeds): +Two slices converted end-to-end with the full suite + per-file coverage gate +green and a real `npm run build` (1033 tests on the hand-rolled branch; 1022 on +`spike/signals-core`, which drops the 11 primitive-internal tests): - **tabs** (`tabs` + `activeTabId`) — *has* an `activeTab()` accessor → 16 callers insulated; low mechanical churn, but one behavioral test relocation (the @@ -52,8 +68,10 @@ Two slices converted end-to-end with the full suite + coverage gate green mechanical `.value` and concentrated in the panel's own test file; no assertion rewrites. -Cumulative spike cost: 12 files, +432/−142, most of which is the reusable -primitive (95) + its tests (178). +Both branches share the slice conversions verbatim. Moving from the hand-rolled +primitive to `@preact/signals-core` was a 5-line diff (import swaps + deleting +`src/core/signal.js` and its test), demonstrating the API parity and that the +choice is reversible. ## Consequences @@ -64,5 +82,8 @@ primitive (95) + its tests (178). - Per-slice cost is mostly mechanical `.value` edits in that slice's tests. - **Rule:** give each converted slice an accessor helper (like `activeTab()`) to contain reader churn and localize behavior. +- Adding `@preact/signals-core` makes **three** bundled runtime deps. On adoption, + update CLAUDE.md rule 4 ("two bundled runtime dependencies") and the count in + THIRD-PARTY-NOTICES.md (done) / `build/build.mjs` comments (done). - Re-evaluate Preact/Solid only if the UI later grows many interdependent components with rich local state, or a genuine large-list render need appears. From 5fb74e7a391406c59356483ee8043712fed6b09f Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 07:15:12 +0200 Subject: [PATCH 06/14] spike: convert the run-flow slice (resultView + running) to signals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01FLVYgQpkbEbHKwEAA4aawZ --- src/state.js | 15 ++++++----- src/ui/app.js | 51 +++++++++++++++++++++++------------- src/ui/results.js | 16 +++++------ src/ui/shortcuts.js | 2 +- tests/unit/app.test.js | 31 +++++++++++++++------- tests/unit/results.test.js | 10 +++++-- tests/unit/shortcuts.test.js | 4 +-- tests/unit/state.test.js | 6 ++--- 8 files changed, 86 insertions(+), 49 deletions(-) diff --git a/src/state.js b/src/state.js index 9b0c5d9..395cc53 100644 --- a/src/state.js +++ b/src/state.js @@ -49,9 +49,10 @@ export function createState(read = { loadJSON, loadStr }) { sidebarPx: clamp(parseInt(read.loadStr(KEYS.sidebarPx, '248'), 10), 180, 420), editorPct: num(KEYS.editorPct, 45, 15, 85), sideSplitPct: num(KEYS.sideSplitPct, 58, 25, 85), - // Reactive (signals): renderTabs + the editor/results/save-button repaint via - // an effect that reads these, so mutating them is all a caller does — no - // manual refresh() list to keep in sync. Read/write through `.value`. + // Reactive (signals): mutating these drives repaints via effects in + // createApp — no manual refresh() list to keep in sync. Read/write through + // `.value`. tabs/activeTabId drive renderTabs + the editor + the save button; + // the results pane + Run button react to resultView/running (below). tabs: signal([newTabObj('t1')]), activeTabId: signal('t1'), schema: null, @@ -59,9 +60,11 @@ export function createState(read = { loadJSON, loadStr }) { schemaFilter: '', expandedTables: new Set(), serverVersion: null, - running: false, + // Run state (signals): `running` flips the Run button + results pane via + // effects; `resultView` is the active Table/JSON/Chart tab. Via `.value`. + running: signal(false), abortController: null, - resultView: 'table', + resultView: signal('table'), // `forceExplain` is set by the Explain button to put an ordinary query into // EXPLAIN-view mode; a normal Run clears it (session-only). The active view is // derived per-run from the typed statement / clicked tab, not stored here. @@ -116,7 +119,7 @@ export function saveQuery(state, tab, name, description, save = saveJSON, now = const chart = tabChart(tab); // Remember the current result view (Table/JSON/Chart) so a restore reopens the // same data representation; the transient raw view isn't persisted. - const view = SAVED_VIEWS.has(state.resultView) ? state.resultView : undefined; + const view = SAVED_VIEWS.has(state.resultView.value) ? state.resultView.value : undefined; let entry = savedForTab(state, tab); if (entry) { entry.name = nm; diff --git a/src/ui/app.js b/src/ui/app.js index ef09ed3..c5814bf 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -26,7 +26,7 @@ import * as oauth from '../net/oauth.js'; import * as ch from '../net/ch-client.js'; import { mountEditor, insertAtCursor, replaceEditor, SCHEMA_GRAPH_MIME } from './editor.js'; import { renderTabs, selectTab, newTab, closeTab, loadIntoNewTab } from './tabs.js'; -import { effect } from '@preact/signals-core'; +import { effect, batch } from '@preact/signals-core'; import { renderSchema } from './schema.js'; import { renderResults } from './results.js'; import { openSchemaView } from './explain-graph.js'; @@ -399,7 +399,7 @@ export function createApp(env = {}) { app.tickElapsed = tickElapsed; async function run(opts) { - if (app.state.running) return; // already running — cancel via cancel()/Esc + if (app.state.running.value) return; // already running — cancel via cancel()/Esc const tab = app.activeTab(); if (!tab.sql.trim()) return; await ensureConfig(); @@ -442,17 +442,21 @@ export function createApp(env = {}) { tab.result = newResult(fmt); if (explainView) tab.result.explainView = explainView; app.state.resultSort = { col: null, dir: 'asc' }; - // Keep the current Table/JSON/Chart tab across re-runs (#34); a saved-query - // open passes its remembered view in opts.view to restore that instead. - const view = opts && opts.view; - app.state.resultView = ['table', 'json', 'chart'].includes(view) ? view : app.state.resultView; - app.state.running = true; app.state.runT0 = t0; app.state.runQueryId = cryptoObj.randomUUID ? cryptoObj.randomUUID() : 'q' + t0; - setRunBtn(true); - renderResults(app); app.state.abortController = new AbortController(); app.state.runTick = setInterval(tickElapsed, 100); + // Keep the current Table/JSON/Chart tab across re-runs (#34); a saved-query + // open passes its remembered view in opts.view to restore that instead. + const view = opts && opts.view; + // Flip the run signals last, in one batch: the results + Run-button effects + // fire on this write and read runT0/elapsed, so the bookkeeping above must + // already be set. (The old explicit setRunBtn(true)/renderResults are now + // those effects' job.) + batch(() => { + app.state.resultView.value = ['table', 'json', 'chart'].includes(view) ? view : app.state.resultView.value; + app.state.running.value = true; + }); try { const out = await ch.runQuery(chCtx, runSql, { @@ -475,19 +479,20 @@ export function createApp(env = {}) { } finally { clearInterval(app.state.runTick); app.state.runTick = null; - app.state.running = false; app.state.abortController = null; app.state.runQueryId = null; app.state.runT0 = null; tab.result.progress.elapsed_ns = (now() - t0) * 1e6; - setRunBtn(false); - renderResults(app); + // Flip running off last: the results + Run-button effects fire here and + // render the final stats, so elapsed_ns must already be recorded. (Old + // explicit setRunBtn(false)/renderResults are now those effects' job.) + app.state.running.value = false; if (!tab.result.error && !tab.result.cancelled) app.recordHistory(tab); } } // Stop an in-flight query: abort the stream and KILL QUERY on the server. function cancel() { - if (!app.state.running) return; + if (!app.state.running.value) return; if (app.state.abortController) app.state.abortController.abort(); ch.killQuery(chCtx, app.state.runQueryId, sqlString); } @@ -525,8 +530,8 @@ export function createApp(env = {}) { tab.result = newResult('Table'); tab.result.error = msg; tab.result.formatError = true; // a format error, not a run result (so success can clear just this) - app.state.resultView = 'table'; - renderResults(app); + app.state.resultView.value = 'table'; + renderResults(app); // explicit: the format-error tab.result is an in-place write, and resultView may already be 'table' (no effect) const pos = parseErrorPos(msg); if (pos != null) app.dom.editorRevealCaret(pos); } @@ -941,16 +946,26 @@ export function renderApp(app, helpers) { mountEditor(app, app.dom.editorRegion); // Reactive repaint of the tab-dependent surface — replaces the old tabs.js // refresh(): re-runs whenever the tab list or active tab changes, so tab ops - // just mutate the signals. (Result-data repaints still call renderResults - // directly from the run flow; those aren't tab changes.) + // just mutate the signals. effect(() => { app.state.tabs.value; app.state.activeTabId.value; renderTabs(app); if (app.dom.editorSync) app.dom.editorSync(); - renderResults(app); app.updateSaveBtn(); }); + // Reactive repaint of the results pane: re-runs on a tab switch, a Table/JSON/ + // Chart view change, or a run-state flip. (renderResults' activeTab() also + // reads tabs.value, so a tab-list change repaints here too.) Streaming-data + // repaints still call renderResults directly from run()'s onChunk. + effect(() => { + app.state.activeTabId.value; + app.state.resultView.value; + app.state.running.value; + renderResults(app); + }); + // The Run button reflects the run state (label + disabled). + effect(() => app.setRunBtn(app.state.running.value)); renderSchema(app); // Reactive repaint of the side panel: re-runs when the active panel changes // (Library ↔ History). Data-driven repaints (savedQueries/history mutations) diff --git a/src/ui/results.js b/src/ui/results.js index 3df35eb..3d41a28 100644 --- a/src/ui/results.js +++ b/src/ui/results.js @@ -92,8 +92,8 @@ export function renderResults(app) { const inner = h('div', { class: 'res-body' }); // While running, pin a streaming strip to the top of the body: a determinate // fill at read/total when known, else an indeterminate sweep. - if (app.state.running) inner.appendChild(streamStrip(r)); - const streamingBlank = app.state.running && (!r || (r.rows.length === 0 && r.rawText == null)); + if (app.state.running.value) inner.appendChild(streamStrip(r)); + const streamingBlank = app.state.running.value && (!r || (r.rows.length === 0 && r.rawText == null)); if (streamingBlank) { inner.appendChild(h('div', { class: 'placeholder starting' }, h('span', { class: 'spin' }, Icon.spinner()), @@ -116,9 +116,9 @@ export function renderResults(app) { inner.appendChild(h('div', { class: 'raw-text-view', tabindex: '0' }, r.rawText)); } else if (r.rows.length === 0) { inner.appendChild(h('div', { class: 'placeholder' }, h('div', null, 'Query returned 0 rows.'))); - } else if (app.state.resultView === 'json') { + } else if (app.state.resultView.value === 'json') { inner.appendChild(renderJson(r)); - } else if (app.state.resultView === 'chart') { + } else if (app.state.resultView.value === 'chart') { inner.appendChild(renderChart(app, r)); } else { inner.appendChild(renderTable(app, r)); @@ -188,10 +188,10 @@ function buildToolbar(app, r) { { id: 'chart', label: 'Chart', icon: Icon.chart() }, ]; for (const v of views) { - const isActive = app.state.resultView === v.id || (isRaw && v.id === 'raw'); + const isActive = app.state.resultView.value === v.id || (isRaw && v.id === 'raw'); tabs.appendChild(h('button', { class: 'result-view-tab' + (isActive ? ' active' : ''), - onclick: () => { app.state.resultView = v.id; renderResults(app); }, + onclick: () => { app.state.resultView.value = v.id; }, }, v.icon, h('span', null, v.label))); } } @@ -200,7 +200,7 @@ function buildToolbar(app, r) { // EXPLAIN views suppress the ms/rows/bytes stats — they're not meaningful for a // plan and the freed space lets the five tabs breathe. const showStats = !(r && r.explainView); - if (app.state.running) { + if (app.state.running.value) { // Live counters (accent, mono) + Cancel — replaces the static stats while // streaming. The ms element is updated in place by app.tickElapsed(). if (showStats) { @@ -432,7 +432,7 @@ export function renderChart(app, r) { // Gate on run state BEFORE deriving the config: while a query streams its // columns can be empty (pre-meta), and letting chartCfgFor see that empty // schema would clobber a restored saved/shared config with autoChart(null). - if (app.state.running) return chartEmpty(Icon.spinner(), 'Chart renders when the query completes.'); + if (app.state.running.value) return chartEmpty(Icon.spinner(), 'Chart renders when the query completes.'); const cfg = chartCfgFor(tab, r.columns); if (!cfg) return chartEmpty(Icon.chart(), 'These results aren’t chartable — add a numeric column to plot them.'); diff --git a/src/ui/shortcuts.js b/src/ui/shortcuts.js index 2571745..64d94d5 100644 --- a/src/ui/shortcuts.js +++ b/src/ui/shortcuts.js @@ -57,7 +57,7 @@ export function handleKeydown(e, app) { const mod = e.metaKey || e.ctrlKey; const signedIn = app.isSignedIn(); // Esc cancels an in-flight query (aborts the stream + KILL QUERY). - if (e.key === 'Escape' && app.state.running) { + if (e.key === 'Escape' && app.state.running.value) { e.preventDefault(); app.actions.cancel(); return 'cancel'; diff --git a/tests/unit/app.test.js b/tests/unit/app.test.js index 5b60200..13dcebb 100644 --- a/tests/unit/app.test.js +++ b/tests/unit/app.test.js @@ -291,17 +291,30 @@ describe('query run', () => { const routes = [[(u, sql) => /SELECT 1/.test(sql), resp({ body: streamBody(['{"meta":[{"name":"a","type":"UInt8"}]}\n', '{"row":{"a":"1"}}\n']) })]]; const { app } = appForRun(routes, { Chart: class { destroy() {} } }); // Chart seam so the chart view renders app.activeTab().sql = 'SELECT 1'; - app.state.resultView = 'chart'; + app.state.resultView.value = 'chart'; await app.actions.run(); // no opts → keep the current (chart) tab - expect(app.state.resultView).toBe('chart'); + expect(app.state.resultView.value).toBe('chart'); await app.actions.run({ view: 'json' }); // saved-query open restores its view - expect(app.state.resultView).toBe('json'); + expect(app.state.resultView.value).toBe('json'); await app.actions.run({ view: 'table' }); - expect(app.state.resultView).toBe('table'); + expect(app.state.resultView.value).toBe('table'); await app.actions.run({ view: 'chart' }); - expect(app.state.resultView).toBe('chart'); + expect(app.state.resultView.value).toBe('chart'); await app.actions.run({ view: 'bogus' }); // unknown view → keep current (chart) - expect(app.state.resultView).toBe('chart'); + expect(app.state.resultView.value).toBe('chart'); + }); + it('switching the result view repaints via the effect (the view-tab onclick only sets the signal)', async () => { + const routes = [[(u, sql) => /SELECT 1/.test(sql), resp({ body: streamBody(['{"meta":[{"name":"a","type":"UInt8"}]}\n', '{"row":{"a":"1"}}\n']) })]]; + const { app } = appForRun(routes); + app.activeTab().sql = 'SELECT 1'; + await app.actions.run(); + const region = app.dom.resultsRegion; + expect(region.querySelector('.res-table')).not.toBeNull(); // table view by default + const jsonTab = [...region.querySelectorAll('.result-view-tab')].find((b) => b.textContent.includes('JSON')); + jsonTab.dispatchEvent(new Event('click', { bubbles: true })); + expect(app.state.resultView.value).toBe('json'); + expect(region.querySelector('.json-view')).not.toBeNull(); // repainted by the results effect, not the onclick + expect(region.querySelector('.res-table')).toBeNull(); }); it('no-ops on empty SQL', async () => { const { app } = appForRun([]); @@ -311,12 +324,12 @@ describe('query run', () => { }); it('run() while already running is a no-op (cancel is separate)', async () => { const { app } = appForRun([]); - app.state.running = true; + app.state.running.value = true; const ac = { abort: vi.fn() }; app.state.abortController = ac; await app.actions.run(); expect(ac.abort).not.toHaveBeenCalled(); // re-running no longer aborts - expect(app.state.running).toBe(true); + expect(app.state.running.value).toBe(true); }); it('setRunBtn: "Running…" with no trailing "null"; "Run" + kbd when idle', () => { const { app } = appForRun([]); @@ -341,7 +354,7 @@ describe('query run', () => { const { app, e } = appForRun([]); app.actions.cancel(); // idle → no-op, no throw const abort = vi.fn(); - app.state.running = true; + app.state.running.value = true; app.state.abortController = { abort, signal: {} }; app.state.runQueryId = 'qid-1'; app.actions.cancel(); diff --git a/tests/unit/results.test.js b/tests/unit/results.test.js index b1832b4..90ebb18 100644 --- a/tests/unit/results.test.js +++ b/tests/unit/results.test.js @@ -9,7 +9,13 @@ const click = (el) => el.dispatchEvent(new Event('click', { bubbles: true })); function appWithResult(result, over = {}) { const app = makeApp(); app.activeTab().result = result; - Object.assign(app.state, over); + // Signal-aware assign: resultView/running are signals — write through .value; + // plain fields are set directly. + for (const [k, v] of Object.entries(over)) { + const cur = app.state[k]; + if (cur && typeof cur === 'object' && 'value' in cur) cur.value = v; + else app.state[k] = v; + } return app; } @@ -116,7 +122,7 @@ describe('renderResults states', () => { renderResults(app); const jsonTab = [...app.dom.resultsRegion.querySelectorAll('.result-view-tab')].find((b) => b.textContent.includes('JSON')); click(jsonTab); - expect(app.state.resultView).toBe('json'); + expect(app.state.resultView.value).toBe('json'); }); }); diff --git a/tests/unit/shortcuts.test.js b/tests/unit/shortcuts.test.js index 440bbb9..d4f85c5 100644 --- a/tests/unit/shortcuts.test.js +++ b/tests/unit/shortcuts.test.js @@ -63,10 +63,10 @@ describe('handleKeydown', () => { }); it('Escape cancels a running query, and is a no-op otherwise', () => { const app = makeApp(); - app.state.running = false; + app.state.running.value = false; expect(handleKeydown(ev({ key: 'Escape' }), app)).toBeNull(); expect(app.actions.cancel).not.toHaveBeenCalled(); - app.state.running = true; + app.state.running.value = true; expect(handleKeydown(ev({ key: 'Escape' }), app)).toBe('cancel'); expect(app.actions.cancel).toHaveBeenCalled(); }); diff --git a/tests/unit/state.test.js b/tests/unit/state.test.js index dd8c393..f727f43 100644 --- a/tests/unit/state.test.js +++ b/tests/unit/state.test.js @@ -246,15 +246,15 @@ describe('saved queries', () => { const save = vi.fn(); const tab = s.tabs.value[0]; tab.sql = 'SELECT 1'; - s.resultView = 'chart'; + s.resultView.value = 'chart'; const e = saveQuery(s, tab, 'V', '', save, 100); expect(e.view).toBe('chart'); // re-save under a different view → updates - s.resultView = 'json'; + s.resultView.value = 'json'; saveQuery(s, tab, 'V', '', save, 200); expect(s.savedQueries[0].view).toBe('json'); // raw view (TSV/JSON output) is not a saved view → dropped - s.resultView = 'raw'; + s.resultView.value = 'raw'; saveQuery(s, tab, 'V', '', save, 300); expect(s.savedQueries[0].view).toBeUndefined(); }); From ab6eb245581feea1535e47a7f0d54a48091a33f9 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 07:23:13 +0200 Subject: [PATCH 07/14] spike: convert the library-title slice (libraryName + libraryDirty) to signals MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01FLVYgQpkbEbHKwEAA4aawZ --- src/state.js | 40 ++++++++++++++++++---------------- src/ui/app.js | 18 ++++++++++------ src/ui/file-menu.js | 25 ++++++++++----------- src/ui/saved-history.js | 5 ++--- tests/helpers/fake-app.js | 1 - tests/unit/file-menu.test.js | 38 ++++++++++++++++---------------- tests/unit/state.test.js | 42 ++++++++++++++++++------------------ 7 files changed, 88 insertions(+), 81 deletions(-) diff --git a/src/state.js b/src/state.js index 395cc53..2e5f8d3 100644 --- a/src/state.js +++ b/src/state.js @@ -74,10 +74,12 @@ export function createState(read = { loadJSON, loadStr }) { savedQueries: read.loadJSON(KEYS.saved, []), history: read.loadJSON(KEYS.history, []), // The saved-query collection treated as a named document ("the Library"). - // `libraryName` is persisted; `libraryDirty` (unsaved changes since the last - // file Save/Replace/New) is session-only and resets on reload. - libraryName: read.loadStr(KEYS.libraryName, DEFAULT_LIBRARY_NAME), - libraryDirty: false, + // Signals: the header title (name + unsaved-changes dot) repaints via an + // effect that reads these. `libraryName` is persisted; `libraryDirty` + // (unsaved changes since the last file Save/Replace/New) is session-only and + // resets on reload. Read/write via `.value`. + libraryName: signal(read.loadStr(KEYS.libraryName, DEFAULT_LIBRARY_NAME)), + libraryDirty: signal(false), // Transient search text for the Library/History side panel (session-only, // cleared on a tab switch); never persisted. libraryFilter: '', @@ -136,7 +138,7 @@ export function saveQuery(state, tab, name, description, save = saveJSON, now = tab.savedId = entry.id; } tab.name = nm; - state.libraryDirty = true; + state.libraryDirty.value = true; save(KEYS.saved, state.savedQueries); return entry; } @@ -156,7 +158,7 @@ export function renameSaved(state, id, name, description, save = saveJSON) { if (desc) entry.description = desc; else delete entry.description; } for (const t of tabsForSaved(state, id)) t.name = nm; - state.libraryDirty = true; + state.libraryDirty.value = true; save(KEYS.saved, state.savedQueries); } @@ -165,7 +167,7 @@ export function toggleFavorite(state, id, save = saveJSON) { const entry = state.savedQueries.find((q) => q.id === id); if (!entry) return; entry.favorite = !entry.favorite; - state.libraryDirty = true; + state.libraryDirty.value = true; save(KEYS.saved, state.savedQueries); } @@ -204,7 +206,7 @@ export function filterHistory(list, query) { export function importSaved(state, queries, save = saveJSON, genId = () => makeId('s', Date.now())) { const { merged, added, updated, skipped } = mergeSaved(state.savedQueries, queries, genId); state.savedQueries = merged; - state.libraryDirty = true; + state.libraryDirty.value = true; save(KEYS.saved, state.savedQueries); return { added, updated, skipped }; } @@ -213,7 +215,7 @@ export function importSaved(state, queries, save = saveJSON, genId = () => makeI export function deleteSaved(state, id, save = saveJSON) { state.savedQueries = state.savedQueries.filter((q) => q.id !== id); for (const t of tabsForSaved(state, id)) t.savedId = null; - state.libraryDirty = true; + state.libraryDirty.value = true; save(KEYS.saved, state.savedQueries); } @@ -231,9 +233,9 @@ function pruneTabLinks(state) { /** Rename the library (blank → the default name). Marks dirty; persists name. */ export function renameLibrary(state, name, saveName = saveStr) { - state.libraryName = String(name || '').trim() || DEFAULT_LIBRARY_NAME; - state.libraryDirty = true; - saveName(KEYS.libraryName, state.libraryName); + state.libraryName.value = String(name || '').trim() || DEFAULT_LIBRARY_NAME; + state.libraryDirty.value = true; + saveName(KEYS.libraryName, state.libraryName.value); } /** Start an empty, default-named library. Clears dirty; open tabs are kept @@ -241,10 +243,10 @@ export function renameLibrary(state, name, saveName = saveStr) { export function newLibrary(state, save = saveJSON, saveName = saveStr) { state.savedQueries = []; pruneTabLinks(state); - state.libraryName = DEFAULT_LIBRARY_NAME; - state.libraryDirty = false; + state.libraryName.value = DEFAULT_LIBRARY_NAME; + state.libraryDirty.value = false; save(KEYS.saved, state.savedQueries); - saveName(KEYS.libraryName, state.libraryName); + saveName(KEYS.libraryName, state.libraryName.value); } /** Replace the library with `queries`, adopting the loaded file's base name. @@ -268,10 +270,10 @@ export function replaceLibrary(state, queries, fileName, save = saveJSON, saveNa }); pruneTabLinks(state); const base = String(fileName || '').replace(/\.[^.]+$/, '').trim(); - state.libraryName = base || DEFAULT_LIBRARY_NAME; - state.libraryDirty = false; + state.libraryName.value = base || DEFAULT_LIBRARY_NAME; + state.libraryDirty.value = false; save(KEYS.saved, state.savedQueries); - saveName(KEYS.libraryName, state.libraryName); + saveName(KEYS.libraryName, state.libraryName.value); } /** Append `queries` into the library via the standard merge dedupe (sets dirty @@ -282,7 +284,7 @@ export function appendLibrary(state, queries, save = saveJSON, genId = () => mak /** Mark the library as saved to a file (clears the unsaved-changes dot). */ export function markLibrarySaved(state) { - state.libraryDirty = false; + state.libraryDirty.value = false; } /** Record a successful run in history (most-recent first, capped at 50). */ diff --git a/src/ui/app.js b/src/ui/app.js index c5814bf..fab5ccb 100644 --- a/src/ui/app.js +++ b/src/ui/app.js @@ -102,11 +102,10 @@ export function createApp(env = {}) { app.saveStr = saveStr; app.savePref = (name, value) => saveStr(KEYS[name], String(value)); app.FileReader = env.FileReader || win.FileReader; - // Exposed seams for the header File menu (file-menu.js): the file-download - // helper (defined below) and a library-title refresh (dirty dot + name) run - // after a library mutation made outside file-menu.js (e.g. the save popover). + // Exposed seam for the header File menu (file-menu.js): the file-download + // helper (defined below). The library title (name + dirty dot) repaints via a + // libraryName/libraryDirty effect, so callers just mutate those signals. app.downloadFile = downloadFile; - app.updateLibraryTitle = () => renderLibraryTitle(app); // --- identity ---------------------------------------------------------- // The host queries actually go to. chCtx.origin already resolves to the basic @@ -771,8 +770,7 @@ export function createApp(env = {}) { app.updateSaveBtn(); app.actions.rerenderTabs(); renderSavedHistory(app); - app.updateLibraryTitle(); - flashToast('Saved', { document: doc }); + flashToast('Saved', { document: doc }); // saveQuery dirtied the library → title effect adds the dot }; input.addEventListener('keydown', (e) => { if (e.key === 'Enter') { e.preventDefault(); commit(); } }); // In the multiline description, plain Enter inserts a newline; ⌘/Ctrl+Enter commits. @@ -974,6 +972,14 @@ export function renderApp(app, helpers) { app.state.sidePanel.value; renderSavedHistory(app); }); + // Reactive repaint of the header library title (name + unsaved-changes dot): + // re-runs when the name or dirty flag changes. The edit-mode toggle is driven + // separately (editingLibrary is not a signal — file-menu.js renders it directly). + effect(() => { + app.state.libraryName.value; + app.state.libraryDirty.value; + renderLibraryTitle(app); + }); app.loadVersion(); app.loadSchema(); app.loadReference(); diff --git a/src/ui/file-menu.js b/src/ui/file-menu.js index e946e45..008839e 100644 --- a/src/ui/file-menu.js +++ b/src/ui/file-menu.js @@ -36,16 +36,18 @@ export function renderLibraryTitle(app) { const state = app.state; slot.replaceChildren(); if (app.editingLibrary) { - const input = h('input', { class: 'lib-name-input', value: state.libraryName }); + const input = h('input', { class: 'lib-name-input', value: state.libraryName.value }); let done = false; // Enter/blur commit; Escape cancels. The guard stops the blur fired by the // re-render teardown from undoing a cancel (same pattern as saved rename). const finish = (commit) => { if (done) return; done = true; - if (commit && input.value.trim()) renameLibrary(state, input.value, app.saveStr); + // Leave edit mode first, so the renameLibrary write below repaints the + // button view via the title effect rather than a transient input. app.editingLibrary = false; - renderLibraryTitle(app); + if (commit && input.value.trim()) renameLibrary(state, input.value, app.saveStr); + renderLibraryTitle(app); // explicit: the cancel/no-op path changes no signal }; input.addEventListener('keydown', (e) => { if (e.key === 'Enter') { e.preventDefault(); finish(true); } @@ -59,8 +61,8 @@ export function renderLibraryTitle(app) { slot.appendChild(h('button', { class: 'lib-name', title: 'Rename library', onclick: () => { app.editingLibrary = true; renderLibraryTitle(app); }, - }, h('span', { class: 'lib-name-text' }, state.libraryName), - state.libraryDirty ? h('span', { class: 'lib-dirty', title: 'Unsaved changes since last save / load' }) : null)); + }, h('span', { class: 'lib-name-text' }, state.libraryName.value), + state.libraryDirty.value ? h('span', { class: 'lib-dirty', title: 'Unsaved changes since last save / load' }) : null)); } /** Open the File dropdown anchored under the File button (Esc / outside-click close). */ @@ -138,18 +140,17 @@ function readJsonFile(app, file, cb) { function saveJsonAction(app) { const qs = app.state.savedQueries; if (!qs.length) { flashToast('Nothing to save', { document: app.document }); return; } - app.downloadFile(fileBase(app.state.libraryName) + '.json', 'application/json', + app.downloadFile(fileBase(app.state.libraryName.value) + '.json', 'application/json', JSON.stringify(buildExportDoc(qs, new Date().toISOString()), null, 2)); - markLibrarySaved(app.state); - renderLibraryTitle(app); + markLibrarySaved(app.state); // clears libraryDirty → title effect drops the dot flashToast('Saved ' + queries(qs.length) + ' → .json', { document: app.document }); } function downloadAction(app, fmt) { const qs = app.state.savedQueries; if (!qs.length) { flashToast('Nothing to save', { document: app.document }); return; } - if (fmt === 'md') app.downloadFile(fileBase(app.state.libraryName) + '.md', 'text/markdown', buildMarkdownDoc(qs)); - else app.downloadFile(fileBase(app.state.libraryName) + '.sql', 'application/sql', buildSqlDoc(qs)); + if (fmt === 'md') app.downloadFile(fileBase(app.state.libraryName.value) + '.md', 'text/markdown', buildMarkdownDoc(qs)); + else app.downloadFile(fileBase(app.state.libraryName.value) + '.sql', 'application/sql', buildSqlDoc(qs)); flashToast('Saved ' + queries(qs.length) + ' → .' + fmt, { document: app.document }); } @@ -188,11 +189,11 @@ function doNew(app) { } /** Re-sync the surfaces a library change touches: Save button (tab links may be - * pruned), the saved list (count + rows), and the title (name + dirty dot). */ + * pruned) and the saved list (count + rows). The title (name + dirty dot) + * repaints itself via the libraryName/libraryDirty effect in createApp. */ function afterLibraryChange(app) { app.updateSaveBtn(); renderSavedHistory(app); - renderLibraryTitle(app); } // ── confirm dialogs (reuse the modal-backdrop/card visual language) ────────── diff --git a/src/ui/saved-history.js b/src/ui/saved-history.js index 32d6d39..eb3512a 100644 --- a/src/ui/saved-history.js +++ b/src/ui/saved-history.js @@ -106,7 +106,7 @@ function renderSaved(app, list) { if (app.editingSavedId === q.id) { list.appendChild(savedEditForm(app, q)); continue; } const star = h('button', { class: 'sv-star' + (q.favorite ? ' on' : ''), title: q.favorite ? 'Unfavorite' : 'Favorite', - onclick: (e) => { e.stopPropagation(); toggleFavorite(state, q.id, app.saveJSON); renderSavedHistory(app); app.updateLibraryTitle(); }, + onclick: (e) => { e.stopPropagation(); toggleFavorite(state, q.id, app.saveJSON); renderSavedHistory(app); }, }, Icon.star(q.favorite)); const row = h('div', { class: 'saved-row', ...dragProps(q.sql), onclick: () => { app.actions.loadIntoNewTab(q.name, q.sql, q.id, q.chart); app.actions.run({ view: q.view }); } }, @@ -119,7 +119,7 @@ function renderSaved(app, list) { }, Icon.pencil()), h('button', { class: 'sv-act', title: 'Delete', - onclick: (e) => { e.stopPropagation(); deleteSaved(state, q.id, app.saveJSON); app.updateSaveBtn(); renderSavedHistory(app); app.updateLibraryTitle(); }, + onclick: (e) => { e.stopPropagation(); deleteSaved(state, q.id, app.saveJSON); app.updateSaveBtn(); renderSavedHistory(app); }, }, Icon.trash())), q.description ? h('div', { class: 'desc' }, q.description) : null, h('div', { class: 'preview' }, q.sql.split('\n')[0])); @@ -149,7 +149,6 @@ function savedEditForm(app, q) { } app.editingSavedId = null; renderSavedHistory(app); - app.updateLibraryTitle(); }; nameInput.addEventListener('keydown', (e) => { if (e.key === 'Enter') { e.preventDefault(); finish(true); } diff --git a/tests/helpers/fake-app.js b/tests/helpers/fake-app.js index f0c3d2c..abcde45 100644 --- a/tests/helpers/fake-app.js +++ b/tests/helpers/fake-app.js @@ -40,7 +40,6 @@ export function makeApp(over = {}) { saveStr: vi.fn(), downloadFile: vi.fn(), updateSaveBtn: vi.fn(), - updateLibraryTitle: vi.fn(), elapsedMs: () => 0, editingSavedId: null, showLogin: vi.fn(), diff --git a/tests/unit/file-menu.test.js b/tests/unit/file-menu.test.js index feab2cb..a7ec051 100644 --- a/tests/unit/file-menu.test.js +++ b/tests/unit/file-menu.test.js @@ -26,8 +26,8 @@ afterEach(() => document.body.replaceChildren()); describe('library title', () => { it('renders the name + dirty dot; inline rename commits on Enter and persists', () => { const app = mount(); - app.state.libraryName = 'My queries'; - app.state.libraryDirty = true; + app.state.libraryName.value = 'My queries'; + app.state.libraryDirty.value = true; renderLibraryTitle(app); expect(app.dom.libraryTitle.querySelector('.lib-name-text').textContent).toBe('My queries'); expect(app.dom.libraryTitle.querySelector('.lib-dirty')).not.toBeNull(); @@ -37,38 +37,38 @@ describe('library title', () => { expect(input.value).toBe('My queries'); input.value = 'Renamed'; key(input, 'Enter'); - expect(app.state.libraryName).toBe('Renamed'); + expect(app.state.libraryName.value).toBe('Renamed'); expect(app.editingLibrary).toBe(false); expect(app.saveStr).toHaveBeenCalled(); - app.state.libraryDirty = false; + app.state.libraryDirty.value = false; renderLibraryTitle(app); expect(app.dom.libraryTitle.querySelector('.lib-dirty')).toBeNull(); }); it('inline rename: Escape cancels, blur commits, empty commit is a no-op, double-fire guarded', () => { const app = mount(); - app.state.libraryName = 'Orig'; + app.state.libraryName.value = 'Orig'; renderLibraryTitle(app); // Escape cancels click(app.dom.libraryTitle.querySelector('.lib-name')); let input = app.dom.libraryTitle.querySelector('.lib-name-input'); input.value = 'X'; key(input, 'Escape'); - expect(app.state.libraryName).toBe('Orig'); + expect(app.state.libraryName.value).toBe('Orig'); // empty name commit → no rename click(app.dom.libraryTitle.querySelector('.lib-name')); input = app.dom.libraryTitle.querySelector('.lib-name-input'); input.value = ' '; key(input, 'Enter'); - expect(app.state.libraryName).toBe('Orig'); + expect(app.state.libraryName.value).toBe('Orig'); // blur commits, then a second event on the detached input is guarded click(app.dom.libraryTitle.querySelector('.lib-name')); input = app.dom.libraryTitle.querySelector('.lib-name-input'); input.value = 'Blurred'; input.dispatchEvent(new Event('blur')); - expect(app.state.libraryName).toBe('Blurred'); + expect(app.state.libraryName.value).toBe('Blurred'); key(input, 'Enter'); - expect(app.state.libraryName).toBe('Blurred'); + expect(app.state.libraryName.value).toBe('Blurred'); }); it('renderLibraryTitle no-ops without a slot', () => { @@ -120,15 +120,15 @@ describe('Save JSON / Markdown / SQL downloads', () => { expect(app.downloadFile).not.toHaveBeenCalled(); expect(toast()).toBe('Nothing to save'); app.state.savedQueries = [{ id: 's1', name: 'A', sql: '1', favorite: true }]; - app.state.libraryName = 'My Lib'; - app.state.libraryDirty = true; + app.state.libraryName.value = 'My Lib'; + app.state.libraryDirty.value = true; openFileMenu(app); click(item(/Save JSON/)); const [fname, mime, content] = app.downloadFile.mock.calls[0]; expect(fname).toBe('My Lib.json'); expect(mime).toBe('application/json'); expect(JSON.parse(content).format).toBe('altinity-sql-browser/saved-queries'); - expect(app.state.libraryDirty).toBe(false); + expect(app.state.libraryDirty.value).toBe(false); expect(toast()).toBe('Saved 1 query → .json'); }); @@ -139,7 +139,7 @@ describe('Save JSON / Markdown / SQL downloads', () => { expect(app.downloadFile).not.toHaveBeenCalled(); expect(toast()).toBe('Nothing to save'); app.state.savedQueries = [{ id: 's1', name: 'A', sql: 'SELECT 1', favorite: false, description: 'd' }]; - app.state.libraryName = 'Lib'; + app.state.libraryName.value = 'Lib'; openFileMenu(app); click(item(/Download Markdown/)); expect(app.downloadFile.mock.calls.at(-1).slice(0, 2)).toEqual(['Lib.md', 'text/markdown']); @@ -147,11 +147,11 @@ describe('Save JSON / Markdown / SQL downloads', () => { click(item(/Download SQL/)); expect(app.downloadFile.mock.calls.at(-1).slice(0, 2)).toEqual(['Lib.sql', 'application/sql']); // an unnamed / whitespace-only library name falls back to "queries" - app.state.libraryName = ''; + app.state.libraryName.value = ''; openFileMenu(app); click(item(/Download Markdown/)); expect(app.downloadFile.mock.calls.at(-1)[0]).toBe('queries.md'); - app.state.libraryName = ' '; + app.state.libraryName.value = ' '; openFileMenu(app); click(item(/Download SQL/)); expect(app.downloadFile.mock.calls.at(-1)[0]).toBe('queries.sql'); @@ -180,7 +180,7 @@ describe('Open / Append (JSON only)', () => { expect(dialog.textContent).toContain('current 2 saved queries'); click(document.querySelector('.fm-dialog-confirm')); expect(app.state.savedQueries.map((q) => q.name)).toEqual(['New', 'New2']); - expect(app.state.libraryName).toBe('team'); + expect(app.state.libraryName.value).toBe('team'); expect(app.updateSaveBtn).toHaveBeenCalled(); expect(toast()).toBe('Opened library · 2 queries'); }); @@ -198,7 +198,7 @@ describe('Open / Append (JSON only)', () => { input.dispatchEvent(new Event('change', { bubbles: true })); expect(document.querySelector('.fm-dialog-card')).toBeNull(); expect(app.state.savedQueries.map((q) => q.name)).toEqual(['New']); - expect(app.state.libraryName).toBe('lib'); + expect(app.state.libraryName.value).toBe('lib'); }); it('Append item closes the menu, merges the file, and toasts counts', () => { @@ -251,13 +251,13 @@ describe('New Library + confirm dialogs', () => { expect(document.querySelector('.fm-dialog-backdrop')).toBeNull(); expect(toast()).toBe('Started a new library'); app.state.savedQueries = [{ id: 's1', name: 'A', sql: '1', favorite: false }]; - app.state.libraryName = 'Old'; + app.state.libraryName.value = 'Old'; openFileMenu(app); click(item(/New Library/)); expect(document.querySelector('.fm-dialog-card').textContent).toContain('Start a new library?'); click(document.querySelector('.fm-dialog-confirm')); expect(app.state.savedQueries).toEqual([]); - expect(app.state.libraryName).toBe('SQL Library'); + expect(app.state.libraryName.value).toBe('SQL Library'); }); it('confirm dialog: Cancel, backdrop click, and Escape all dismiss; a card click does not', () => { diff --git a/tests/unit/state.test.js b/tests/unit/state.test.js index f727f43..d25d51c 100644 --- a/tests/unit/state.test.js +++ b/tests/unit/state.test.js @@ -34,8 +34,8 @@ describe('createState', () => { expect(s.tabs.value).toHaveLength(1); expect(s.savedQueries).toEqual([]); expect(s.expandedTables).toBeInstanceOf(Set); - expect(s.libraryName).toBe(DEFAULT_LIBRARY_NAME); - expect(s.libraryDirty).toBe(false); + expect(s.libraryName.value).toBe(DEFAULT_LIBRARY_NAME); + expect(s.libraryDirty.value).toBe(false); }); it('reads + clamps persisted prefs', () => { const s = createState(reader({ @@ -49,7 +49,7 @@ describe('createState', () => { [KEYS.libraryName]: 'My team queries', })); expect(s.theme).toBe('light'); - expect(s.libraryName).toBe('My team queries'); + expect(s.libraryName.value).toBe('My team queries'); expect(s.sidebarPx).toBe(420); expect(s.editorPct).toBe(15); expect(s.sideSplitPct).toBe(85); @@ -274,46 +274,46 @@ describe('library document', () => { it('dirty flag: saved-query mutations set it; markLibrarySaved clears it', () => { const s = createState(reader()); const tab = s.tabs.value[0]; tab.sql = 'SELECT 1'; - expect(s.libraryDirty).toBe(false); + expect(s.libraryDirty.value).toBe(false); saveQuery(s, tab, 'Q', '', vi.fn()); - expect(s.libraryDirty).toBe(true); + expect(s.libraryDirty.value).toBe(true); markLibrarySaved(s); - expect(s.libraryDirty).toBe(false); + expect(s.libraryDirty.value).toBe(false); toggleFavorite(s, tab.savedId, vi.fn()); // favorite the just-saved entry - expect(s.libraryDirty).toBe(true); + expect(s.libraryDirty.value).toBe(true); markLibrarySaved(s); renameSaved(s, tab.savedId, 'Q2', undefined, vi.fn()); - expect(s.libraryDirty).toBe(true); + expect(s.libraryDirty.value).toBe(true); markLibrarySaved(s); deleteSaved(s, tab.savedId, vi.fn()); - expect(s.libraryDirty).toBe(true); + expect(s.libraryDirty.value).toBe(true); markLibrarySaved(s); importSaved(s, [{ name: 'I', sql: 'i' }], vi.fn(), () => 'gi'); - expect(s.libraryDirty).toBe(true); + expect(s.libraryDirty.value).toBe(true); }); it('renameLibrary trims + persists + marks dirty; blank falls back to the default', () => { const s = createState(reader()); const saveName = vi.fn(); renameLibrary(s, ' My queries ', saveName); - expect(s.libraryName).toBe('My queries'); - expect(s.libraryDirty).toBe(true); + expect(s.libraryName.value).toBe('My queries'); + expect(s.libraryDirty.value).toBe(true); expect(saveName).toHaveBeenCalledWith(KEYS.libraryName, 'My queries'); renameLibrary(s, ' ', saveName); - expect(s.libraryName).toBe(DEFAULT_LIBRARY_NAME); + expect(s.libraryName.value).toBe(DEFAULT_LIBRARY_NAME); }); it('newLibrary clears queries + name, clears dirty, prunes dangling tab links', () => { const s = createState(reader()); s.savedQueries = [{ id: 's1', name: 'A', sql: '1', favorite: false }]; - s.libraryName = 'Old'; s.libraryDirty = true; + s.libraryName.value = 'Old'; s.libraryDirty.value = true; s.tabs.value[0].savedId = 's1'; // dangling after clear → pruned s.tabs.value.push(newTabObj('t2')); // no savedId → skipped by prune const save = vi.fn(), saveName = vi.fn(); newLibrary(s, save, saveName); expect(s.savedQueries).toEqual([]); - expect(s.libraryName).toBe(DEFAULT_LIBRARY_NAME); - expect(s.libraryDirty).toBe(false); + expect(s.libraryName.value).toBe(DEFAULT_LIBRARY_NAME); + expect(s.libraryDirty.value).toBe(false); expect(s.tabs.value[0].savedId).toBeNull(); expect(save).toHaveBeenCalledWith(KEYS.saved, []); expect(saveName).toHaveBeenCalledWith(KEYS.libraryName, DEFAULT_LIBRARY_NAME); @@ -323,7 +323,7 @@ describe('library document', () => { const s = createState(reader()); s.savedQueries = [{ id: 'old', name: 'X', sql: 'x', favorite: false }]; s.tabs.value[0].savedId = 'old'; // becomes dangling → pruned - s.libraryDirty = true; + s.libraryDirty.value = true; const chart = { cfg: { type: 'bar' }, key: 'k' }; const incoming = [ { id: 'keep', name: 'A', sql: '1', favorite: true, description: 'd', chart, view: 'json' }, @@ -334,8 +334,8 @@ describe('library document', () => { replaceLibrary(s, incoming, 'My Library.json', save, saveName, () => 'g' + (++n)); expect(s.savedQueries.map((q) => q.id)).toEqual(['keep', 'g1']); expect(s.savedQueries[0]).toMatchObject({ name: 'A', sql: '1', favorite: true, description: 'd', chart, view: 'json' }); - expect(s.libraryName).toBe('My Library'); // extension stripped - expect(s.libraryDirty).toBe(false); + expect(s.libraryName.value).toBe('My Library'); // extension stripped + expect(s.libraryDirty.value).toBe(false); expect(s.tabs.value[0].savedId).toBeNull(); expect(save).toHaveBeenCalledWith(KEYS.saved, s.savedQueries); expect(saveName).toHaveBeenCalledWith(KEYS.libraryName, 'My Library'); @@ -363,7 +363,7 @@ describe('library document', () => { it('replaceLibrary with no usable file name falls back to the default', () => { const s = createState(reader()); replaceLibrary(s, [{ name: 'A', sql: '1' }], '.json', vi.fn(), vi.fn(), () => 'g'); - expect(s.libraryName).toBe(DEFAULT_LIBRARY_NAME); + expect(s.libraryName.value).toBe(DEFAULT_LIBRARY_NAME); }); it('appendLibrary merges via importSaved (dedupe), returns counts, sets dirty', () => { @@ -375,7 +375,7 @@ describe('library document', () => { ], vi.fn(), () => 'gb'); expect(r).toEqual({ added: 1, updated: 0, skipped: 1 }); expect(s.savedQueries.map((q) => q.name)).toEqual(['A', 'B']); - expect(s.libraryDirty).toBe(true); + expect(s.libraryDirty.value).toBe(true); }); it('library ops default their persistence seams (real storage helpers)', () => { From e9af0e4477eb79585e345df9e7f2bc19fe523d27 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 07:24:39 +0200 Subject: [PATCH 08/14] =?UTF-8?q?docs:=20record=20signals-core=20adoption?= =?UTF-8?q?=20=E2=80=94=20CLAUDE.md=20rule=204=20(three=20deps)=20+=20ADR-?= =?UTF-8?q?0001=20Accepted?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01FLVYgQpkbEbHKwEAA4aawZ --- CLAUDE.md | 8 +++++--- docs/ADR-0001-reactivity.md | 29 ++++++++++++++++++----------- 2 files changed, 23 insertions(+), 14 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 281cd08..378b151 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -22,9 +22,11 @@ both bundled — see hard rule 4). Quality is held by tests. (see README "Configuring OAuth"). 4. **The build is esbuild only; runtime deps are rare and deliberate.** Source files are the tested files; esbuild bundles `src/main.js` → `dist/sql.html`. - There are **two** bundled runtime dependencies — **Chart.js** (the Chart - result view) and **@dagrejs/dagre** (the EXPLAIN pipeline-graph layout) — both - inlined into the artifact, so the page still makes zero third-party requests. + There are **three** bundled runtime dependencies — **Chart.js** (the Chart + result view), **@dagrejs/dagre** (the EXPLAIN pipeline-graph layout), and + **@preact/signals-core** (the reactivity primitive — see + `docs/ADR-0001-reactivity.md`) — all inlined into the artifact, so the page + still makes zero third-party requests. Adding *another* runtime dependency is a deliberate decision (it grows the single served file) — don't do it casually. When a feature needs a library, keep the testable logic pure in `src/core/` (chart axis/role/pivot math in diff --git a/docs/ADR-0001-reactivity.md b/docs/ADR-0001-reactivity.md index a4a0e7f..6d12ec3 100644 --- a/docs/ADR-0001-reactivity.md +++ b/docs/ADR-0001-reactivity.md @@ -1,10 +1,11 @@ # ADR-0001: Reactivity — incremental signals, not a framework -- **Status:** Proposed -- **Date:** 2026-06-29 -- **Context tracking:** roadmap #68 +- **Status:** Accepted — adopted on `spike/signals-core` (#88) +- **Date:** 2026-06-29 (adopted 2026-06-30) +- **Context tracking:** roadmap #68; adoption #88 - **Evidence:** `spike/signals` (hand-rolled primitive + two converted slices), - `spike/signals-core` (same two slices on `@preact/signals-core`) + `spike/signals-core` (the chosen primitive; tabs/sidePanel, then the + `resultView`+`running` and `libraryName`+`libraryDirty` scalar slices) ## Context @@ -25,8 +26,11 @@ per-file 100/100/100/100 coverage gate (rule 1), and the injected-seam layering ## Decision Adopt **`@preact/signals-core`** (`signal()` / `effect()` / `computed()` / -`batch()` / `untracked()`) and migrate state **one slice at a time**, behind -accessor helpers. **Do not** adopt a UI framework (React/Preact/Solid). +`batch()` / `untracked()`) and migrate state **one slice at a time**, reading +and writing through `.value`. An accessor helper (like `activeTab()`) is +*optional* — used where it usefully contains reader churn, skipped where the +churn is trivially mechanical (see the Consequences guideline). **Do not** adopt +a UI framework (React/Preact/Solid). A hand-rolled ~70-line primitive was prototyped first and works (`spike/signals`), but `@preact/signals-core` was chosen over it: same `.value` API (so the @@ -80,10 +84,13 @@ choice is reversible. - **Migration is monotonic**: a slice keeps some direct render calls until its co-dependent slices are also signals. No slice un-converts another. - Per-slice cost is mostly mechanical `.value` edits in that slice's tests. -- **Rule:** give each converted slice an accessor helper (like `activeTab()`) to - contain reader churn and localize behavior. -- Adding `@preact/signals-core` makes **three** bundled runtime deps. On adoption, - update CLAUDE.md rule 4 ("two bundled runtime dependencies") and the count in - THIRD-PARTY-NOTICES.md (done) / `build/build.mjs` comments (done). +- **Guideline (not a rule):** add an accessor helper (like `activeTab()`) for a + slice with many scattered readers to contain churn; slices with few or + localized readers convert fine with bare `.value`. Validated by the + `sidePanel` slice (no helper — "worst case") and the `resultView`/`running` + and `libraryName`/`libraryDirty` scalar slices, all helper-free. +- Adding `@preact/signals-core` makes **three** bundled runtime deps. On adoption + the dependency count was updated in CLAUDE.md rule 4, THIRD-PARTY-NOTICES.md, + and `build/build.mjs` comments (all done). - Re-evaluate Preact/Solid only if the UI later grows many interdependent components with rich local state, or a genuine large-list render need appears. From b1e368cf518e914c6f3ac23680156d5ca80db4af Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 12:21:57 +0200 Subject: [PATCH 09/14] =?UTF-8?q?docs:=20CLAUDE.md=20rule=205=20=E2=80=94?= =?UTF-8?q?=20no=20UI=20framework;=20signals=20+=20imperative-seam=20adapt?= =?UTF-8?q?ers?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01FLVYgQpkbEbHKwEAA4aawZ --- CLAUDE.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/CLAUDE.md b/CLAUDE.md index 378b151..6cf98af 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -34,6 +34,20 @@ both bundled — see hard rule 4). Quality is held by tests. 100%-covered) and make the library call an **injected seam** (`app.Chart` / `app.Dagre`, like the fetch/crypto seams) so the DOM wrapper stays fully tested rather than dropping below the coverage gate. +5. **No UI framework; signals for state, imperative adapters for islands.** State + reactivity is `@preact/signals-core` (`signal`/`effect`/`computed`/`batch`), + migrated slice-by-slice (ADR-0001). **No React/Preact/Solid** — a Preact spike + on the schema panel (`spike/preact-schema`, ADR-0001 addendum) confirmed a + component model removes the in-place-mutation pain but buys a second render + paradigm the roadmap doesn't justify. The hard, third-party, or + high-frequency-pointer surfaces (the editor, the EXPLAIN/schema graphs, + Chart.js, result-grid resize/sort) stay **imperative behind an injected seam** — + signals coordinate state, they don't own every mousemove. CodeMirror 6 is the + pre-approved next runtime dep, behind an `EditorPort` seam, to land when + schema-aware autocomplete (#84) does (#21). When a *second* consumer of a + complex UI pattern appears, extract a shared primitive (e.g. `EditorPort`, + `GraphSurface`, a result-view registry, `Drawer`) rather than copy it — but + don't build a primitive speculatively for a single caller. ## How to add a result view / panel / feature From 8d59926fe2ea21ba83f79170253ceebd00f9ecd4 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 07:43:54 +0200 Subject: [PATCH 10/14] docs(adr): Preact schema-panel spike findings + recommendation (#88) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01FLVYgQpkbEbHKwEAA4aawZ --- docs/ADR-0001-reactivity.md | 58 +++++++++++++++++++++++++++++++++++++ 1 file changed, 58 insertions(+) diff --git a/docs/ADR-0001-reactivity.md b/docs/ADR-0001-reactivity.md index 6d12ec3..4c36bab 100644 --- a/docs/ADR-0001-reactivity.md +++ b/docs/ADR-0001-reactivity.md @@ -94,3 +94,61 @@ choice is reversible. and `build/build.mjs` comments (all done). - Re-evaluate Preact/Solid only if the UI later grows many interdependent components with rich local state, or a genuine large-list render need appears. + +## Addendum — Preact spike on the schema panel (#88, branch `spike/preact-schema`) + +The scalar slices (`resultView`/`running`, `libraryName`/`libraryDirty`) fit +signals-core cleanly. The **schema panel** does not: `state.schema` was a nested +array mutated *in place* (`db.expanded`, an `expandedTables` Set, `tb.columns` +flipping null→'loading'→array), and signals only react to reference changes — so +a signals-core conversion would just relocate the manual-invalidation pain into +`schema.value = [...]` bumps. That makes it the fair test of whether a +component+vDOM model earns adoption. We built it as Preact components +(`SchemaTree → DbRow → TableRow → ColumnRow`) to gather evidence — explicitly +re-opening this ADR's "no framework" line for *complex panels only*. + +**What the spike proved (the thesis holds):** +- The in-place-mutation anti-pattern is **gone**. Per-row expand state and lazy + columns are component-local `useState`; the `expandedTables` Set and + `db.expanded` were deleted; `loadColumns` became `fetchColumns` (returns the + columns for local state, still caches to `tb.columns` for autocomplete — a data + cache now decoupled from rendering). `state.schema`/`schemaError`/`schemaFilter` + are signals the tree reads via `@preact/signals` auto-tracking; no manual + `renderSchema()` calls remain. +- **The 100/100/100/100 per-file coverage gate is achievable** on the component + file (`src/ui/schema.js`) with tests ported to preact's `render` + `act()`. +- Signal→component auto-tracking works under happy-dom (load and filter repaint + the mounted tree). + +**What it cost (measured / observed):** +- **Bundle: +6.8 KB gzip** (148,044 → 154,873 B; raw 457,892 → 474,440) — close + to this ADR's +5.9 KB estimate. ~+4.6% of the artifact, still zero third-party + requests (inlined). +- **A second paradigm.** signals-core `effect()`s drive the rest of `createApp` + (tabs/results/title); the schema tree is Preact. Two render models coexist — + the main ongoing cost. +- **Integration seams with the imperative layer:** `Icon.*()` returns live SVG + DOM nodes that Preact can't embed, so a `ref`-mounter bridges them; preact's + `h` vs the project's hand-rolled `h`; `loadColumns` had to be reshaped. +- **Test friction:** scenarios staged by poking global state (`expandedTables`, + `tb.columns`) now require driving interactions; every interaction needs `act()`; + the double-click detector forced a fake-timer gap between open/collapse clicks. + Coverage still reaches 100%, but assertions shifted from state to DOM/behaviour. +- **LOC:** `src/ui/schema.js` 149 → 185 (+36), plus `preact` + `@preact/signals` + deps (would make **five** bundled runtime deps; CLAUDE.md rule 4 + + THIRD-PARTY-NOTICES would need updating *iff* adopted — not done on the spike). +- Used preact's `h()` (zero build/test config); JSX would improve ergonomics + further but needs esbuild + vitest jsx config — not required to evaluate the + component model. + +**Recommendation: do not adopt Preact now; stay signals-core.** The component +model genuinely removes the anti-pattern and is fully testable, but the costs +(a second render paradigm app-wide, +6.8 KB, the icon/`h`/columns integration +seams) are paid across the whole app to benefit **one** panel. That matches this +ADR's original reasoning — the need is invalidation, not a component model, and +there is no large-list requirement. Keep the schema panel as a **documented +imperative exception** (or, if its `tb.columns`/expand churn becomes a real +maintenance drag, convert it with signals-core using *replaced* Set/Map-valued +signals rather than in-place mutation). The `spike/preact-schema` branch stands +as the evidence; re-open the decision only when several complex, interdependent, +rich-local-state panels are actually on the roadmap (per the trigger above). From fe84e2c44f2eb056d49fb8a90589383fc197adc9 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 12:45:54 +0200 Subject: [PATCH 11/14] docs(CLAUDE.md): contributor working-discipline + reconcile runtime-dep count MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01FLVYgQpkbEbHKwEAA4aawZ --- CLAUDE.md | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 6cf98af..6a1b74d 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -1,8 +1,8 @@ # Contributor guide — altinity-sql-browser A modular ES-module SPA that builds to one self-contained HTML file served from -ClickHouse. No framework; runtime deps are rare and deliberate (currently two, -both bundled — see hard rule 4). Quality is held by tests. +ClickHouse. No framework; runtime deps are rare and deliberate (currently three, +all bundled — see hard rule 4). Quality is held by tests. ## Hard rules @@ -73,3 +73,20 @@ Touch these in one change: Pure-by-construction modules, injected side-effect seams, per-file coverage thresholds, and a single ClickHouse-served artifact built by esbuild. + +## Working discipline + +- **Surface out-of-scope findings, don't bury them.** Spot a real bug, data + inconsistency, deprecated API, or future footgun outside the current task → + open an issue labeled `inbox` (file:line + why deferred) and tell the user. + High signal only, not style nits. +- **Reconcile forward work after a substantive change.** A change to behavior, + schema, or a settled decision can stale tracked work. In the same commit, + reconcile what it reshaped: the roadmap meta-issue (currently #68) — re-check + or re-scope the track it touches; the affected issue's body (Goal/Acceptance); + the relevant ADR addendum and `CHANGELOG.md` `[Unreleased]`; and any issue it + obsoletes (close via "Closes #N" in the PR). Flag it if the rework is large. + (Trivial typo/comment changes exempt.) +- **Convert friction into memory.** If a task needed retried commits or hit an + unexpected failure (test/env/scope surprise), save a memory so the next + session doesn't repeat it. From add0d9d3bfef56a4c04681737821798f70b0a050 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 12:45:54 +0200 Subject: [PATCH 12/14] docs(changelog): note the signals-core reactivity adoption under [Unreleased] 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) Claude-Session: https://claude.ai/code/session_01FLVYgQpkbEbHKwEAA4aawZ --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8473579..1965989 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,15 @@ auto-generated per-PR notes; this file is the curated, human-readable history. ## [Unreleased] +### Changed +- State reactivity now uses `@preact/signals-core` (the third bundled runtime + dependency), adopted incrementally per + [ADR-0001](docs/ADR-0001-reactivity.md): the tab list, side panel, run state + (`running`/`resultView`), and the library title repaint via signal `effect`s + instead of manual render calls. No user-facing behavior change. A Preact + schema-panel spike was evaluated and **rejected** — the app stays + framework-free (ADR-0001 addendum). (#88) + ## [0.1.5] - 2026-06-29 ### Added From e402fa271ff242bf6e3f01333123f6185dd046d6 Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 12:46:47 +0200 Subject: [PATCH 13/14] chore(.github): PR reconcile-check + roadmap issue template 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) Claude-Session: https://claude.ai/code/session_01FLVYgQpkbEbHKwEAA4aawZ --- .github/ISSUE_TEMPLATE/roadmap.md | 23 +++++++++++++++++++++++ .github/PULL_REQUEST_TEMPLATE.md | 1 + 2 files changed, 24 insertions(+) create mode 100644 .github/ISSUE_TEMPLATE/roadmap.md diff --git a/.github/ISSUE_TEMPLATE/roadmap.md b/.github/ISSUE_TEMPLATE/roadmap.md new file mode 100644 index 0000000..30911b0 --- /dev/null +++ b/.github/ISSUE_TEMPLATE/roadmap.md @@ -0,0 +1,23 @@ +--- +name: Roadmap item +about: A planned phase / tracked feature with a spec (links to roadmap #68) +labels: enhancement +--- + +### Goal + + +### Files / scope + + +### Key implementation + + +### Acceptance criteria + + +### Re-evaluation trigger + + +### Tracking + diff --git a/.github/PULL_REQUEST_TEMPLATE.md b/.github/PULL_REQUEST_TEMPLATE.md index a50dbaf..627f113 100644 --- a/.github/PULL_REQUEST_TEMPLATE.md +++ b/.github/PULL_REQUEST_TEMPLATE.md @@ -8,3 +8,4 @@ - [ ] Layers kept honest: pure logic in `src/core/`, network in `src/net/` (injected fetch), DOM in `src/ui/` - [ ] No new runtime dependency (or it's a deliberate, justified addition — see CONTRIBUTING) - [ ] README / `CHANGELOG.md` (`[Unreleased]`) updated if behavior or the deployed surface changed +- [ ] Reconciled affected tracked work (roadmap #68, the issue body, ADR/CHANGELOG) if this change reshaped it From edf89a1c76e5a32ab77ff497a78cbc1be8647d7a Mon Sep 17 00:00:00 2001 From: Boris Tyshkevich Date: Tue, 30 Jun 2026 13:33:05 +0200 Subject: [PATCH 14/14] fix(e2e): import-map the bare @preact/signals-core specifier in the editor harness MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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) Claude-Session: https://claude.ai/code/session_01FLVYgQpkbEbHKwEAA4aawZ --- tests/e2e/editor.html | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/tests/e2e/editor.html b/tests/e2e/editor.html index 0e18ce9..01650e0 100644 --- a/tests/e2e/editor.html +++ b/tests/e2e/editor.html @@ -15,6 +15,12 @@
+ +