fix(app): keep terminal mounted when switching session tabs in a workspace
This commit is contained in:
parent
5fecf7ae9e
commit
d9e62f4eee
3 changed files with 150 additions and 23 deletions
|
|
@ -89,6 +89,7 @@ import { formatServerError, isSessionNotFoundError } from "@/utils/server-errors
|
|||
import { legacySessionHref, requireServerKey, sessionHref } from "@/utils/session-route"
|
||||
import { useUsageExceededDialogs } from "./session/usage-exceeded-dialogs"
|
||||
import { createSessionOwnership } from "./session/session-ownership"
|
||||
import { SessionRouteBoundary } from "./session/route-boundary"
|
||||
|
||||
type FollowupItem = FollowupDraft & { id: string }
|
||||
type FollowupEdit = Pick<FollowupItem, "id" | "prompt" | "context">
|
||||
|
|
@ -146,34 +147,38 @@ export function SessionPage() {
|
|||
export function TargetSessionRoute() {
|
||||
const params = useParams<{ serverKey: string; id: string }>()
|
||||
return (
|
||||
<Show when={`${params.serverKey}\0${params.id}`} keyed>
|
||||
<SessionRouteErrorBoundary sessionID={params.id} serverKey={requireServerKey(params.serverKey)} padded>
|
||||
<ResolvedTargetSessionRoute />
|
||||
</SessionRouteErrorBoundary>
|
||||
</Show>
|
||||
<SessionRouteBoundary
|
||||
serverKey={params.serverKey}
|
||||
sessionID={params.id}
|
||||
fallback={(error) => (
|
||||
<SessionRouteFallback
|
||||
error={error}
|
||||
sessionID={params.id}
|
||||
serverKey={requireServerKey(params.serverKey)}
|
||||
padded
|
||||
/>
|
||||
)}
|
||||
>
|
||||
<ResolvedTargetSessionRoute />
|
||||
</SessionRouteBoundary>
|
||||
)
|
||||
}
|
||||
|
||||
function SessionRouteErrorBoundary(
|
||||
props: ParentProps<{ sessionID?: string; serverKey?: ServerConnection.Key; padded?: boolean }>,
|
||||
) {
|
||||
function SessionRouteFallback(props: {
|
||||
error: unknown
|
||||
sessionID?: string
|
||||
serverKey?: ServerConnection.Key
|
||||
padded?: boolean
|
||||
}) {
|
||||
const settings = useSettings()
|
||||
return (
|
||||
<ErrorBoundary
|
||||
fallback={(error) =>
|
||||
settings.general.newLayoutDesigns() ? (
|
||||
<SessionRouteFrame padded={props.padded}>
|
||||
<SessionPanelFrame newLayout raised={!!props.sessionID}>
|
||||
<SessionErrorFallback error={error} sessionID={props.sessionID} serverKey={props.serverKey} />
|
||||
</SessionPanelFrame>
|
||||
</SessionRouteFrame>
|
||||
) : (
|
||||
<ErrorPage error={error} />
|
||||
)
|
||||
}
|
||||
>
|
||||
{props.children}
|
||||
</ErrorBoundary>
|
||||
<Show when={settings.general.newLayoutDesigns()} fallback={<ErrorPage error={props.error} />}>
|
||||
<SessionRouteFrame padded={props.padded}>
|
||||
<SessionPanelFrame newLayout raised={!!props.sessionID}>
|
||||
<SessionErrorFallback error={props.error} sessionID={props.sessionID} serverKey={props.serverKey} />
|
||||
</SessionPanelFrame>
|
||||
</SessionRouteFrame>
|
||||
</Show>
|
||||
)
|
||||
}
|
||||
|
||||
|
|
|
|||
37
packages/app/src/pages/session/route-boundary.ts
Normal file
37
packages/app/src/pages/session/route-boundary.ts
Normal file
|
|
@ -0,0 +1,37 @@
|
|||
import { ErrorBoundary, Show, createComponent, createEffect, on } from "solid-js"
|
||||
import type { JSX } from "solid-js"
|
||||
|
||||
// Structural boundary for the target session route: decides when the route
|
||||
// subtree remounts and how route-level errors are scoped. Kept free of app
|
||||
// contexts (and JSX) so the remount semantics can be tested directly.
|
||||
//
|
||||
// Keyed by server only. Workspace-scoped state (notably TerminalProvider and
|
||||
// its PTY WebSockets) lives inside the route subtree, so switching session
|
||||
// tabs within the same workspace must not remount it; session changes are
|
||||
// handled reactively below (TargetSessionPage re-keys per workspace).
|
||||
export function SessionRouteBoundary(props: {
|
||||
serverKey: string | undefined
|
||||
sessionID: string | undefined
|
||||
fallback: (error: unknown) => JSX.Element
|
||||
children: JSX.Element
|
||||
}) {
|
||||
return createComponent(Show, {
|
||||
get when() {
|
||||
return props.serverKey
|
||||
},
|
||||
keyed: true,
|
||||
get children() {
|
||||
return createComponent(ErrorBoundary, {
|
||||
fallback: (error: unknown, reset: () => void) => {
|
||||
// Without a per-session remount, a stale error (e.g. session not
|
||||
// found) must clear when navigating to a different session.
|
||||
createEffect(on(() => props.sessionID, reset, { defer: true }))
|
||||
return props.fallback(error)
|
||||
},
|
||||
get children() {
|
||||
return props.children
|
||||
},
|
||||
})
|
||||
},
|
||||
})
|
||||
}
|
||||
85
packages/app/test-browser/session-route-boundary.test.ts
Normal file
85
packages/app/test-browser/session-route-boundary.test.ts
Normal file
|
|
@ -0,0 +1,85 @@
|
|||
import { expect, test } from "bun:test"
|
||||
import { createComponent, createSignal, onCleanup } from "solid-js"
|
||||
import { render } from "solid-js/web"
|
||||
import { SessionRouteBoundary } from "@/pages/session/route-boundary"
|
||||
|
||||
// Terminals (and other workspace-scoped state) live inside the route subtree,
|
||||
// so switching session tabs within the same server must not remount it.
|
||||
test("switching sessions on the same server does not remount the route subtree", () => {
|
||||
const [server, setServer] = createSignal("srv_1")
|
||||
const [session, setSession] = createSignal("ses_a")
|
||||
let mounts = 0
|
||||
let disposals = 0
|
||||
const Probe = () => {
|
||||
mounts += 1
|
||||
onCleanup(() => {
|
||||
disposals += 1
|
||||
})
|
||||
return null
|
||||
}
|
||||
|
||||
const dispose = render(
|
||||
() =>
|
||||
createComponent(SessionRouteBoundary, {
|
||||
get serverKey() {
|
||||
return server()
|
||||
},
|
||||
get sessionID() {
|
||||
return session()
|
||||
},
|
||||
fallback: () => null,
|
||||
get children() {
|
||||
return createComponent(Probe, {})
|
||||
},
|
||||
}),
|
||||
document.createElement("div"),
|
||||
)
|
||||
|
||||
const initialMounts = mounts
|
||||
expect(initialMounts).toBeGreaterThan(0)
|
||||
|
||||
setSession("ses_b")
|
||||
expect(mounts).toBe(initialMounts)
|
||||
expect(disposals).toBe(0)
|
||||
|
||||
setServer("srv_2")
|
||||
expect(mounts).toBeGreaterThan(initialMounts)
|
||||
expect(disposals).toBeGreaterThan(0)
|
||||
|
||||
dispose()
|
||||
})
|
||||
|
||||
// Without a per-session remount, the error boundary must clear a stale error
|
||||
// (e.g. session not found) when navigating to a different session.
|
||||
test("route error clears when navigating to a different session", () => {
|
||||
const [session, setSession] = createSignal("ses_a")
|
||||
const [broken, setBroken] = createSignal(true)
|
||||
const Thrower = () => {
|
||||
if (broken()) throw new Error(`Session not found: ${session()}`)
|
||||
return "content"
|
||||
}
|
||||
|
||||
const container = document.createElement("div")
|
||||
const dispose = render(
|
||||
() =>
|
||||
createComponent(SessionRouteBoundary, {
|
||||
serverKey: "srv_1",
|
||||
get sessionID() {
|
||||
return session()
|
||||
},
|
||||
fallback: () => "error-fallback",
|
||||
get children() {
|
||||
return createComponent(Thrower, {})
|
||||
},
|
||||
}),
|
||||
container,
|
||||
)
|
||||
|
||||
expect(container.textContent).toBe("error-fallback")
|
||||
|
||||
setBroken(false)
|
||||
setSession("ses_b")
|
||||
expect(container.textContent).toBe("content")
|
||||
|
||||
dispose()
|
||||
})
|
||||
Loading…
Add table
Add a link
Reference in a new issue