Skip to content

Commit 64662db

Browse files
refactor: migrate filter persistence from localStorage to cookies
Fixes hydration timing issue where sidebar Issues link had stale href. Changes: - Add cookie utilities (src/lib/cookies/preferences.ts, client.ts) - MainLayout reads cookies server-side and passes to Sidebar as props - Sidebar accepts issuesPath and initialCollapsed as props - BackToIssuesLink accepts href prop instead of using hook - use-search-filters sets cookie client-side (synchronous) - Remove use-issue-link.ts hook (replaced by cookie approach) Benefits: - No hydration mismatch (server and client render same values) - No race conditions (client-side cookie set is synchronous) - Sidebar collapsed state also migrated to cookies Co-Authored-By: Claude Opus 4.5 <noreply@anthropic.com>
1 parent 8f76d42 commit 64662db

File tree

12 files changed

+151
-75
lines changed

12 files changed

+151
-75
lines changed

e2e/smoke/issue-list.spec.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -297,7 +297,7 @@ test.describe("Issue List Features", () => {
297297
await page.getByTestId("mobile-menu-trigger").click();
298298
}
299299

300-
// The sidebar link uses useIssueLink which reads from localStorage
300+
// The sidebar link uses issuesPath prop read from cookie on the server
301301
await page.getByRole("link", { name: "Issues", exact: true }).click();
302302
await expect(page).toHaveURL(/q=Thing/);
303303

src/app/(app)/m/[initials]/i/[issueNumber]/page.tsx

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@ import { formatIssueId } from "~/lib/issues/utils";
1414
import { ImageGallery } from "~/components/images/ImageGallery";
1515
import type { Issue, IssueWithAllRelations } from "~/lib/types";
1616
import { BackToIssuesLink } from "~/components/issues/BackToIssuesLink";
17+
import { getLastIssuesPath } from "~/lib/cookies/preferences";
1718

1819
/**
1920
* Issue Detail Page (Protected Route)
@@ -28,6 +29,9 @@ export default async function IssueDetailPage({
2829
// Get params (Next.js 16: params is a Promise)
2930
const { initials, issueNumber } = await params;
3031

32+
// Read user preferences from cookies
33+
const issuesPath = await getLastIssuesPath();
34+
3135
// Auth guard
3236
const supabase = await createClient();
3337
const {
@@ -160,7 +164,7 @@ export default async function IssueDetailPage({
160164
return (
161165
<PageShell className="space-y-8" size="wide">
162166
{/* Back button */}
163-
<BackToIssuesLink />
167+
<BackToIssuesLink href={issuesPath} />
164168

165169
{/* Header */}
166170
<div className="space-y-3">

src/components/issues/BackToIssuesLink.tsx

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,15 @@
1-
"use client";
2-
31
import type React from "react";
42
import Link from "next/link";
53
import { ArrowLeft } from "lucide-react";
6-
import { useIssueLink } from "~/hooks/use-issue-link";
74

