Skip to content

Commit 38efdac

Browse files
authored
Merge pull request #6934 from shin-core/fix/install-tab-ssr-hydration-6814-v3
fix(ui): defer homepage install-tab localStorage read past hydration
2 parents ca60dc0 + 4a840a6 commit 38efdac

2 files changed

Lines changed: 79 additions & 16 deletions

File tree

apps/loopover-ui/src/routes/index.test.tsx

Lines changed: 71 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,5 @@
1-
import { render, screen, fireEvent } from "@testing-library/react";
1+
import { render, screen, fireEvent, waitFor } from "@testing-library/react";
2+
import { renderToString } from "react-dom/server";
23
import { beforeEach, describe, expect, it } from "vitest";
34

45
import { ClientSetupTabs } from "./index";
@@ -8,9 +9,9 @@ import { ClientSetupTabs } from "./index";
89
// the shared Radix-backed Tabs primitive. This is the regression test for that gap.
910

1011
describe("ClientSetupTabs keyboard navigation (#6813)", () => {
11-
// ClientSetupTabs persists the active tab to localStorage ("gt:install-tab") and reads it back as the
12-
// initial state on mount -- without clearing it, a later test's initial "miners" tab assumption breaks
13-
// because an earlier test in this file left a different tab persisted.
12+
// ClientSetupTabs persists the active tab to localStorage ("gt:install-tab") and reads it back on mount
13+
// -- without clearing it, a later test's initial "miners" tab assumption breaks because an earlier test
14+
// in this file left a different tab persisted.
1415
beforeEach(() => {
1516
window.localStorage.clear();
1617
});
@@ -100,3 +101,69 @@ describe("ClientSetupTabs keyboard navigation (#6813)", () => {
100101
expect(codexTab.getAttribute("aria-selected")).toBe("true");
101102
});
102103
});
104+
105+
const STORAGE_KEY = "gt:install-tab";
106+
107+
function selectedTabName(): string | null {
108+
return screen.getByRole("tab", { selected: true }).textContent;
109+
}
110+
111+
describe("ClientSetupTabs SSR-safe hydration (#6814)", () => {
112+
beforeEach(() => {
113+
window.localStorage.clear();
114+
});
115+
116+
it("renders the same markup with or without a saved tab, so hydration cannot mismatch", () => {
117+
// The invariant. The server has no `window` and always emits "miners"; the client's first render must
118+
// agree. Reading localStorage in a useState initializer broke that for a returning visitor, because the
119+
// initializer ran during the very first render. renderToString is the only way to observe that first
120+
// paint -- testing-library's render() wraps in act(), which flushes the mount effect before any
121+
// assertion can see the pre-effect markup.
122+
const withoutSaved = renderToString(<ClientSetupTabs />);
123+
window.localStorage.setItem(STORAGE_KEY, JSON.stringify("cursor"));
124+
const withSaved = renderToString(<ClientSetupTabs />);
125+
126+
expect(withSaved).toBe(withoutSaved);
127+
expect(withSaved).toContain('aria-selected="true"');
128+
});
129+
130+
it("applies the saved tab after mount, once hydration is safely past", async () => {
131+
window.localStorage.setItem(STORAGE_KEY, JSON.stringify("cursor"));
132+
render(<ClientSetupTabs />);
133+
134+
await waitFor(() => expect(selectedTabName()).toContain("Cursor"));
135+
});
136+
137+
it("stays on the default tab when nothing is saved", async () => {
138+
render(<ClientSetupTabs />);
139+
140+
expect(selectedTabName()).toContain("Miner CLI");
141+
// Give the mount-time read a chance to land before asserting it changed nothing.
142+
await waitFor(() => expect(selectedTabName()).toContain("Miner CLI"));
143+
});
144+
145+
it("persists a clicked tab so the next visit restores it", async () => {
146+
render(<ClientSetupTabs />);
147+
148+
fireEvent.click(screen.getByRole("tab", { name: /Claude Desktop/i }));
149+
150+
await waitFor(() => expect(selectedTabName()).toContain("Claude Desktop"));
151+
expect(window.localStorage.getItem(STORAGE_KEY)).toBe(JSON.stringify("claude"));
152+
});
153+
154+
it("falls back to the default when the stored value is unreadable", async () => {
155+
// Pre-#6814 the tab was written as a bare string. "cursor" is not valid JSON, so the hook's read throws
156+
// and the component degrades to the default -- a one-time reset for a returning visitor rather than a
157+
// broken tab. Deliberately a NON-default value: storing "miners" here would pass even if the fallback
158+
// were broken.
159+
window.localStorage.setItem(STORAGE_KEY, "cursor");
160+
render(<ClientSetupTabs />);
161+
162+
await waitFor(() => expect(selectedTabName()).toContain("Miner CLI"));
163+
// And the reset is self-healing: the next write lands in the hook's JSON format.
164+
fireEvent.click(screen.getByRole("tab", { name: /Codex/i }));
165+
await waitFor(() =>
166+
expect(window.localStorage.getItem(STORAGE_KEY)).toBe(JSON.stringify("codex")),
167+
);
168+
});
169+
});

apps/loopover-ui/src/routes/index.tsx

Lines changed: 8 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@ import { useEffect, useRef, useState, type KeyboardEvent } from "react";
33
import { Check, Copy } from "lucide-react";
44
import { toast } from "sonner";
55

6+
import { useLocalStorage } from "@/lib/use-local-storage";
67
import { cn } from "@/lib/utils";
78

89
import { AnimatedTerminal } from "@/components/site/animated-terminal";
@@ -442,6 +443,8 @@ loopover-mcp analyze-branch --login your-login --json`}
442443

443444
type ClientTabId = "miners" | "codex" | "claude" | "cursor" | "remote";
444445

446+
const INSTALL_TAB_STORAGE_KEY = "gt:install-tab";
447+
445448
const CLIENT_TABS: Array<{
446449
id: ClientTabId;
447450
label: string;
@@ -511,10 +514,11 @@ args = ["-y", "@loopover/mcp@latest", "--stdio"]`,
511514
];
512515

513516
export function ClientSetupTabs() {
514-
const [active, setActive] = useState<ClientTabId>(() => {
515-
if (typeof window === "undefined") return "miners";
516-
return (window.localStorage.getItem("gt:install-tab") as ClientTabId) ?? "miners";
517-
});
517+
// Deferred read via useLocalStorage (#6814): a useState initializer runs during the FIRST render, but the
518+
// server render has no `window` and always yields "miners", so a returning visitor with a saved tab
519+
// hydrated into markup the server never produced. The hook reads on mount instead and persists on write,
520+
// which also replaces the write-back effect this component used to run.
521+
const [active, setActive] = useLocalStorage<ClientTabId>(INSTALL_TAB_STORAGE_KEY, "miners");
518522
const [copied, setCopied] = useState(false);
519523
const current = CLIENT_TABS.find((t) => t.id === active) ?? CLIENT_TABS[0];
520524
const tabRefs = useRef<Map<string, HTMLButtonElement>>(new Map());
@@ -543,14 +547,6 @@ export function ClientSetupTabs() {
543547
}
544548
};
545549

546-
useEffect(() => {
547-
try {
548-
window.localStorage.setItem("gt:install-tab", active);
549-
} catch {
550-
/* noop */
551-
}
552-
}, [active]);
553-
554550
const onCopy = async () => {
555551
try {
556552
await navigator.clipboard.writeText(current.snippet);

0 commit comments

Comments
 (0)