fix: sidebar crash #31 — layout-push CSS-variable leak + per-tab error containment
Two failure modes of a sidebar render crash, fixed together: 1. Layout-push leak (root cause): the layout-push effect wrote --dsh-sidebar-width/--dsh-sidebar-height on document.documentElement without a cleanup, so ANY unmount of the Sidebar (error-boundary swap, plugin disable, HMR) left the variables behind and layout.css kept squeezing #root with a stale margin — "the sidebar cannot be hidden" until a full reload. The effect now removes both properties on cleanup (the CSS fallback 0px restores the layout). 2. Crash containment (UX): the root error boundary previously replaced the WHOLE sidebar — toggle cluster included — with an error strip on any tab crash. A new shared RenderBoundary (src/client/RenderBoundary.tsx) now scopes containment per tab: a crashing viewer/editor shows a dismissible strip inside its own pane while the toggle cluster, the other tabs, and the panel stay alive. The descriptor is rendered as a real element (createElement) so even top-level component throws land in the boundary, not in TabContent's own render. The root boundary remains as the last resort for Workbench/shell errors. Tests (tests/sidebar-crash.spec.tsx, jsdom + real Sidebar/store harness): - unmount clears both CSS variables (red before the fix: 307px leaked) - a crashing tab is contained: strip in-pane, collapse button alive, layout push still live - the retry button recovers a tab whose crash has since been fixed Full suite: 425 tests pass; tsc --noEmit clean.
This commit is contained in:
@@ -0,0 +1,49 @@
|
||||
/**
|
||||
* The generic render error boundary for the sidebar tree: a render error in
|
||||
* the wrapped subtree shows a dismissible error strip (retry re-renders the
|
||||
* children) instead of blanking the shell. Used at two scopes:
|
||||
*
|
||||
* - ROOT (index.tsx, `css.boundaryError`): last-resort containment for
|
||||
* errors in the sidebar shell itself (Workbench, drag layout, …) — a full
|
||||
* swap keeps the page alive.
|
||||
* - PER-TAB (Sidebar.tsx TabContent, `css.tabBoundaryError`): a crashing
|
||||
* viewer/editor shows a strip inside ITS OWN pane; the toggle cluster, the
|
||||
* other tabs, and the panel itself stay alive (issue #31 — a tab crash
|
||||
* must never take down the whole sidebar).
|
||||
*
|
||||
* The className prop selects the strip's geometry: the root's full-height
|
||||
* fixed rail vs. the tab's pane-filling block.
|
||||
*/
|
||||
import { Component, type ErrorInfo, type ReactNode } from 'react'
|
||||
import { t } from './locales.ts'
|
||||
import css from './sidebar.module.css'
|
||||
|
||||
export class RenderBoundary extends Component<{ children?: ReactNode; className?: string }, { error: string | null }> {
|
||||
state = { error: null as string | null }
|
||||
|
||||
static getDerivedStateFromError(error: unknown): { error: string } {
|
||||
return { error: error instanceof Error ? error.message : String(error) }
|
||||
}
|
||||
|
||||
componentDidCatch(error: Error, info: ErrorInfo): void {
|
||||
console.error('[dsh-better-sidebar] render error:', error, info.componentStack)
|
||||
}
|
||||
|
||||
render(): ReactNode {
|
||||
if (this.state.error !== null) {
|
||||
return (
|
||||
<div className={this.props.className}>
|
||||
<span>dsh-better-sidebar: {this.state.error}</span>
|
||||
<button
|
||||
type="button"
|
||||
className={css.terminalRetry}
|
||||
onClick={() => { this.setState({ error: null }) }}
|
||||
>
|
||||
{t('terminalRetry')}
|
||||
</button>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
return this.props.children
|
||||
}
|
||||
}
|
||||
+29
-5
@@ -25,7 +25,7 @@
|
||||
* drawer floats). Widening does not migrate back: the tabs keep living in
|
||||
* the right tree.
|
||||
*/
|
||||
import { useCallback, useEffect, useMemo, useRef, useState, type ReactNode } from 'react'
|
||||
import { createElement, useCallback, useEffect, useMemo, useRef, useState, type ReactNode } from 'react'
|
||||
import { useSyncExternalStore } from 'react'
|
||||
import clsx from 'clsx'
|
||||
import { IconCloseFill14, Tooltip } from '@deepseek-ai/dsh-client-ui-primitives'
|
||||
@@ -44,6 +44,7 @@ import type { NewTabOption } from './TabBar.tsx'
|
||||
import type { TabDragPayload } from './TabBar.tsx'
|
||||
import { relativeTo } from './paths.ts'
|
||||
import { OrphanedTab } from './OrphanedTab.tsx'
|
||||
import { RenderBoundary } from './RenderBoundary.tsx'
|
||||
import { detectNewDirectSubagent } from './subagent-detect.ts'
|
||||
import { detectNewJob } from './subagent-jobs.ts'
|
||||
import { t } from './locales.ts'
|
||||
@@ -77,10 +78,22 @@ function TabContent(props: {
|
||||
if (descriptor === undefined) {
|
||||
return <OrphanedTab ctx={ctx} store={store} scope={scope} tab={tab} visible={visible} />
|
||||
}
|
||||
return descriptor.component({
|
||||
ctx, store, scope, tab, visible, expanded,
|
||||
onToggleDir, onReferenceFile, onOpenDiff, onSubagentJump,
|
||||
})
|
||||
// One boundary per tab: a render crash in a viewer/editor shows a strip in
|
||||
// THIS tab's pane only — the toggle cluster, the other tabs, and the panel
|
||||
// stay alive (issue #31). The tab strip (close button) lives outside, so a
|
||||
// crashing tab stays closable; the root boundary in index.tsx remains the
|
||||
// last resort for errors in the sidebar shell itself. The descriptor is
|
||||
// rendered as a REAL element (not called directly): a direct call would
|
||||
// throw inside TabContent's own render, which the boundary cannot catch —
|
||||
// as a child fiber, every render error (top-level or deep) lands in it.
|
||||
return createElement(
|
||||
RenderBoundary,
|
||||
{ className: css.tabBoundaryError },
|
||||
createElement(descriptor.component, {
|
||||
ctx, store, scope, tab, visible, expanded,
|
||||
onToggleDir, onReferenceFile, onOpenDiff, onSubagentJump,
|
||||
}),
|
||||
)
|
||||
}
|
||||
|
||||
/** The + menu options for the current state, driven by the tab registry.
|
||||
@@ -512,6 +525,17 @@ export function Sidebar(props: { ctx: Context; store: SidebarStore }) {
|
||||
: 0
|
||||
document.documentElement.style.setProperty('--dsh-sidebar-width', `${width}px`)
|
||||
document.documentElement.style.setProperty('--dsh-sidebar-height', `${height}px`)
|
||||
// Unmount must release the push (issue #31): when the boundary swaps the
|
||||
// whole sidebar after a render crash (or the plugin fiber is disposed /
|
||||
// HMR), the CSS variables would otherwise stay on <html> and layout.css
|
||||
// keeps squeezing #root with a stale margin — "the sidebar cannot be
|
||||
// hidden" until a full reload. removeProperty restores the CSS fallback
|
||||
// (var(--dsh-sidebar-width, 0px)); React re-runs cleanup+setup in the
|
||||
// same commit on state changes, so there is no visible flicker.
|
||||
return () => {
|
||||
document.documentElement.style.removeProperty('--dsh-sidebar-width')
|
||||
document.documentElement.style.removeProperty('--dsh-sidebar-height')
|
||||
}
|
||||
}, [narrow, snapshot.state?.panelOpen, snapshot.state?.width, snapshot.state?.bottomOpen, snapshot.state?.bottomHeight])
|
||||
useEffect(() => {
|
||||
if (anyDragging) document.body.setAttribute('data-dsh-sidebar-dragging', '')
|
||||
|
||||
+8
-35
@@ -8,7 +8,7 @@
|
||||
* bundle itself is a module-table consumer only (react + ui-primitives +
|
||||
* xterm, all provided or inlined).
|
||||
*/
|
||||
import { Component, createElement, type ErrorInfo, type ReactNode } from 'react'
|
||||
import { createElement } from 'react'
|
||||
import { createRoot, type Root } from 'react-dom/client'
|
||||
import type { Context } from '../context-types.ts'
|
||||
import { createSidebarStore } from './state.ts'
|
||||
@@ -16,6 +16,7 @@ import { createBetterSidebarService } from './service.ts'
|
||||
import { resetChunks } from './chunk-loader.ts'
|
||||
import { registerBuiltins } from './builtins/index.ts'
|
||||
import { Sidebar } from './Sidebar.tsx'
|
||||
import { RenderBoundary } from './RenderBoundary.tsx'
|
||||
import { registerOpenPathInterception, registerTurnTailInterception } from './intercept.tsx'
|
||||
import { registerLinkInterception } from './link-intercept.ts'
|
||||
import { registerImeGuard } from './ime-guard.ts'
|
||||
@@ -31,40 +32,12 @@ import './layout.css'
|
||||
export const inject = ['slots', 'sessions', 'connection', 'workspaces', 'locale']
|
||||
|
||||
/**
|
||||
* Error boundary over the sidebar tree: a render error must never blank the
|
||||
* whole panel silently — it shows a dismissible error strip and logs the
|
||||
* stack for diagnosis.
|
||||
* Error boundary over the sidebar tree (root scope): a render error in the
|
||||
* sidebar SHELL itself must never blank the page silently — the shared
|
||||
* RenderBoundary shows a dismissible error strip and logs the stack. The
|
||||
* per-tab scope (Sidebar.tsx) catches viewer/editor crashes first; this root
|
||||
* boundary stays as the last resort for Workbench/shell errors.
|
||||
*/
|
||||
class SidebarBoundary extends Component<{ children: ReactNode }, { error: string | null }> {
|
||||
state = { error: null as string | null }
|
||||
|
||||
static getDerivedStateFromError(error: unknown): { error: string } {
|
||||
return { error: error instanceof Error ? error.message : String(error) }
|
||||
}
|
||||
|
||||
componentDidCatch(error: Error, info: ErrorInfo): void {
|
||||
console.error('[dsh-better-sidebar] render error:', error, info.componentStack)
|
||||
}
|
||||
|
||||
render(): ReactNode {
|
||||
if (this.state.error !== null) {
|
||||
return (
|
||||
<div className={css.boundaryError}>
|
||||
<span>dsh-better-sidebar: {this.state.error}</span>
|
||||
<button
|
||||
type="button"
|
||||
className={css.terminalRetry}
|
||||
onClick={() => { this.setState({ error: null }) }}
|
||||
>
|
||||
{t('terminalRetry')}
|
||||
</button>
|
||||
</div>
|
||||
)
|
||||
}
|
||||
return this.props.children
|
||||
}
|
||||
}
|
||||
|
||||
/**
|
||||
* Client plugin body.
|
||||
* @param ctx - the client cordis context (slots, sessions).
|
||||
@@ -141,7 +114,7 @@ export function apply(ctx: Context): void {
|
||||
host.setAttribute('data-dsh-better-sidebar', '')
|
||||
document.body.appendChild(host)
|
||||
root = createRoot(host)
|
||||
root.render(createElement(SidebarBoundary, null, createElement(Sidebar, { ctx, store: sidebarStore })))
|
||||
root.render(createElement(RenderBoundary, { className: css.boundaryError }, createElement(Sidebar, { ctx, store: sidebarStore })))
|
||||
} catch (error) {
|
||||
fail('mount', error)
|
||||
}
|
||||
|
||||
@@ -1370,6 +1370,22 @@
|
||||
color: var(--dsw-alias-label-primary);
|
||||
}
|
||||
|
||||
/* The PER-TAB error strip: fills one tab's content area (unlike
|
||||
.boundaryError, the full-height fixed rail for shell-level crashes), so a
|
||||
crashing viewer/editor never hides the toggle cluster or the other tabs. */
|
||||
.tabBoundaryError {
|
||||
flex: 1;
|
||||
min-height: 0;
|
||||
display: flex;
|
||||
flex-direction: column;
|
||||
gap: 8px;
|
||||
align-items: flex-start;
|
||||
padding: 12px 16px;
|
||||
overflow: auto;
|
||||
font: var(--dsw-font-xxs-12);
|
||||
color: var(--dsw-alias-state-error-primary);
|
||||
}
|
||||
|
||||
/* ── Git ───────────────────────────────────────────────────────────────── */
|
||||
|
||||
.git {
|
||||
|
||||
Reference in New Issue
Block a user