diff --git a/docs/frontend-inactive-shell-tools-performance-review.md b/docs/frontend-inactive-shell-tools-performance-review.md new file mode 100644 index 00000000..931accf7 --- /dev/null +++ b/docs/frontend-inactive-shell-tools-performance-review.md @@ -0,0 +1,79 @@ +# Frontend inactive shell tools performance review + +## Scope + +This review targets Electron launch when the optional New York vertical tab sidebar is disabled and +the Connect Device dialog is closed. Both are the normal startup state. Extension state, tab-strip +selection, pane sizing, settings controls, device-resume state, and lightweight open/close launchers +remain eager. + +## Findings + +`vertical-sidebar/extension.tsx` statically imports the 34.1 KiB sidebar view even though the +extension defaults to disabled and its pane renderer is never called in that state. Startup still +parses and evaluates the view, its menus, drag interactions, preview providers, and row components. + +The shell also imports the 19.9 KiB Connect Device controller so one menu item can call `open()`, and +the closed overlay host subscribes to that controller for the lifetime of the app. The controller's +pairing state machine, bridge operations, timers, and device types therefore participate in every +startup. A small request store can preserve instant open and interrupted-pairing resume behavior +without loading the controller until the exact dialog fallback is visible. + +No existing optimization comment proposes deferring either inactive boundary. + +## Proposed change + +Keep the extension registration and all state cells permanent. Load the sidebar view only when its +pane renderer is requested. If persisted state enables the sidebar, begin the same cached import +during extension activation so it overlaps the rest of startup. + +Keep a tiny Connect Device request store eager. Shell and Settings actions update that store. When a +request or persisted pairing resume exists, the existing overlay host immediately renders its exact +dialog skeleton while a shared import loads the controller and final dialog. Controller close and +cancellation completion clear the request store, preserving the existing asynchronous cancellation +contract. + +The sidebar fallback and final view share exact StyleX geometry for the permanent full-height root, +macOS traffic-light seat, navigation inset, and footer. Static row placeholders use the canonical 28 +px sidebar row height. The Connect Device fallback retains its permanent dialog header, 288 px stage, +close affordance, and copy layout. Dynamic tab labels, status, grouping, device state, and controls +arrive with their chunks without changing outer geometry. + +## Development check + +Extend `pnpm --filter @honk/app dev:startup-review` so it fails while either inactive implementation +is in the eager application graph. Run each guard before implementation to prove the baseline and +after the combined change to start Vite. Use one corroborating production build, focused +extension/controller tests, and the bounded Electron first-commit/first-frame probe before merging. + +The acceptance target is at least 2% less eager application source or Electron window-ready time, +with no measurable increase in initial React render duration. The sidebar-only checkpoint reached +1.92% median window-ready and 1.96% first-frame improvement, so it is not an independent publishable +candidate and must be measured again with the closed controller boundary. + +## Non-goals + +- Do not change extension storage, enablement, pane sizing, titlebar selection, or settings behavior. +- Do not change tab grouping, filtering, ordering, drag/drop, menus, previews, or status rendering. +- Do not change device pairing, cancellation, exposure, restart, resume, or polling behavior. +- Do not change build configuration or allocators. +- Do not use browser automation or app-level control for measurement. + +## Results + +The dev startup graph changed from 142 modules / 1,256.5 KiB to 146 modules / 1,208.0 KiB. The four +small launcher, resource, and shared-layout modules replace 48.5 KiB of inactive implementation +source, a 3.86% reduction. Both guarded implementation modules are absent from the eager graph. + +A single production build per tree corroborates the split. Initial JavaScript fell from 2,828,919 to +2,803,219 raw bytes (-0.91%) and from 861,322 to 855,209 gzip bytes (-0.71%). + +The bounded Electron review used one warm-up followed by three runs per tree. Median +process-to-ready fell from 5,926 ms to 5,819 ms (-1.81%), median window-created-to-ready fell from +5,094 ms to 4,987 ms (-2.10%), and median first post-ready frame fell from 5,105.6 ms to 4,998.4 ms +(-2.10%). + +The same runs used a temporary root React Profiler, removed after measurement. Median initial actual +render duration fell from 2.0 ms to 1.9 ms, while median commit-to-next-frame fell from 6.4 ms to 5.6 +ms. The combined boundary clears the startup target without slowing component rendering to first +paint. diff --git a/packages/app/src/connect-device-controller.ts b/packages/app/src/connect-device-controller.ts index 239eb6a0..bd7e49f3 100644 --- a/packages/app/src/connect-device-controller.ts +++ b/packages/app/src/connect-device-controller.ts @@ -16,8 +16,8 @@ import { issueDesktopRemotePairing, restartDesktopRemoteHost, } from "./desktop-bridge"; +import { connectDeviceRequest } from "./connect-device-request-store"; -const PAIRING_RESUME_KEY = "honk.desktop.resume-device-pairing"; const RELAUNCH_TIMEOUT_MS = 15_000; type PairingContext = { @@ -624,11 +624,13 @@ export const connectDeviceController = createConnectDeviceController({ const timer = window.setInterval(listener, 1_000); return () => window.clearInterval(timer); }, - resume: { - get: () => window.localStorage.getItem(PAIRING_RESUME_KEY) === "1", - set: () => window.localStorage.setItem(PAIRING_RESUME_KEY, "1"), - clear: () => window.localStorage.removeItem(PAIRING_RESUME_KEY), - }, + resume: connectDeviceRequest.resume, +}); + +connectDeviceController.subscribe(() => { + if (connectDeviceController.getSnapshot().status === "closed") { + connectDeviceRequest.actions.close(); + } }); export function useConnectDeviceSnapshot(): ConnectDeviceSnapshot { diff --git a/packages/app/src/connect-device-request-store.ts b/packages/app/src/connect-device-request-store.ts new file mode 100644 index 00000000..d1cbb0cb --- /dev/null +++ b/packages/app/src/connect-device-request-store.ts @@ -0,0 +1,47 @@ +import { useSyncExternalStore } from "react"; + +const PAIRING_RESUME_KEY = "honk.desktop.resume-device-pairing"; + +const listeners = new Set<() => void>(); +let isRequested = readResumeRequest(); + +function readResumeRequest(): boolean { + return typeof window !== "undefined" && window.localStorage.getItem(PAIRING_RESUME_KEY) === "1"; +} + +function publish(next: boolean): void { + if (isRequested === next) return; + isRequested = next; + for (const listener of listeners) listener(); +} + +const connectDeviceRequest = { + subscribe(listener: () => void): () => void { + listeners.add(listener); + return () => listeners.delete(listener); + }, + getSnapshot: (): boolean => isRequested, + actions: { + open: (): void => publish(true), + close: (): void => publish(false), + }, + resume: { + get: readResumeRequest, + set(): void { + window.localStorage.setItem(PAIRING_RESUME_KEY, "1"); + }, + clear(): void { + window.localStorage.removeItem(PAIRING_RESUME_KEY); + }, + }, +} as const; + +function useConnectDeviceRequest(): boolean { + return useSyncExternalStore( + (listener) => connectDeviceRequest.subscribe(listener), + connectDeviceRequest.getSnapshot, + () => false, + ); +} + +export { connectDeviceRequest, useConnectDeviceRequest }; diff --git a/packages/app/src/connect-device.tsx b/packages/app/src/connect-device.tsx index 0908258e..2ece56ea 100644 --- a/packages/app/src/connect-device.tsx +++ b/packages/app/src/connect-device.tsx @@ -28,7 +28,14 @@ import { radiusVars, spaceVars, } from "@honk/ui/tokens.stylex"; -import { useEffect, useRef, useState, type ReactElement, type ReactNode } from "react"; +import { + useEffect, + useLayoutEffect, + useRef, + useState, + type ReactElement, + type ReactNode, +} from "react"; import { toQR } from "toqr"; import { @@ -36,6 +43,7 @@ import { type ConnectDeviceSnapshot, useConnectDeviceSnapshot, } from "./connect-device-controller"; +import { useConnectDeviceRequest } from "./connect-device-request-store"; import { canManageDesktopRemoteHost, openDesktopExternal } from "./desktop-bridge"; import { actions as settingsActions } from "./settings-store"; import { actions as toastActions } from "./toast-store"; @@ -789,15 +797,22 @@ export function ConnectDeviceBody(props: { export function ConnectDeviceDialog(): ReactElement | null { const available = canManageDesktopRemoteHost(); const snapshot = useConnectDeviceSnapshot(); + const isRequested = useConnectDeviceRequest(); useEffect(() => { if (available) connectDeviceController.actions.resume(); }, [available]); + useLayoutEffect(() => { + if (available && isRequested && snapshot.status === "closed") { + connectDeviceController.actions.open(); + } + }, [available, isRequested, snapshot.status]); + if (!available) return null; return ( { if (!open) connectDeviceController.actions.close(); }} diff --git a/packages/app/src/desktop-extensions/vertical-sidebar/extension.tsx b/packages/app/src/desktop-extensions/vertical-sidebar/extension.tsx index 73ea3b6e..2f2ed090 100644 --- a/packages/app/src/desktop-extensions/vertical-sidebar/extension.tsx +++ b/packages/app/src/desktop-extensions/vertical-sidebar/extension.tsx @@ -1,6 +1,7 @@ import { defineHonkDesktopExtension } from "../sdk"; import { decodeStatusFilters, decodeStringList, type StatusFilter } from "./model"; -import { VerticalSidebar } from "./view"; +import { loadVerticalSidebarView } from "./resource"; +import { VerticalSidebarSurface } from "./surface"; const SIDEBAR_DEFAULT_SIZE = 232; const SIDEBAR_MIN_SIZE = 184; @@ -30,6 +31,10 @@ export const verticalSidebarExtension = defineHonkDesktopExtension({ decode: decodeStatusFilters, }); + if (enabled.get()) { + void loadVerticalSidebarView().catch(() => undefined); + } + honk.desktop.titlebar.tabStrip({ id: "default-tabs", hidden: enabled }); honk.desktop.panes.add({ id: "tabs", @@ -39,7 +44,7 @@ export const verticalSidebarExtension = defineHonkDesktopExtension({ minSize: SIDEBAR_MIN_SIZE, maxSize: SIDEBAR_MAX_SIZE, render: () => ( - +
+ +
+ + + ); +} + +export { VerticalSidebarLoading }; diff --git a/packages/app/src/desktop-extensions/vertical-sidebar/resource.ts b/packages/app/src/desktop-extensions/vertical-sidebar/resource.ts new file mode 100644 index 00000000..59e4803e --- /dev/null +++ b/packages/app/src/desktop-extensions/vertical-sidebar/resource.ts @@ -0,0 +1,14 @@ +type VerticalSidebarViewModule = typeof import("./view"); + +let sharedViewModule: Promise | null = null; + +function loadVerticalSidebarView(): Promise { + if (sharedViewModule !== null) return sharedViewModule; + sharedViewModule = import("./view").catch((error: unknown) => { + sharedViewModule = null; + throw error; + }); + return sharedViewModule; +} + +export { loadVerticalSidebarView }; diff --git a/packages/app/src/desktop-extensions/vertical-sidebar/surface.tsx b/packages/app/src/desktop-extensions/vertical-sidebar/surface.tsx new file mode 100644 index 00000000..dba16c5e --- /dev/null +++ b/packages/app/src/desktop-extensions/vertical-sidebar/surface.tsx @@ -0,0 +1,19 @@ +import { lazy, Suspense, type ReactElement } from "react"; + +import { VerticalSidebarLoading } from "./loading"; +import { loadVerticalSidebarView } from "./resource"; +import type { VerticalSidebarInput } from "./types"; + +const DeferredVerticalSidebar = lazy(() => + loadVerticalSidebarView().then((module) => ({ default: module.VerticalSidebar })), +); + +function VerticalSidebarSurface(input: VerticalSidebarInput): ReactElement { + return ( + }> + + + ); +} + +export { VerticalSidebarSurface }; diff --git a/packages/app/src/desktop-extensions/vertical-sidebar/types.ts b/packages/app/src/desktop-extensions/vertical-sidebar/types.ts new file mode 100644 index 00000000..e1b60823 --- /dev/null +++ b/packages/app/src/desktop-extensions/vertical-sidebar/types.ts @@ -0,0 +1,12 @@ +import type { HonkDesktopCell, HonkDesktopTabs } from "../sdk"; +import type { StatusFilter } from "./model"; + +type VerticalSidebarInput = { + readonly tabs: HonkDesktopTabs; + readonly collapsedGroups: HonkDesktopCell; + readonly workspaceOrder: HonkDesktopCell; + readonly workspacesOpen: HonkDesktopCell; + readonly threadFilters: HonkDesktopCell; +}; + +export type { VerticalSidebarInput }; diff --git a/packages/app/src/desktop-extensions/vertical-sidebar/view.tsx b/packages/app/src/desktop-extensions/vertical-sidebar/view.tsx index 91b28b63..4f71f4f1 100644 --- a/packages/app/src/desktop-extensions/vertical-sidebar/view.tsx +++ b/packages/app/src/desktop-extensions/vertical-sidebar/view.tsx @@ -27,7 +27,6 @@ import { iconVars, motionVars, radiusVars, - shellVars, sidebarVars, } from "@honk/ui/tokens.stylex"; import { @@ -45,6 +44,7 @@ import { useAppSettings } from "../../app-settings-store"; import { canPickFolder, pickFolder } from "../../desktop-bridge"; import { OpenTabContextMenu, WorkspaceContextMenu } from "../../tab-context-menu"; import type { HonkDesktopCell, HonkDesktopTabs } from "../sdk"; +import { verticalSidebarLayout } from "./layout.stylex"; import { STATUS_FILTER_OPTIONS, buildWorkspaceDrop, @@ -62,6 +62,7 @@ import { type StatusFilter, type WorkspaceTabGroup, } from "./model"; +import type { VerticalSidebarInput } from "./types"; const DRAG_ACTIVATION_DISTANCE = 4; const WORKSPACE_ORDER_CAP = 50; @@ -82,38 +83,6 @@ const TITLE_MUTED = { color: colorVars["--honk-color-text-muted"] } as const; const TITLE_FAINT = { color: colorVars["--honk-color-text-faint"] } as const; const styles = create({ - root: { - width: "100%", - height: "100%", - minWidth: 0, - minHeight: 0, - display: "flex", - flexDirection: "column", - backgroundColor: "transparent", - }, - // Empty seat that keeps the first row clear of the macOS traffic lights, which sit over the - // sidebar's top strip. Cursor reserves the same band with its 35px sidebar top bar. - topBar: { - height: shellVars["--honk-shell-titlebar-h"], - flexShrink: 0, - paddingTop: shellVars["--honk-shell-titlebar-seat"], - }, - navigation: { - minHeight: 0, - flexGrow: 1, - overflowY: "auto", - paddingInline: sidebarVars["--honk-sidebar-gutter-inline"], - paddingBlockStart: sidebarVars["--honk-sidebar-gutter-inline"], - paddingBlockEnd: sidebarVars["--honk-sidebar-gutter-inline"], - }, - navigationContent: { - minWidth: 0, - display: "flex", - flexDirection: "column", - // Cursor separates its header action stack from the scroller's groups by the group gap - // rather than the 1px row gap. - gap: sidebarVars["--honk-sidebar-section-gap"], - }, // Cursor masks the scroller instead of drawing a scrolled divider; `black` here is a mask // alpha, not a surface color. fadeTop: { @@ -326,11 +295,6 @@ const styles = create({ fontSize: sidebarVars["--honk-sidebar-label-size"], lineHeight: sidebarVars["--honk-sidebar-label-leading"], }, - footer: { - flexShrink: 0, - paddingInline: sidebarVars["--honk-sidebar-gutter-inline"], - paddingBlock: sidebarVars["--honk-sidebar-gutter-inline"], - }, }); // Scales the 20px matrix into the 16px leading slot without changing glyph geometry. @@ -368,14 +332,6 @@ type WorkspaceDragHandlers = { readonly pointerCancel: RowPointerHandler; }; -type VerticalSidebarInput = { - readonly tabs: HonkDesktopTabs; - readonly collapsedGroups: HonkDesktopCell; - readonly workspaceOrder: HonkDesktopCell; - readonly workspacesOpen: HonkDesktopCell; - readonly threadFilters: HonkDesktopCell; -}; - export function VerticalSidebar(input: VerticalSidebarInput): ReactElement { const snapshot = useTabs(input.tabs); const persistedCollapsedKeys = useCell(input.collapsedGroups); @@ -426,16 +382,16 @@ export function VerticalSidebar(input: VerticalSidebarInput): ReactElement { return ( -