Skip to content

refactor: one definition of the live connection list #963

Closed

Description

@EVWorth

"Every connection that can run a statement right now" (connected servers plus open SQLite files) is built in four places. They agree today, but where a run goes and whether a tab counts as orphaned (#944) both depend on them agreeing, so a change to one that misses the others would bring back bugs of that kind.

Reproduce

grep -n "activeConnections.map" src/hooks/useQueryExecution.ts src/hooks/useResultsFollowActiveTab.ts src/components/editor/SQLEditor.tsx
src/hooks/useQueryExecution.ts:45:    ...activeConnections.map((c) => c.id),
src/hooks/useResultsFollowActiveTab.ts:29:    ...activeConnections.map((c) => c.id),
src/components/editor/SQLEditor.tsx:32:    ...activeConnections.map((c) => c.id),

The fourth is liveConnectionIds() in src/lib/run-connection.ts, shared by the stores and useConnectionSwitching since #961.

Cause

The three hook and component copies are built from store selectors so they re-render when connections change; liveConnectionIds() reads store state once and can't replace them directly.

Fix

Add a useLiveConnectionIds hook next to liveConnectionIds() that subscribes to both stores and returns the same list, and use it in the three files. No behaviour change; the existing tests for those hooks and SQLEditor should pass unchanged.

While there: src/components/layout/useStartupReconnect.ts:20 uses a ! non-null assertion (t.profileId!), which src/CLAUDE.md rules out. Narrow with a type guard in the filter instead.

Activity

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Metadata

Metadata

Assignees

No one assigned

    Labels

    No labels
    No labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions