Skip to content

refactor(server): drop dead snapshotAll; quote identifiers in trigger DDL#7

Closed
grrowl wants to merge 1 commit into
feat/ssrfrom
advisor/006-dead-code-identifier-quoting
Closed

refactor(server): drop dead snapshotAll; quote identifiers in trigger DDL#7
grrowl wants to merge 1 commit into
feat/ssrfrom
advisor/006-dead-code-identifier-quoting

Conversation

@grrowl

@grrowl grrowl commented Jun 11, 2026

Copy link
Copy Markdown
Owner

Executes plan 006 from the advisor audit. Smallest of the set: −12/+12 in src/server/changes.ts.

What

  • Delete snapshotAll — zero callers anywhere (the real snapshot path goes through compileSubsetQuery), and it was one of the two unquoted-identifier interpolation sites.
  • Double-quote every identifier position in the three CDC CREATE TRIGGER statements (trigger name, ON table, NEW./OLD. pk); the '${tbl}' value literal stays single-quoted. The registry IDENT regex remains the real gate — quoting is consistency + defense in depth, matching sql-compiler.ts and ensureTriggers' DROP.

hydrateRows deliberately untouched — owned by the plan-004 PR.

Verification

No new tests by design: cdc, compaction, schema-evolution (idempotency) and ensure-triggers (reaping set-diff vs sqlite_master) pin every failure mode; all green on the reviewer's independent re-run.

🤖 Generated with Claude Code

… DDL

`snapshotAll` had zero callers in src/ or tests/ and was never exported
from src/server/index.ts — deleting it removes an unquoted table-name
interpolation that modelled the wrong habit.

`installTriggers` now double-quotes every identifier position (trigger
name, table name, pk column) for consistency with sql-compiler.ts and
ensureTriggers' existing DROP. The regex gate (assertValidCollection)
remains the real safety net; quoting just makes the helper safe even if
a future caller bypasses the registry.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@grrowl

grrowl commented Jun 13, 2026

Copy link
Copy Markdown
Owner Author

Superseded by #13, which landed plan-006 on main (adapted for main + published @tanstack/db, validated green) as part of the 0.3.2 bug-fix batch.

Not merging into feat/ssr: feat/ssr will receive this via its rebase onto the new main, so merging here would duplicate/conflict with that. Closing as superseded.

The advisor/* branch is retained (not deleted) until the feat/ssr reconciliation is done — it's the source for restoring any feat/ssr-specific bits (e.g. the readSyncSnapshot test case, the extracted scheduleReconnect method) that main's adapted versions intentionally omit.

@grrowl grrowl closed this Jun 13, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant