Skip to content

🧹 [Code Health] Fix React hydration issues and TypeScript strictness #85

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
APPLEPIE6969 wants to merge 1 commit into
base: main
Choose a base branch
from
Open

🧹 [Code Health] Fix React hydration issues and TypeScript strictness #85

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
14 changes: 10 additions & 4 deletions app/courses/create/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,10 +10,15 @@ import Link from "next/link"
export default function CreateCourse() {
const { data: session, status } = useSession()
const router = useRouter()
const [isLoading, setIsLoading] = useState(true)
const [courseName, setCourseName] = useState("")
const [subject, setSubject] = useState("")
const [description, setDescription] = useState("")
const [isMounted, setIsMounted] = useState(false)

useEffect(() => {
// eslint-disable-next-line react-hooks/set-state-in-effect
setIsMounted(true)
}, [])

useEffect(() => {
if (status === "unauthenticated") {
Expand All @@ -22,20 +27,21 @@ export default function CreateCourse() {
const email = session?.user?.email;
if (email && !isOnboardingComplete(email)) {
router.push("/onboarding")
} else {
setIsLoading(false)
}
}
}, [status, session, router])

const isRedirecting = status === "unauthenticated" ||
(status === "authenticated" && session?.user?.email && !isOnboardingComplete(session.user.email))

const handleSubmit = (e: React.FormEvent) => {
e.preventDefault()
// In a real app, this would save to a database
// For now, just redirect back to courses
router.push("/courses")
}

if (status === "loading" || isLoading) {
if (status === "loading" || !isMounted || isRedirecting) {
return (
<div className="flex h-screen w-full items-center justify-center bg-background-dark">
<div className="flex flex-col items-center gap-4">
Expand Down
30 changes: 17 additions & 13 deletions app/dashboard/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -60,12 +60,17 @@ export default function Dashboard() {
const { data: session, status } = useSession()
const router = useRouter()
const { t } = useLanguage()
const [userStats, setUserStats] = useState<UserStats>(defaultStats)
const [userData, setUserData] = useState<UserData>(emptyUserData)
const [isLoading, setIsLoading] = useState(true)
const [showTutorial, setShowTutorial] = useState(false)
const [activityPeriod, setActivityPeriod] = useState("week")
const { theme, toggleTheme, mounted } = useTheme()
const [isClientMounted, setIsClientMounted] = useState(false)

// Track client hydration
useEffect(() => {
// eslint-disable-next-line react-hooks/set-state-in-effect
setIsClientMounted(true)
}, [])

// Check authentication and onboarding
useEffect(() => {
Expand All @@ -75,8 +80,7 @@ export default function Dashboard() {
}

if (status === "authenticated" && session?.user?.email) {
// Check if user has completed onboarding
const email = session?.user?.email;
const email = session.user.email;
if (email && !isOnboardingComplete(email)) {
router.push("/onboarding")
return
Expand All @@ -85,24 +89,24 @@ export default function Dashboard() {
// Record today's activity
recordActivity()

// Load user stats
const profile = getUserProfile()
if (profile?.stats) {
setUserStats(profile.stats)
}


// Check if tutorial should be shown
if (email && !isTutorialComplete(email)) {
// eslint-disable-next-line react-hooks/set-state-in-effect
setShowTutorial(true)
}

// Load user data (empty for new users)
setUserData(emptyUserData)
setIsLoading(false)
}
}, [status, session, router])

// Compute redirect state for render short-circuiting
const isRedirecting = status === "unauthenticated" ||
(status === "authenticated" && session?.user?.email && !isOnboardingComplete(session.user.email))

// Derive stats directly instead of using state to prevent set-state-in-effect
const userStats = isClientMounted ? (getUserProfile()?.stats || defaultStats) : defaultStats

Comment on lines 89 to +109
Copy link
Copy Markdown

@qodo-code-review qodo-code-review Bot May 5, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Action required

1. Stale stats after record 🐞 Bug ≡ Correctness

In Dashboard, recordActivity() mutates localStorage, but the effect often performs no state change
afterwards (setUserData(emptyUserData) can be a no-op), so the UI can keep showing pre-update stats
(e.g., dailyStreak) until another unrelated rerender occurs.
Agent Prompt 
### Issue description
`recordActivity()` updates the user profile in `localStorage`, but `Dashboard` may not rerender afterwards (because `setUserData(emptyUserData)` can set the same object reference and be ignored). As a result, `userStats` (derived during render from `getUserProfile()`) can remain stale (e.g., daily streak not incrementing) until some unrelated state change happens.

### Issue Context
- `userStats` is computed from `getUserProfile()?.stats` during render.
- `recordActivity()` updates `profile.stats` + persists via `saveUserProfile()`, but does not itself notify React.
- If the tutorial is already complete, `setShowTutorial(true)` won’t run, and `setUserData(emptyUserData)` may not trigger a rerender.

### Fix Focus Areas
- Ensure a rerender occurs after `recordActivity()` when it updates data used in render.
  - Option A: reintroduce a `userStats` state and set it after `recordActivity()`.
  - Option B: keep derived stats, but add a lightweight state “version” (e.g., `profileRevision`) that you increment after `recordActivity()` (and after other profile mutations) to force a rerender.
  - Avoid `setUserData(emptyUserData)` as a rerender mechanism; if you truly need it, set a new object reference.

- app/dashboard/page.tsx[53-109]
- lib/userStore.ts[166-198]

ⓘ Copy this prompt and use it to remediate the issue with your preferred AI generation tools

// Get display name from session or profile
const displayName = session?.user?.name?.split(" ")[0] || "Learner"

Expand All @@ -111,7 +115,7 @@ export default function Dashboard() {
const hasActivity = userData.weeklyActivity.some(v => v > 0)
const hasEvents = userData.upcomingEvents.length > 0

if (status === "loading" || isLoading) {
if (status === "loading" || !isClientMounted || isRedirecting) {
return (
<div className="flex h-screen w-full items-center justify-center bg-background-dark">
<div className="flex flex-col items-center gap-4">
Expand Down
1 change: 1 addition & 0 deletions app/layout.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@ export default function RootLayout({
/>
<link rel="preconnect" href="https://fonts.googleapis.com" />
<link rel="preconnect" href="https://fonts.gstatic.com" crossOrigin="anonymous" />
{/* eslint-disable-next-line @next/next/no-page-custom-font */}
<link href="https://fonts.googleapis.com/css2?family=Material+Symbols+Outlined:wght,FILL@100..700,0..1&display=swap" rel="stylesheet" />
</head>
<body className={`${inter.className} bg-[var(--background)] text-[var(--foreground)] font-display overflow-x-hidden antialiased selection:bg-primary selection:text-white transition-colors duration-300`}>
Expand Down
22 changes: 18 additions & 4 deletions app/quizzes/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,12 @@ export default function MyQuizzes() {
const router = useRouter()
const { t } = useLanguage()
const [quizzes, setQuizzes] = useState<SavedQuiz[]>([])
const [isLoading, setIsLoading] = useState(true)
const [isMounted, setIsMounted] = useState(false)

useEffect(() => {
// eslint-disable-next-line react-hooks/set-state-in-effect
setIsMounted(true)
}, [])

useEffect(() => {
if (status === "unauthenticated") {
Expand All @@ -29,11 +34,20 @@ export default function MyQuizzes() {
router.push("/onboarding")
return
}
}
}, [status, session, router])

const isRedirecting = status === "unauthenticated" ||
(status === "authenticated" && session?.user?.email && !isOnboardingComplete(session.user.email))

// Load quizzes
useEffect(() => {
if (status === "authenticated" && session?.user?.email && !isRedirecting) {
// eslint-disable-next-line react-hooks/set-state-in-effect
setQuizzes(getUserQuizzes())
setIsLoading(false)
}
}, [status, session, router])
}, [status, session, isRedirecting])


const handleDelete = (id: string) => {
if (confirm(t("quizzes.delete_confirm"))) {
Expand All @@ -52,7 +66,7 @@ export default function MyQuizzes() {
})
}

if (status === "loading" || isLoading) {
if (status === "loading" || !isMounted || isRedirecting) {
return (
<div className="flex h-screen w-full items-center justify-center bg-background-dark">
<div className="flex flex-col items-center gap-4">
Expand Down
14 changes: 10 additions & 4 deletions app/schedule/page.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,12 @@ import { isOnboardingComplete } from "@/lib/userStore"
export default function Schedule() {
const { data: session, status } = useSession()
const router = useRouter()
const [isLoading, setIsLoading] = useState(true)
const [isMounted, setIsMounted] = useState(false)

useEffect(() => {
// eslint-disable-next-line react-hooks/set-state-in-effect
setIsMounted(true)
}, [])

useEffect(() => {
if (status === "unauthenticated") {
Expand All @@ -19,13 +24,14 @@ export default function Schedule() {
const email = session?.user?.email;
if (email && !isOnboardingComplete(email)) {
router.push("/onboarding")
} else {
setIsLoading(false)
}
}
}, [status, session, router])

if (status === "loading" || isLoading) {
const isRedirecting = status === "unauthenticated" ||
(status === "authenticated" && session?.user?.email && !isOnboardingComplete(session.user.email))

if (status === "loading" || !isMounted || isRedirecting) {
return (
<div className="flex h-screen w-full items-center justify-center bg-background-dark">
<div className="flex flex-col items-center gap-4">
Expand Down
2 changes: 1 addition & 1 deletion lib/auth.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import NextAuth from "next-auth"
import Google from "next-auth/providers/google"
import GitHub from "next-auth/providers/github"

// @ts-ignore
// @ts-expect-error - Expected due to missing or mismatched types from NextAuth beta
export const { handlers, signIn, signOut, auth } = NextAuth({
providers: [
Google({
Expand Down
2 changes: 1 addition & 1 deletion lib/security.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ describe('security lib', () => {

// Passing something that isn't a string might trigger an error if not handled
// although verifyData expects a string type.
// @ts-ignore
// @ts-expect-error - Testing invalid runtime type input for verifyData
const isValid = verifyData(testData, { not: 'a string' });
assert.strictEqual(isValid, false);
});
Expand Down
8 changes: 4 additions & 4 deletions lib/setupTests.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,13 +32,13 @@ export class MockStorage {

export function setupTests() {
if (typeof global.window === "undefined") {
// @ts-ignore
global.window = {};
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(global as any).window = {};
}

if (typeof global.localStorage === "undefined") {
// @ts-ignore
global.localStorage = new MockStorage();
// eslint-disable-next-line @typescript-eslint/no-explicit-any
(global as any).localStorage = new MockStorage();
}
}

Expand Down