refactor: consolidate shared helpers, prune deps, server-first RSC

[AnnatarHe]

Summary

Codebase quality sweep that consolidates duplicated patterns, prunes unused/redundant packages, and pushes more components to server-first RSC. Organized as 8 self-contained commits that can each be reviewed independently.

• Shared helpers — wenquBooksByIdsQueryOptions, resolveMediaUrl, getUserSlug, uniqueById, useAuthResultEffect, checkIsPremium; ~25 call sites migrated.
• Dependency pruning — removed 13 unused/redundant packages (react-switch, react-select, react-blurhash, @sentry/tracing, recharts, react-transition-group, intersection-observer, @uptrace/node, opentelemetry-node-metrics, classnames, @heroicons/react, @types/heapdump, @types/react-transition-group); migrated 14 files from @heroicons/react → lucide-react; dropped react-cool-inview for lake-ui/hooks/use-in-viewport.
• Button consolidation — deleted button/index.tsx and button/button-simple.tsx; every caller now uses the single feature-complete button/button.tsx (isLoading, variants, sizes).
• Settings layouts — collapsed 5 near-identical settings/{exports,orders,account,web,webhooks}/layout.tsx files into a shared SettingsSectionLayout. Extracted ExportTriggerButton; removed inline SVG spinner duplication in flomo/notion/mail export cards; deleted dead export.email.tsx.
• GlassCard / uniqueById / loading wrappers — extracted glassCardClass for 5 verbatim comment cards, replaced the dedup-by-id reduce in square/profile lists with uniqueById, and collapsed 10 identical loading.tsx files to single-line re-exports.
• Auth page shell — extracted fetchPublicDataWithBooks and AuthPageShell; migrated auth-v4, github, and callback to reuse them (removes 3× radial-gradient duplication + 3× public-data prefetch).
• Server-first RSC — dropped 'use client' from button.tsx, payment/canceled/content.tsx, and link-indicator.tsx; converted premium/badge and profile-analytics to async RSC using getTranslation(); split policy/privacy/page.tsx and auth/signin/layout.tsx into server shells with small client islands (PageTrack, TrackedGithubLink).

Not in this PR (intentionally deferred)

• @tanstack/react-table → lake-ui/table migration (4 tables with heavy custom styling — needs visual regression testing).
• Confirm-dialog @floating-ui/react → lake-ui/modal migration — different API, needs manual verification.

Both can be done as follow-ups once this PR is merged.

Test plan

• `pnpm install` resolves cleanly with the smaller dep tree
• `pnpm codegen && pnpm build` succeeds
• `pnpm lint` and `pnpm format:check` clean
• `pnpm test` passes
• `pnpm dev` smoke-test:
  • Auth flows (auth-v4, github callback) — `AuthPageShell` renders and book covers load
  • Dashboard home/square/book/profile — masonry lists still paginate, dedup works
  • Settings pages (`/settings/{exports,orders,account,web,webhooks}`) — shared layout renders, export modals submit, spinners show via `isLoading`
  • Comments page — glass cards look unchanged
  • Profile page — analytics (now RSC) + activity wall render
  • Privacy page — loads without hydration errors and `PageTrack` still fires
  • Sign-in — github tracking click still fires via `TrackedGithubLink` island
• Bundle diff via `@next/bundle-analyzer` to confirm client-bundle shrinkage

🤖 Generated with Claude Code

---

[gemini-code-assist]

Code Review

This pull request refactors the authentication and dashboard pages by centralizing layout components, simplifying data prefetching, and standardizing UI elements. It replaces @heroicons/react with lucide-react, introduces utility functions for image resolution and user slug generation, and removes unused dependencies. Feedback includes improving the safety of domain length checks, adding guards for empty prefetch queries, simplifying deduplication logic using Set, and sorting query keys to ensure cache stability.

[gemini-code-assist]

Accessing p.domain.length is unsafe if p.domain is null or undefined. Given that GraphQL fields are often nullable, adding a truthiness check or using optional chaining prevents potential runtime crashes if the user profile data is incomplete.

Suggested change

	return p.domain.length > 2 ? p.domain : p.id
	return (p.domain && p.domain.length > 2) ? p.domain : p.id

[gemini-code-assist]

It is more efficient to guard the prefetchQuery call with a check for dbIds.length > 0. If the array is empty, the prefetch is unnecessary and would result in a network request with an empty query parameter, which is inconsistent with the guards used in other parts of the codebase (e.g., in square/page.tsx).

  if (dbIds.length > 0) {
    await rq.prefetchQuery(wenquBooksByIdsQueryOptions(dbIds))
  }

[gemini-code-assist]

The logic for filtering and deduplicating doubanIds can be significantly simplified using a Set. This is more idiomatic in modern JavaScript and easier to maintain than a reduce with an includes check.

    return Array.from(new Set(doubanIds.filter(isValidDoubanId)))

