-
Notifications
You must be signed in to change notification settings - Fork 5
fix: reduce settings SSE noise and restore typecheck #385
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,5 +1,5 @@ | ||
| import { Router, Route, Navigate, useParams } from "@solidjs/router" | ||
| import { createSignal, onMount, onCleanup, For } from "solid-js" | ||
| import { createSignal, onMount, onCleanup, For, type Accessor } from "solid-js" | ||
| import { BasePathProvider, useBasePath } from "./context/base-path" | ||
| import { ServerProvider, useServer } from "./context/server" | ||
| import { BrandingProvider } from "./context/branding" | ||
|
|
@@ -33,15 +33,15 @@ function getLastSessionHref(encodedDir: string): string { | |
|
|
||
| function DirectoryIndex() { | ||
| const params = useParams<{ dir: string }>() | ||
| return <Navigate href={getLastSessionHref(params.dir)} replace /> | ||
| return <Navigate href={getLastSessionHref(params.dir)} /> | ||
| } | ||
|
|
||
| function SessionIndex() { | ||
| 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} /> | ||
| } | ||
|
Comment on lines
40
to
45
|
||
|
|
||
| function AppRoutes() { | ||
|
|
@@ -71,16 +71,21 @@ function AppRoutes() { | |
| * Reads the active directory from window.location (outside Router context). | ||
| * Re-evaluates on popstate and on history.pushState/history.replaceState navigation. | ||
| */ | ||
| function useActiveDirectory() { | ||
| function useRouteState() { | ||
| const [dir, setDir] = createSignal<string | undefined>( | ||
| typeof window === "undefined" ? undefined : deriveDirectoryFromPathname(), | ||
| ) | ||
| const [pathname, setPathname] = createSignal( | ||
| typeof window === "undefined" ? "" : window.location.pathname, | ||
| ) | ||
|
|
||
| onMount(() => { | ||
| // Ensure correct value once mounted (covers SSR hydration) | ||
| setDir(deriveDirectoryFromPathname()) | ||
| function update() { | ||
| setDir(deriveDirectoryFromPathname()) | ||
| setPathname(window.location.pathname) | ||
| } | ||
|
|
||
| function update() { setDir(deriveDirectoryFromPathname()) } | ||
| update() | ||
|
|
||
| // Patch pushState/replaceState to detect SolidJS Router navigations | ||
| // instead of polling with setInterval | ||
|
|
@@ -97,7 +102,10 @@ function useActiveDirectory() { | |
| }) | ||
| }) | ||
|
|
||
| return dir | ||
| return { | ||
| activeDirectory: dir, | ||
| pathname, | ||
| } | ||
| } | ||
|
|
||
| function useProjectsList() { | ||
|
|
@@ -129,7 +137,11 @@ function useProjectsList() { | |
| return projects | ||
| } | ||
|
|
||
| function AppWithServer(props: { projects: () => Project[]; activeDirectory: () => string | undefined }) { | ||
| function AppWithServer(props: { | ||
| projects: () => Project[] | ||
| activeDirectory: Accessor<string | undefined> | ||
| pathname: Accessor<string> | ||
| }) { | ||
| const { serverUrl, activeServerKey } = useServer() | ||
|
|
||
| // Key by server config to force full remount when switching or editing servers | ||
|
|
@@ -141,7 +153,11 @@ function AppWithServer(props: { projects: () => Project[]; activeDirectory: () = | |
| <BrandingProvider> | ||
| <RecentProjectsProvider> | ||
| <SavedPromptsProvider directory={props.activeDirectory}> | ||
| <GlobalEventsProvider projects={props.projects} activeDirectory={props.activeDirectory}> | ||
| <GlobalEventsProvider | ||
| projects={props.projects} | ||
| activeDirectory={props.activeDirectory} | ||
| pathname={props.pathname} | ||
| > | ||
| <CommandProvider> | ||
| <AppRoutes /> | ||
| </CommandProvider> | ||
|
|
@@ -158,11 +174,15 @@ function AppWithServer(props: { projects: () => Project[]; activeDirectory: () = | |
|
|
||
| export function App() { | ||
| const projects = useProjectsList() | ||
| const activeDirectory = useActiveDirectory() | ||
| const route = useRouteState() | ||
|
|
||
| return ( | ||
| <ServerProvider> | ||
| <AppWithServer projects={projects} activeDirectory={activeDirectory} /> | ||
| <AppWithServer | ||
| projects={projects} | ||
| activeDirectory={route.activeDirectory} | ||
| pathname={route.pathname} | ||
| /> | ||
| </ServerProvider> | ||
| ) | ||
| } | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removing
replacefrom this redirect changes browser history behavior: navigating to/:dirwill 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 supportsreplaceon<Navigate>, consider implementing this redirect viauseNavigate()with{ replace: true }in an effect/onMount to preserve the previous semantics.