refactor(schema): convert the schema slice to signals (finishes #88) (#91)#93
Merged
Merged
Conversation
Converts the schema panel's state to @preact/signals-core, the last slice of the #88 reactivity migration. `schema`/`schemaError`/`schemaFilter` become `signal(...)`; the in-place anti-pattern the ADR flagged is removed two ways: - per-row expand state moves from the mutated `db.expanded` bool + the `expandedTables` Set into one Set-valued `expanded` signal (keys `db:`/`tb:`), updated copy-on-write via a `toggleKey` helper; - lazy column loads replace the `schema` reference (`{...tb, columns}`) instead of mutating `tb.columns` in place. `tb.columns` stays the completion cache `buildCompletions` reads, so `completions.js` is untouched (lower churn than a separate Map-valued signal — the chosen "Approach C"). Two `effect()`s in `createApp` (schema tree + error banner) replace every scattered `renderSchema()` call; the expand+first-fetch is wrapped in `batch()` so the row opens with its spinner in a single repaint, and `loadSchema`'s schema+error writes are batched too. The panel stays imperative — no Preact (spike rejected, ADR-0001 addendum). No user-facing change. `loadColumns` drops its now-unused `tableObj` param (it locates the table by db/table name and replaces by reference). The vestigial `expanded:false` from ch-client's loadSchema is left as-is (out of this slice's scope). Tests swept to `.value` + the `expanded` Set keys across schema/app/state specs; `completions.test.js` untouched. Per-file 100% gate holds; build clean; e2e green on Chromium/Firefox/WebKit. Reconciles CHANGELOG [Unreleased] + the ADR addendum; completes the #88 migration. Closes #91. Part of #68 (Phase 1). Completes #88. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
13 tasks
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What & why
Final slice of the #88 reactivity migration (ADR-0001): convert the schema panel's state to
@preact/signals-core.schema/schemaError/schemaFilterbecomesignal(...), and the in-place mutation the ADR flagged as the "awkward slice" is removed — without adopting Preact (that spike was rejected; the panel stays a documented imperative exception, ADR-0001 addendum). No user-facing change.Approach (chosen "C" — reviewed by an independent Plan agent)
db.expanded(mutated bool) + theexpandedTablesSet → one Set-valuedexpandedsignal (keysdb:/tb:), updated copy-on-write via a localtoggleKeyhelper.loadColumnsnow replaces theschemareference ({...tb, columns}) instead of mutatingtb.columnsin place.tb.columnsstays the completion cachebuildCompletionsreads — socompletions.jsis untouched (lower churn than a separate Map-valued signal, and the core gate stays at 100%).loadColumnsdrops its now-unusedtableObjparam.effect()s increateApp(schema tree + error banner) replace every scatteredrenderSchema()call. The expand + first column-fetch is wrapped inbatch()so the row opens with its spinner in one repaint;loadSchema's schema+error writes are batched too.Notes
expanded:falseproduced bych-client.loadSchemais left as-is (out of this slice's listed scope; nothing readsdb.expandedanymore).dbKey/tbKeyreuse (single source of truth for the expand-vs-dblclick key); left the ch-client field per scope.Checklist
npm testpasses (per-file 100% gate; 1028 tests) —schema.js100/100/100/100schema/app/statespecs swept to.value+expandedSet keys;completions.test.jsuntouched)npm run buildsucceeds (single-filedist/sql.html)src/ui)CHANGELOG.md([Unreleased]) updatedCloses #91. Part of #68 (Phase 1). Completes #88 — the reactivity migration is done; #88 can be closed on merge.
🤖 Generated with Claude Code