[gemini-code-assist]

Sorting the dbIds array before using it in the queryKey and request URL is highly recommended. Since TanStack Query uses the key for caching, different permutations of the same IDs would lead to cache misses and redundant network requests. Sorting ensures that the cache key remains stable regardless of the input order, improving both client-side and server-side cache efficiency.

Suggested change

	export function wenquBooksByIdsQueryOptions(dbIds: string[]) {
	return {
	queryKey: ['wenqu', 'books', 'dbIds', dbIds] as const,
	queryFn: () =>
	wenquRequest<WenquSearchResponse>(
	`/books/search?dbIds=${dbIds.join('&dbIds=')}`
	),
	staleTime: duration3Days,
	gcTime: duration3Days,
	}
	}
	export function wenquBooksByIdsQueryOptions(dbIds: string[]) {
	const sortedIds = [...dbIds].sort()
	return {
	queryKey: ['wenqu', 'books', 'dbIds', sortedIds] as const,
	queryFn: () =>
	wenquRequest<WenquSearchResponse>(
	`/books/search?dbIds=${sortedIds.join('&dbIds=')}`
	),
	staleTime: duration3Days,
	gcTime: duration3Days,
	}
	}

[devin-ai-integration]

Devin Review found 2 potential issues.

View 7 additional findings in Devin Review.

[devin-ai-integration]

🔴 resolveMediaUrl(undefined) returns empty string, crashing Next.js Image component

In navigation-bar/avatar.tsx, avatarUrl has type string | undefined. The old code ${CDN_DEFAULT_DOMAIN}/${avatarUrl} would produce a non-empty string like "https://cdn.example.com/undefined" when avatarUrl was undefined — a broken URL but not a crash. The new code calls resolveMediaUrl(avatarUrl) which returns '' for undefined input (src/utils/image.ts:11-13). This empty string is then passed directly to <Image src={avatar}> at line 25, and Next.js Image component throws a runtime error when src is an empty string.

Prompt for agents

In src/components/navigation-bar/avatar.tsx, resolveMediaUrl(avatarUrl) returns an empty string when avatarUrl is undefined. This empty string is passed to the Next.js Image component which requires a non-empty src string.

Either:
1. Add a guard/fallback in the component: if avatar is empty, render a placeholder div instead of Image, or use a default avatar URL.
2. Or change resolveMediaUrl to return a default placeholder image URL instead of empty string when path is falsy.

The simplest fix would be to add a conditional render: if avatar is empty, show a placeholder element instead of the Image component.

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[devin-ai-integration]

🔴 Unstable getRedirectPath callback in useEffect deps causes repeated auth side effects

The useAuthResultEffect hook includes getRedirectPath in its useEffect dependency array (line 90). All callers (useAuthBy3rdPartSuccessed, useLoginV3Successed, useAuthByPhoneSuccessed) pass inline arrow functions that create new references on every render. Once auth succeeds and all guards pass, every subsequent re-render creates a new getRedirectPath → triggers the effect → calls onAuthEnd again (which calls syncLoginStateToServer, writes to localStorage/sessionStorage) and queues another navigate. The old per-function implementations had stable deps [called, loading, error, authResponse, navigate] that wouldn't re-trigger after auth completed.

Suggested change

	}, [
	called,
	loading,
	error,
	authResponse,
	navigate,
	options.skipZeroIdGuard,
	getRedirectPath,
	])
	}, [
	called,
	loading,
	error,
	authResponse,
	navigate,
	options.skipZeroIdGuard,
	// getRedirectPath is intentionally omitted — callers pass inline arrows
	// eslint-disable-next-line react-hooks/exhaustive-deps
	])

---

Was this helpful? React with 👍 or 👎 to provide feedback.

[claude]

Code Review

This is a well-organized refactoring PR with clear goals. The DRY improvements, dependency pruning, and RSC conversions are all valuable. Here are my findings organized by priority.

---

🔴 Issues Worth Fixing Before Merge

1. No error handling in fetchPublicDataWithBooks (src/app/auth/public-data-prefetch.ts)

The Apollo query and React Query prefetch both run without any try/catch:

const data = await client.query<PublicDataQuery>({ ... })  // can throw
await rq.prefetchQuery(wenquBooksByIdsQueryOptions(dbIds))  // can throw

This function is called by auth-v4/page.tsx, github/page.tsx, and callback/layout.tsx — all auth entry points. A transient network error would produce an unhandled rejection and surface as a Next.js error page rather than a graceful degradation. Consider wrapping in try/catch and returning an empty/partial result on failure.

---

2. Empty dbIds produces a malformed request URL (src/services/wenqu.ts)

`/books/search?dbIds=${dbIds.join('&dbIds=')}`

