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 diff --git a/CHANGELOG.md b/CHANGELOG.md index c23f59e..c1b6243 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,15 @@ auto-generated per-PR notes; this file is the curated, human-readable history. Chromium/Firefox/Safari are supported (Safari verified green on CI); the full browser/ClickHouse/IdP matrix is tracked in #71. (#69) +### 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 diff --git a/CLAUDE.md b/CLAUDE.md index 281cd08..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 @@ -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 @@ -32,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 @@ -57,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. 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 new file mode 100644 index 0000000..4c36bab --- /dev/null +++ b/docs/ADR-0001-reactivity.md @@ -0,0 +1,154 @@ +# ADR-0001: Reactivity — incremental signals, not a framework + +- **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` (the chosen primitive; tabs/sidePanel, then the + `resultView`+`running` and `libraryName`+`libraryDirty` scalar 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 **`@preact/signals-core`** (`signal()` / `effect()` / `computed()` / +`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 +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. + +## Options considered (all measured) + +| Option | artifact Δ (gzip) | Verdict | +|---|---|---| +| **@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 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 + 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 + 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. + +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 + +- 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. +- **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. + +## 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). 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/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..2e5f8d3 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 '@preact/signals-core'; /** A tab's chart state as a persistable payload `{ cfg, key }`, or null. */ export function tabChart(tab) { @@ -48,29 +49,37 @@ 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): 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, schemaError: null, 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. 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"). - // `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: '', @@ -80,7 +89,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 +99,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) { @@ -112,7 +121,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; @@ -129,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; } @@ -149,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); } @@ -158,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); } @@ -197,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 }; } @@ -206,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); } @@ -219,14 +228,14 @@ 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. */ 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 @@ -234,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. @@ -261,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 @@ -275,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 0f5bacf..fab5ccb 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, batch } from '@preact/signals-core'; import { renderSchema } from './schema.js'; import { renderResults } from './results.js'; import { openSchemaView } from './explain-graph.js'; @@ -101,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 @@ -398,7 +398,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(); @@ -441,17 +441,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, { @@ -474,19 +478,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); } @@ -524,8 +529,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); } @@ -639,7 +644,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 ------------------------------------------------------ @@ -765,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. @@ -938,11 +942,44 @@ 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. + effect(() => { + app.state.tabs.value; + app.state.activeTabId.value; + renderTabs(app); + if (app.dom.editorSync) app.dom.editorSync(); + 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); - renderSavedHistory(app); - app.updateSaveBtn(); + // 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); + }); + // 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/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/saved-history.js b/src/ui/saved-history.js index 76e6076..eb3512a 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()); @@ -104,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 }); } }, @@ -117,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])); @@ -147,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/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/src/ui/tabs.js b/src/ui/tabs.js index 9a1fc1a..d0e94ec 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 '@preact/signals-core'; /** 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/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 @@
+ +