Fixed 401 error on the frontend side if it failed to fetch /api/@me.#155
Conversation
📝 WalkthroughWalkthrough
ChangesLogin Page – Logic Fixes and Reformatting
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
pages/login.tsx (1)
366-389:⚠️ Potential issue | 🟠 MajorSwitch to
useRouter()for reactive OAuth error handling.Line 367 uses the singleton
Routerobject in the effect dependency array, which does not guarantee React's proper reactivity tracking. The effect can miss theisReadytransition and skip error toast/query cleanup during navigation. Use theuseRouter()hook instead, which is the documented pattern for Pages Router and ensures proper reactive subscription.Proposed fix
-import Router from "next/router"; +import { useRouter } from "next/router"; const Login: NextPage = () => { + const router = useRouter(); // ... useEffect(() => { - if (!Router.isReady || errorToastShown.current) return; - const { error, action, ...rest } = Router.query; + if (!router.isReady || errorToastShown.current) return; + const { error, action, ...rest } = router.query; if (error) { // ...toast branches... errorToastShown.current = true; - Router.replace({ pathname: Router.pathname, query: rest }, undefined, { + router.replace({ pathname: router.pathname, query: rest }, undefined, { shallow: true, }); } - }, [Router.isReady, Router.query]); + }, [router.isReady, router.query, router.pathname, router.replace]);🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@pages/login.tsx` around lines 366 - 389, The useEffect hook is using the singleton Router object instead of the useRouter() hook, which prevents proper reactive tracking of state changes. Replace all references to the singleton Router with a Router reference obtained from calling useRouter() at the component level. This ensures the effect properly subscribes to Router state changes and executes when Router.isReady transitions or Router.query is updated, allowing the error toast and query cleanup logic to execute correctly during navigation.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@pages/login.tsx`:
- Around line 366-389: The useEffect hook is using the singleton Router object
instead of the useRouter() hook, which prevents proper reactive tracking of
state changes. Replace all references to the singleton Router with a Router
reference obtained from calling useRouter() at the component level. This ensures
the effect properly subscribes to Router state changes and executes when
Router.isReady transitions or Router.query is updated, allowing the error toast
and query cleanup logic to execute correctly during navigation.
1 participant
Summary by CodeRabbit
Bug Fixes
Style