Skip to content

Convert the schema slice to signals (schema/schemaError/schemaFilter) — Phase 1 of #88 #91

Description

@BorisTyshkevich

Part of #68 (Phase 1) — the final slice of the #88 reactivity migration. Links to roadmap #68; foundation in docs/ADR-0001-reactivity.md + CLAUDE.md rule 5.

Goal

Finish the #88 migration by converting the schema panel's state — schema, schemaError, schemaFilter — to @preact/signals-core signals, so the tree repaints via an effect() like every other slice. The schema panel stays imperative (no Preact — the spike was rejected, ADR-0001 addendum); the in-place mutation that made this the awkward slice is replaced with reference-replacing updates, not a component model.

Files / scope

  • src/state.jsschema / schemaError / schemaFiltersignal(...).
  • src/ui/app.jsloadSchema / loadColumns write via .value; an effect() in createApp reads the schema signals and calls renderSchema(app) (replacing the scattered manual renderSchema calls); rebuildCompletions / updateBanner read .value; the schema-filter input sets schemaFilter.value.
  • src/ui/schema.jsrenderSchema reads .value; expand toggles + lazy column load update by reference (see below), not in place.
  • Tests — sweep tests/unit/{schema,app,state}.test.js to .value (the unit pattern from the prior slices).
  • Non-goals: not a Preact rewrite (rejected — ADR-0001 addendum); no schema-tree feature changes; the e2e editor harness is unaffected (no dep added).

Key implementation

  • Mirror the established slice pattern (tabs / sidePanel / resultView / libraryName): field → signal, reads/writes via .value, one effect() in createApp replaces manual renderX invalidation, batch() for multi-signal updates.
  • Kill the in-place mutation (the reason this slice was awkward): replace the expandedTables Set with a Set-valued signal updated immutably (expanded.value = new Set(expanded.value).add(key)); replace tb.columns in-place writes either with a Map-valued signal (cols.value = new Map(cols.value).set(key, …)) or by keeping tb.columns as a completion cache and bumping the schema signal. Pick the lower-churn option and document it in the PR.
  • Keep renderSchema imperative (rebuild the tree on the effect) — the documented exception to "no components" (ADR-0001 / CLAUDE.md rule 5).

Acceptance criteria

  • schema / schemaError / schemaFilter are signal(...); all reads/writes go through .value.
  • No in-place mutation drives rendering — expand state + loaded columns update by reference; an effect() repaints the tree (no leftover manual renderSchema calls beyond any deliberate data-path one).
  • Schema load, load-error banner, lazy column load, filter, expand/collapse, double-click insert, shift-click SHOW CREATE, and drag all behave exactly as before — no user-facing change.
  • npm test green at the per-file 100% gate; npm run build clean; e2e green on all three engines.
  • CHANGELOG [Unreleased] + Roadmap to 1.0.0 #68 Phase 1 reconciled; with this slice done, Adopt reactivity via @preact/signals-core, incrementally (ADR-0001) #88 can close.

Re-evaluation trigger

If reference-replacing the tree proves as forgettable as the old manual renderSchema calls (the "relocated pain" the ADR warned about), reconsider whether the schema panel stays a documented imperative exception or warrants the (currently-rejected) component model — but only via a fresh ADR.

Tracking

Phase 1 of #68; the last slice of #88 (closes the migration). The rejected Preact alternative is preserved on spike/preact-schema as evidence.

Metadata

Metadata

Assignees

No one assigned

    Labels

    enhancementNew feature or request

    Type

    No type
    No fields configured for issues without a type.

    Projects

    No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions