[TASK-16676] fix: Request pots redirect#1420
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
WalkthroughReplaces client-side navigation with full-page redirects in Changes
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes
Possibly related PRs
Suggested reviewers
Pre-merge checks and finishing touches❌ Failed checks (1 inconclusive)
✅ Passed checks (2 passed)
✨ Finishing touches
🧪 Generate unit tests (beta)
📜 Recent review detailsConfiguration used: CodeRabbit UI Review profile: CHILL Plan: Pro 📒 Files selected for processing (1)
🧰 Additional context used🧠 Learnings (12)📓 Common learnings📚 Learning: 2024-10-07T15:25:45.170ZApplied to files:
📚 Learning: 2024-10-07T15:28:25.280ZApplied to files:
📚 Learning: 2024-10-08T20:13:42.967ZApplied to files:
📚 Learning: 2025-08-22T07:28:32.281ZApplied to files:
📚 Learning: 2024-12-11T10:13:22.806ZApplied to files:
📚 Learning: 2024-10-08T20:13:42.967ZApplied to files:
📚 Learning: 2024-10-22T18:10:56.955ZApplied to files:
📚 Learning: 2025-09-18T09:30:42.901ZApplied to files:
📚 Learning: 2025-06-18T19:56:55.443ZApplied to files:
📚 Learning: 2024-10-07T13:42:07.299ZApplied to files:
📚 Learning: 2024-10-23T09:38:27.670ZApplied to files:
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
🔇 Additional comments (1)
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 |
There was a problem hiding this comment.
Actionable comments posted: 1
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/hooks/useLogin.tsx(1 hunks)
🧰 Additional context used
🧠 Learnings (7)
📓 Common learnings
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1185
File: src/components/Claim/useClaimLink.tsx:14-0
Timestamp: 2025-09-05T07:31:11.396Z
Learning: In the peanut-ui codebase, `window.history.replaceState` is preferred over `router.replace` when immediate/synchronous URL parameter updates are required, as `router.replace` is asynchronous and doesn't guarantee instant URL changes that subsequent code can rely on. This pattern is used consistently across usePaymentInitiator.ts, Confirm.payment.view.tsx, and useClaimLink.tsx.
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 0
File: :0-0
Timestamp: 2025-07-05T16:58:25.340Z
Learning: Hugo0 successfully refactored sessionStorage usage to React Context in the onramp flow, demonstrating preference for centralized state management over browser storage for component-shared state in React applications.
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1185
File: src/components/Claim/useClaimLink.tsx:14-0
Timestamp: 2025-09-05T07:31:11.396Z
Learning: In the peanut-ui codebase, `window.history.replaceState` is used instead of `router.replace` in URL manipulation functions when immediate/synchronous URL updates are required for subsequent actions to read the updated URL parameters.
📚 Learning: 2025-09-05T07:31:11.396Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1185
File: src/components/Claim/useClaimLink.tsx:14-0
Timestamp: 2025-09-05T07:31:11.396Z
Learning: In the peanut-ui codebase, `window.history.replaceState` is preferred over `router.replace` when immediate/synchronous URL parameter updates are required, as `router.replace` is asynchronous and doesn't guarantee instant URL changes that subsequent code can rely on. This pattern is used consistently across usePaymentInitiator.ts, Confirm.payment.view.tsx, and useClaimLink.tsx.
Applied to files:
src/hooks/useLogin.tsx
📚 Learning: 2025-07-05T16:58:25.340Z
Learnt from: Hugo0
Repo: peanutprotocol/peanut-ui PR: 0
File: :0-0
Timestamp: 2025-07-05T16:58:25.340Z
Learning: Hugo0 successfully refactored sessionStorage usage to React Context in the onramp flow, demonstrating preference for centralized state management over browser storage for component-shared state in React applications.
Applied to files:
src/hooks/useLogin.tsx
📚 Learning: 2024-10-29T16:06:38.812Z
Learnt from: jjramirezn
Repo: peanutprotocol/peanut-ui PR: 495
File: src/components/Create/useCreateLink.tsx:647-657
Timestamp: 2024-10-29T16:06:38.812Z
Learning: In the React code for `useCreateLink` in `src/components/Create/useCreateLink.tsx`, the `switchNetwork` function used within `useCallback` hooks is stable and does not need to be included in the dependency arrays.
Applied to files:
src/hooks/useLogin.tsx
📚 Learning: 2025-09-05T07:31:11.396Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1185
File: src/components/Claim/useClaimLink.tsx:14-0
Timestamp: 2025-09-05T07:31:11.396Z
Learning: In the peanut-ui codebase, `window.history.replaceState` is used instead of `router.replace` in URL manipulation functions when immediate/synchronous URL updates are required for subsequent actions to read the updated URL parameters.
Applied to files:
src/hooks/useLogin.tsx
📚 Learning: 2025-11-04T17:47:06.328Z
Learnt from: Zishan-7
Repo: peanutprotocol/peanut-ui PR: 1396
File: src/app/(mobile-ui)/home/page.tsx:295-304
Timestamp: 2025-11-04T17:47:06.328Z
Learning: In src/app/(mobile-ui)/home/page.tsx, when closing the KycCompletedModal, updateUserById is called without awaiting to provide instant feedback to the user. This fire-and-forget pattern for modal dismissals and UI preference updates is intentional and consistent across the codebase—user experience with instant UI feedback takes priority over waiting for backend sync operations.
Applied to files:
src/hooks/useLogin.tsx
📚 Learning: 2025-05-13T10:05:24.057Z
Learnt from: kushagrasarathe
Repo: peanutprotocol/peanut-ui PR: 845
File: src/components/Request/link/views/Create.request.link.view.tsx:81-81
Timestamp: 2025-05-13T10:05:24.057Z
Learning: In the peanut-ui project, pages that handle request flows (like Create.request.link.view.tsx) are only accessible to logged-in users who will always have a username, making null checks for user?.user.username unnecessary in these contexts.
Applied to files:
src/hooks/useLogin.tsx
🧬 Code graph analysis (1)
src/hooks/useLogin.tsx (1)
src/utils/general.utils.ts (2)
clearRedirectUrl(796-800)getValidRedirectUrl(899-915)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
- GitHub Check: Deploy-Preview
| if (redirect_uri) { | ||
| const validRedirectUrl = getValidRedirectUrl(redirect_uri, '/home') | ||
| router.push(validRedirectUrl) | ||
| window.location.assign(validRedirectUrl) | ||
| } else if (localStorageRedirect) { | ||
| clearRedirectUrl() | ||
| const validRedirectUrl = getValidRedirectUrl(String(localStorageRedirect), '/home') | ||
| router.push(validRedirectUrl) | ||
| window.location.assign(validRedirectUrl) | ||
| } else { | ||
| router.push('/home') | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
Inconsistent redirect methods break SPA experience and may degrade performance.
This change introduces full page reloads (window.location.assign) for external/stored redirects while maintaining client-side navigation (router.push) for the fallback home route. This inconsistency raises several concerns:
-
Performance & UX Impact: Full page reloads are significantly slower than client-side navigation and force users through a complete re-initialization (loading states, context setup, state reset).
-
State Loss: All React state, context, and in-memory data will be lost with
window.location.assign(), potentially disrupting user workflows or requiring data refetch. -
SPA Benefits: This breaks the single-page application experience that Next.js provides, which is one of its core advantages.
-
Inconsistency: If full page reloads are necessary for
redirect_uriandlocalStorageRedirectcases, why is the fallback home route still using client-side navigation? This suggests either:- The full reload is fixing a specific issue with external redirects (auth state, query params, etc.)
- Or there's an alternative solution that maintains SPA benefits
Based on the PR title "fix: Request pots redirect", this appears to be addressing a specific redirect issue. Please clarify:
- What specific issue does the full page reload solve?
- Have you considered alternatives like:
- Using
router.push()withrouter.refresh()to reload server state? - Clearing/resetting specific state before navigation?
- Using
window.history.replaceStateif URL synchronization is the concern (consistent with learnings)?
- Using
🏁 Script executed:
#!/bin/bash
# Verify if redirect URLs typically contain query parameters or external origins that might need fresh page loads
rg -n "redirect_uri|redirect.*=" --type=ts --type=tsx -C 2 | head -50Length of output: 27
---
🏁 Script executed:
```shell
#!/bin/bash
# Search for redirect_uri usage with correct syntax (no --type for tsx)
rg -n "redirect_uri" -C 3 | head -80
Length of output: 6228
🏁 Script executed:
#!/bin/bash
# Look at the useLogin.tsx file directly
cat src/hooks/useLogin.tsx | head -80Length of output: 1947
🏁 Script executed:
#!/bin/bash
# Search for getValidRedirectUrl function to understand validation logic
rg -n "getValidRedirectUrl" -C 5 | head -100Length of output: 5246
---
🏁 Script executed:
```shell
#!/bin/bash
# Get the full getValidRedirectUrl implementation
rg -n "export const getValidRedirectUrl" -A 20 src/utils/general.utils.ts
Length of output: 849
🏁 Script executed:
#!/bin/bash
# Check sanitizeRedirectURL for comparison
rg -n "sanitizeRedirectURL" -C 8 | head -100Length of output: 7487
🏁 Script executed:
#!/bin/bash
# Verify if there's a git history showing why this change was made
git log -p --all -S "window.location.assign" -- "src/hooks/useLogin.tsx" | head -150Length of output: 59
Replace window.location.assign() with router.push() for consistency and to preserve SPA experience.
The validated redirect URLs are same-origin only (enforced by sanitizeRedirectURL), so they don't require full page reloads. Other parts of the codebase handle identical validated redirects differently:
SetupPasskey.tsxline 62:router.push(validRedirectUrl)Landing.tsxline 30:push(sanitizedRedirectUrl)Welcome.tsxline 29:push(sanitizedRedirectUrl)
Using window.location.assign() unnecessarily reloads the entire page for internal navigation, destroying React state, context, and degrading perceived performance. This also creates an inconsistency within useLogin.tsx itself: line 42 correctly uses router.push('/home') for the fallback.
Change lines 36 and 40 from window.location.assign(validRedirectUrl) to router.push(validRedirectUrl) to match the established pattern. If server state refresh is needed after login, consider router.push() followed by router.refresh().
🤖 Prompt for AI Agents
In src/hooks/useLogin.tsx around lines 34 to 43, the code uses
window.location.assign(validRedirectUrl) on lines 36 and 40 which forces a full
page reload and breaks SPA behavior; replace both
window.location.assign(validRedirectUrl) calls with
router.push(validRedirectUrl) to perform client-side navigation and preserve
React state/consistency with other components, and if you need to refresh server
state after navigation consider calling router.refresh() after router.push().
No description provided.