Skip to content

fix: reduce settings SSE noise and restore typecheck#385

Open
geier wants to merge 1 commit intomainfrom
fix/settings-sse-typecheck
Open

fix: reduce settings SSE noise and restore typecheck#385
geier wants to merge 1 commit intomainfrom
fix/settings-sse-typecheck

Conversation

@geier
Copy link
Copy Markdown
Contributor

@geier geier commented Apr 20, 2026

Summary

  • pause background per-project SSE connections while viewing settings pages to avoid a large pile of pending /event requests
  • reuse the existing top-level route tracking instead of patching browser history a second time in GlobalEventsProvider
  • clean up TypeScript issues in routing, theme listeners, entry mounting, layout timer handling, and proxy SSE headers so bun run typecheck passes again

Testing

  • bun run typecheck

Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR reduces background SSE activity while users are on settings pages and addresses several TypeScript typing issues so bun run typecheck passes again.

Changes:

  • Suspend per-project background SSE connections when the current route is a settings page.
  • Centralize route tracking (active directory + pathname) at the app level and pass pathname down to GlobalEventsProvider.
  • Fix assorted TS issues in the proxy SSE headers, theme media query listeners, entry mounting, and timer cleanup.

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.

Show a summary per file
File Description
shared/proxy.ts Builds SSE + CORS response headers via Headers to satisfy types and keep SSE headers explicit.
app-prefixable/src/pages/layout.tsx Adjusts daily timer handling to avoid type issues around setTimeout return types.
app-prefixable/src/entry.tsx Minor refactor of the mount element usage for type/clarity consistency.
app-prefixable/src/context/theme.tsx Adds a typed legacy matchMedia listener path to satisfy TS and support older APIs.
app-prefixable/src/context/server.tsx Fixes a TS cast issue when treating server configs as generic records.
app-prefixable/src/context/global-events.tsx Disconnects/suspends background SSE connections on settings routes (and remote servers).
app-prefixable/src/app.tsx Introduces useRouteState() and threads pathname into GlobalEventsProvider; adjusts index redirects.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines 34 to 37
function DirectoryIndex() {
const params = useParams<{ dir: string }>()
return <Navigate href={getLastSessionHref(params.dir)} replace />
return <Navigate href={getLastSessionHref(params.dir)} />
}
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Removing replace from this redirect changes browser history behavior: navigating to /:dir will now add an extra history entry before redirecting to the session route, which can create a back-button loop (back -> /:dir -> redirect forward again). If the router no longer supports replace on <Navigate>, consider implementing this redirect via useNavigate() with { replace: true } in an effect/onMount to preserve the previous semantics.

Copilot uses AI. Check for mistakes.
Comment on lines 40 to 45
const params = useParams<{ dir: string }>()
const href = getLastSessionHref(params.dir)
if (href === "session") return <Session />
const id = href.replace(/^session\//, "")
return <Navigate href={id} replace />
return <Navigate href={id} />
}
Copy link

Copilot AI Apr 20, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as above: dropping replace here will push an extra history entry when redirecting from /session to the last session id, which can make back navigation bounce between the index route and the redirected route. If replace isn't available on <Navigate>, switch to an imperative navigate(href, { replace: true }) redirect to avoid polluting history.

Copilot uses AI. Check for mistakes.
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.

2 participants