-
Notifications
You must be signed in to change notification settings - Fork 0
Refactor auto login hook #5
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.
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
base: main
Are you sure you want to change the base?
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR extracts inline auto-login logic into a reusable useAutoLogin
hook and updates the LoginPage
to consume it for cleaner separation of concerns.
- Introduces
useAutoLogin
hook to centralize cookie parsing and user fetching - Refactors
pages/login.js
to remove inline effect and use the new hook - Cleans up JSX by removing the stray hook call at render
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
File | Description |
---|---|
hooks/useAutoLogin.js | New hook implementing auto-login logic from cookies and API call |
pages/login.js | Refactored to call useAutoLogin and simplified login/logout UI |
Comments suppressed due to low confidence (1)
hooks/useAutoLogin.js:4
- Add unit tests for
useAutoLogin
to verify behavior when the cookie is absent, malformed, or contains a valid token.
export default function useAutoLogin() {
const parsedData = JSON.parse( | ||
decodeURIComponent(userCookie.split('=')[1]) | ||
); | ||
setUserData(parsedData); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Wrap the JSON.parse call in a try/catch to prevent crashes if the user_data
cookie is malformed or missing.
const parsedData = JSON.parse( | |
decodeURIComponent(userCookie.split('=')[1]) | |
); | |
setUserData(parsedData); | |
try { | |
const parsedData = JSON.parse( | |
decodeURIComponent(userCookie.split('=')[1]) | |
); | |
setUserData(parsedData); | |
} catch (error) { | |
console.error('Failed to parse user_data cookie:', error); | |
} |
Copilot uses AI. Check for mistakes.
const [user, setUser] = useState(null); | ||
const [userData, setUserData] = useState(null); | ||
|
||
useEffect(() => { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Consider adding a cleanup handler or an isMounted
flag to prevent setting state after the component unmounts during the async getCurrentUser
call.
useEffect(() => { | |
useEffect(() => { | |
let isMounted = true; // Flag to track component mount state |
Copilot uses AI. Check for mistakes.
|
||
export default function LoginPage() { | ||
const [ user, setUser ] = useState(null); | ||
const [ userData, setUserData ] = useState(null); | ||
const useAutoLogin = () => { | ||
useEffect(() => { | ||
// if only token is in cookie and no user data | ||
// const token = document.cookie | ||
// .split('; ') | ||
// .find((row) => row.startsWith('token=')) | ||
// ?.split('=')[1]; | ||
const { user, userData } = useAutoLogin(); | ||
|
||
const cookies = document.cookie.split('; '); | ||
const userCookie = cookies.find((cookie) => cookie.startsWith('user_data=')); | ||
const logout = () => { | ||
document.cookie = 'user_data=; expires=Thu, 01 Jan 1970 00:00:00 UTC; path=/;'; | ||
window.location.reload(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
[nitpick] Rather than using window.location.reload()
, consider updating the React state or using Next.js router navigation to handle logout more gracefully.
Copilot uses AI. Check for mistakes.
Summary
useAutoLogin
custom hookLoginPage
Testing
npm run lint
(fails: Invalid next.config.ts options)https://chatgpt.com/codex/tasks/task_e_686360fa4ec48328842e6e04f14768e1