When dbIds is empty, this generates /books/search?dbIds= — an empty query parameter. This was already present in the inline code but is now enshrined in a shared utility used across multiple call sites. Consider guarding with an early return or using a proper URLSearchParams builder.

---

3. UserContent type now requires domain — verify all auth mutation responses include it (src/hooks/hooks.ts)

type UserContent = Pick<User, 'id' | 'name' | 'email' | 'avatar' | 'createdAt' | 'domain'>

domain was added to satisfy getUserSlug() in the post-auth redirect. This is a correct change, but it silently tightens the contract on every caller of useAuthResultEffect. If any auth mutation's GraphQL query doesn't select domain in its fragment, it will compile but produce undefined at runtime, causing the redirect to use the numeric id unexpectedly. Worth a quick check of the generated GraphQL operation fragments (Apple, MetaMask, GitHub, OTP).

---

🟡 Suggestions

4. getUserSlug return type (src/utils/profile.utils.ts)

export function getUserSlug(p: Pick<ProfileQuery['me'], 'id' | 'domain'>): string | number

Every call site immediately calls .toString() on the result. Returning a string directly (return p.domain.length > 2 ? p.domain : String(p.id)) would clean up callers and make the API safer.

---

5. isValidDoubanId — magic number with no explanation (src/services/wenqu.ts)

export function isValidDoubanId(id: string): boolean {
  return id.length > 3
}

The > 3 threshold is non-obvious. A brief comment (or a named constant) explaining what this heuristic guards against would help future maintainers.

---

6. resolveMediaUrl called with optional chaining on a required field (src/app/dash/[userid]/clippings/[clippingid]/@content/page.tsx)

src={resolveMediaUrl(creator?.avatar)}

creator?.avatar is string | undefined. resolveMediaUrl handles undefined by returning '', which would render a broken image. In the old code this was creator?.avatar.startsWith('http') — also broken for undefined, so not a regression. But now that the helper exists, it's worth adding a type guard or ensuring creator is required at this point.

---

7. export.email.tsx deletion — verify it was truly dead

The file had 122 lines of implementation using ExportDestination mutations. The PR states it's "dead code" but no commit or comment explains when it was removed from the UI. Worth confirming with a quick grep that nothing imports it, and that the email export feature is intentionally removed (not just detached).

---

8. setTimeout(navigate, 100) retained in useAuthResultEffect (src/hooks/hooks.ts)

setTimeout(() => {
  navigate(getRedirectPath(authResponse) as Route)
}, 100)

This was in the original useAuthBy3rdPartSuccessed and is carried over. The arbitrary 100ms delay is a fragile timing workaround. Since this is a refactor PR, consider adding a comment explaining why the delay exists (or removing it if it's no longer needed).

---

✅ Things That Look Good

• uniqueById — clean, correctly generic implementation. Replaces several verbose reduce patterns.
• glassCardClass — extracting the verbatim string constant is exactly the right level of abstraction.
• Loading.tsx collapses — export { default } from '...' is idiomatic and eliminates boilerplate wrappers.
• AuthPageShell — well-executed extraction; eliminates the radial-gradient and HydrationBoundary duplication across three auth pages.
• SettingsSectionLayout — the iconBgClass pattern is appropriate since these are hardcoded Tailwind classes, not user input.
• button.tsx 'use client' removal — correct, the component has no hooks or browser APIs. Its interactive behavior comes from the calling context.
• ProfileAnalytics RSC conversion — removing useMemo is fine; the memoization was only meaningful in a client re-render context.
• classnames → clsx/cn migration — good consolidation.
• Deleted Chinese TODO comment in github/page.tsx — the code now reflects the intent (it is done server-side), so removing the note is correct.

[chatgpt-codex-connector]

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: 1c6cd14ae4

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

• Open a pull request for review
• Mark a draft as ready
• Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

[chatgpt-codex-connector]

Remove unstable redirect callback from auth effect deps

Including getRedirectPath in useAuthResultEffect’s dependency array makes the effect fire on every re-render after auth succeeds, because each caller passes a new inline lambda (useAuthBy3rdPartSuccessed, useLoginV3Successed, useAuthByPhoneSuccessed). In those states, the effect repeats onAuthEnd(...) and queues duplicate navigations, which can trigger repeated login-state sync calls and flaky redirects before unmount.

Useful? React with 👍 / 👎.

[chatgpt-codex-connector]

Preserve a non-empty media URL for missing avatar paths

Returning '' for falsy media paths causes consumers that pass the value directly to next/image (for example AvatarOnNavigationBar) to render an invalid image src when a user has no avatar yet. That produces runtime errors in Next.js instead of a degradable broken image, so users with empty avatar values can hit a navigation bar render failure.

Useful? React with 👍 / 👎.