8-
export function BackToIssuesLink(): React.JSX.Element {
9-
const href = useIssueLink();
5+
interface BackToIssuesLinkProps {
6+
/** The href to navigate to, typically read from cookie on the server */
7+
href?: string;
8+
}
109

10+
export function BackToIssuesLink({
11+
href = "/issues",
12+
}: BackToIssuesLinkProps): React.JSX.Element {
1113
return (
1214
<Link
1315
href={href}

src/components/layout/MainLayout.tsx

Lines changed: 16 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,12 +19,22 @@ import Link from "next/link";
1919
import { Button } from "~/components/ui/button";
2020
import { MobileNav } from "./MobileNav";
2121
import { FeedbackWidget } from "~/components/feedback/FeedbackWidget";
22+
import {
23+
getLastIssuesPath,
24+
getSidebarCollapsed,
25+
} from "~/lib/cookies/preferences";
2226

2327
export async function MainLayout({
2428
children,
2529
}: {
2630
children: React.ReactNode;
2731
}): Promise<React.JSX.Element> {
32+
// Read user preferences from cookies (server-side)
33+
const [issuesPath, sidebarCollapsed] = await Promise.all([
34+
getLastIssuesPath(),
35+
getSidebarCollapsed(),
36+
]);
37+
2838
const supabase = await createClient();
2939
const {
3040
data: { user },
@@ -109,7 +119,11 @@ export async function MainLayout({
109119
<div className="flex h-full bg-background text-foreground">
110120
{/* Sidebar - Hidden on mobile */}
111121
<aside className="hidden md:block h-full">
112-
<Sidebar role={userProfile?.role} />
122+
<Sidebar
123+
role={userProfile?.role}
124+
issuesPath={issuesPath}
125+
initialCollapsed={sidebarCollapsed}
126+
/>
113127
</aside>
114128

115129
{/* Main Content */}
@@ -119,7 +133,7 @@ export async function MainLayout({
119133
{/* Mobile Menu Trigger & Left Spacer */}
120134
<div className="flex-1 flex items-center">
121135
<div className="md:hidden mr-4">
122-
<MobileNav role={userProfile?.role} />
136+
<MobileNav role={userProfile?.role} issuesPath={issuesPath} />
123137
</div>
124138
</div>
125139

src/components/layout/MobileNav.tsx

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,8 +15,10 @@ import {
1515

1616
export function MobileNav({
1717
role,
18+
issuesPath,
1819
}: {
1920
role?: "guest" | "member" | "admin" | undefined;
21+
issuesPath?: string;
2022
}): React.JSX.Element {
2123
const [open, setOpen] = useState(false);
2224

@@ -44,6 +46,7 @@ export function MobileNav({
4446
role={role}
4547
onNavigate={() => setOpen(false)}
4648
isMobile={true}
49+
issuesPath={issuesPath}
4750
/>
4851
</SheetContent>
4952
</Sheet>

src/components/layout/Sidebar.test.tsx

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,7 @@
11
import React from "react";
22
import { render, screen } from "@testing-library/react";
33
import userEvent from "@testing-library/user-event";
4-
import { describe, it, expect, vi, beforeEach } from "vitest";
4+
import { describe, it, expect, vi } from "vitest";
55
import { Sidebar } from "./Sidebar";
66

77
// Mock dependencies
@@ -23,11 +23,12 @@ vi.mock("next/image", () => ({
2323
),
2424
}));
2525

26-
describe("Sidebar Accessibility", () => {
27-
beforeEach(() => {
28-
window.localStorage.clear();
29-
});
26+
// Mock client-side cookie storage
27+
vi.mock("~/lib/cookies/client", () => ({
28+
storeSidebarCollapsed: vi.fn(),
29+
}));
3030

31+
describe("Sidebar Accessibility", () => {
3132
it("provides accessible names for links when collapsed", async () => {
3233
const user = userEvent.setup();
3334
render(<Sidebar />);

src/components/layout/Sidebar.tsx

Lines changed: 14 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1,11 +1,11 @@
11
"use client";
22

33
import type React from "react";
4-
import { useState, useEffect } from "react";
4+
import { useState } from "react";
55
import Link from "next/link";
66
import Image from "next/image";
7-
import { useIssueLink } from "~/hooks/use-issue-link";
87
import { usePathname } from "next/navigation";
8+
import { storeSidebarCollapsed } from "~/lib/cookies/client";
99
import {
1010
LayoutDashboard,
1111
Gamepad2,
@@ -47,27 +47,27 @@ export function Sidebar({
4747
role,
4848
onNavigate,
4949
isMobile = false,
50+
issuesPath,
51+
initialCollapsed = false,
5052
}: {
5153
role?: "guest" | "member" | "admin" | undefined;
5254
onNavigate?: () => void;
5355
isMobile?: boolean;
56+
/** The issues link path, read from cookie on the server */
57+
issuesPath?: string | undefined;
58+
/** Initial collapsed state, read from cookie on the server */
59+
initialCollapsed?: boolean;
5460
}): React.JSX.Element {
55-
const [collapsed, setCollapsed] = useState(false);
61+
const [collapsed, setCollapsed] = useState(initialCollapsed);
5662
const pathname = usePathname();
57-
const issuesLink = useIssueLink();
58-
// Load state from localStorage on mount
59-
useEffect(() => {
60-
if (isMobile) return; // Don't load/save collapse state on mobile
61-
const savedState = window.localStorage.getItem("sidebar-collapsed");
62-
if (savedState) {
63-
setCollapsed(JSON.parse(savedState) as boolean);
64-
}
65-
}, [isMobile]);
63+
// Default to /issues if no path provided
64+
const resolvedIssuesPath = issuesPath ?? "/issues";
6665

6766
const toggleSidebar = (): void => {
6867
const newState = !collapsed;
6968
setCollapsed(newState);
70-
window.localStorage.setItem("sidebar-collapsed", JSON.stringify(newState));
69+
// Persist to cookie synchronously
70+
storeSidebarCollapsed(newState);
7171
};
7272

7373
const NavItem = ({
@@ -80,7 +80,7 @@ export function Sidebar({
8080
variant?: string;
8181
};
8282
}): React.JSX.Element => {
83-
const href = item.href === "/issues" ? issuesLink : item.href;
83+
const href = item.href === "/issues" ? resolvedIssuesPath : item.href;
8484
const hrefPath = href.split("?")[0];
8585
const isActive = pathname === hrefPath;
8686
const isReport = item.variant === "accent";

src/hooks/use-issue-link.ts

Lines changed: 0 additions & 46 deletions
This file was deleted.

src/hooks/use-search-filters.ts

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,7 @@ import { useRouter, usePathname } from "next/navigation";
44
import { useCallback, useRef, useEffect } from "react";
55
import type { IssueFilters } from "~/lib/issues/filters";
66
import type { MachineFilters } from "~/lib/machines/filters";
7-
import { storeLastIssuesPath } from "~/hooks/use-issue-link";
7+
import { storeLastIssuesPath } from "~/lib/cookies/client";
88

99
type GenericFilters = IssueFilters | MachineFilters;
1010

@@ -156,6 +156,7 @@ export function useSearchFilters<T extends GenericFilters>(
156156

157157
const newPath = `${pathname}?${params.toString()}`;
158158
if (pathname.startsWith("/issues")) {
159+
// Set cookie synchronously so it's available on next navigation
159160
storeLastIssuesPath(newPath);
160161
}
161162
router.push(newPath);

src/lib/cookies/client.ts

Lines changed: 32 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,32 @@
1+
"use client";
2+
3+
const LAST_ISSUES_PATH_KEY = "lastIssuesPath";
4+
const SIDEBAR_COLLAPSED_KEY = "sidebarCollapsed";
5+
6+
/**
7+
* Sets a cookie on the client side (synchronous).
8+
* The cookie will be immediately available for the next server request.
9+
*/
10+
function setClientCookie(name: string, value: string, maxAge: number): void {
11+
const secure =
12+
typeof window !== "undefined" && window.location.protocol === "https:";
13+
document.cookie = `${name}=${encodeURIComponent(value)}; path=/; max-age=${maxAge}; SameSite=Lax${secure ? "; Secure" : ""}`;
14+
}
15+
16+
/**
17+
* Stores the last issues path in a cookie (client-side, synchronous).
18+
* This is faster than a server action and ensures the cookie is
19+
* available immediately for the next navigation.
20+
*/
21+
export function storeLastIssuesPath(path: string): void {
22+
const maxAge = 60 * 60 * 24 * 365; // 1 year
23+
setClientCookie(LAST_ISSUES_PATH_KEY, path, maxAge);
24+
}
25+
26+
/**
27+
* Stores the sidebar collapsed state in a cookie (client-side, synchronous).
28+
*/
29+
export function storeSidebarCollapsed(collapsed: boolean): void {
30+
const maxAge = 60 * 60 * 24 * 365; // 1 year
31+
setClientCookie(SIDEBAR_COLLAPSED_KEY, collapsed.toString(), maxAge);
32+
}

0 commit comments

Comments
 (0)