Skip to content

[TASK-16676] fix: Request pots redirect #1420

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

Merged
Hugo0 merged 2 commits into from
Nov 7, 2025
Merged

[TASK-16676] fix: Request pots redirect #1420

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
865cc9e
fix: Request pots redirect
Zishan-7 Nov 7, 2025
7602eb2
Update request pot button text
Zishan-7 Nov 7, 2025
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
2 changes: 1 addition & 1 deletion src/components/Payment/PaymentForm/index.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -540,7 +540,7 @@ export const PaymentForm = ({
}

if (showRequestPotInitialView) {
return 'Pay'
return 'Choose payment method'
}

if (isActivePeanutWallet && isInsufficientBalanceError && !isExternalWalletFlow) {
Expand Down
4 changes: 2 additions & 2 deletions src/hooks/useLogin.tsx
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,11 +33,11 @@ export const useLogin = () => {
const redirect_uri = searchParams.get('redirect_uri')
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')
}
Comment on lines 34 to 43
Copy link
Copy Markdown
Contributor

@coderabbitai coderabbitai bot Nov 7, 2025

Choose a reason for hiding this comment

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

⚠️ Potential issue | 🔴 Critical

🧩 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:

  1. 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).

  2. 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.

  3. SPA Benefits: This breaks the single-page application experience that Next.js provides, which is one of its core advantages.

  4. Inconsistency: If full page reloads are necessary for redirect_uri and localStorageRedirect cases, 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() with router.refresh() to reload server state?
    • Clearing/resetting specific state before navigation?
    • Using window.history.replaceState if URL synchronization is the concern (consistent with learnings)?

🏁 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 -50

Length 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 -80

Length of output: 1947

🏁 Script executed:

#!/bin/bash
# Search for getValidRedirectUrl function to understand validation logic
rg -n "getValidRedirectUrl" -C 5 | head -100

Length 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 -100

Length 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 -150

Length 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.tsx line 62: router.push(validRedirectUrl)
  • Landing.tsx line 30: push(sanitizedRedirectUrl)
  • Welcome.tsx line 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().

Expand Down
Loading