-
Notifications
You must be signed in to change notification settings - Fork 26.3k
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
app router: Fix infinite redirect loop in MPA navigation (#49058)
Not 100% convinced that this is the correct fix but it's the best I can think of. Previously, we would sometimes call location.assign()/.replace() hundreds of times (or more) as I described in #48438 and #48309 (comment). Sometimes this would just make things slow but the navigation would eventually succeed; sometimes this would just hang the browser. Now we trigger it only once (or—for a reason I don't quite understand—twice in dev, as you can see in the test) and never commit that render. This also fixes the bug I mentioned in #48438 (comment) where usePathname() and useSearchParams() would return the page we are navigating to (even if that's an external page wholly unrelated to our site!). Fixes #48309, fixes #48438. link NEXT-1028 Co-authored-by: Jimmy Lai <laijimmy0@gmail.com>
- Loading branch information
1 parent
931c101
commit 05cd515
Showing
4 changed files
with
207 additions
and
12 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
71 changes: 71 additions & 0 deletions
71
test/e2e/app-dir/navigation/app/external-push/[storageKey]/page.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,71 @@ | ||
| /*global navigation*/ | ||
| 'use client' | ||
|
|
||
| import { usePathname, useRouter, useSearchParams } from 'next/navigation' | ||
| import { useEffect, useTransition } from 'react' | ||
|
|
||
| let listening = false | ||
| let startedNavigating = false | ||
|
|
||
| export default function Page({ params: { storageKey } }) { | ||
| if (typeof window === 'undefined') { | ||
| throw new Error('Client render only') | ||
| } | ||
|
|
||
| let router = useRouter() | ||
| let path = usePathname() | ||
| let searchParams = useSearchParams().toString() | ||
| if (searchParams) { | ||
| path += `?${searchParams}` | ||
| } | ||
|
|
||
| // Log every pathname+searchParams we've seen | ||
| sessionStorage.setItem(`${storageKey}/path-${path}`, 'true') | ||
|
|
||
| let [isPending, startTransition] = useTransition() | ||
| useEffect(() => { | ||
| if (startedNavigating) { | ||
| sessionStorage.setItem(`${storageKey}/lastIsPending`, isPending) | ||
| } | ||
| }) | ||
|
|
||
| // Read all matching logs and print them | ||
| let storage = Object.fromEntries( | ||
| Object.entries(sessionStorage).flatMap(([key, value]) => | ||
| key.startsWith(`${storageKey}/`) | ||
| ? [[key.slice(storageKey.length + 1), value]] | ||
| : [] | ||
| ) | ||
| ) | ||
|
|
||
| return ( | ||
| <> | ||
| <button | ||
| id="go" | ||
| onClick={() => { | ||
| // Count number of navigations triggered (in browsers that support it) | ||
| sessionStorage.setItem( | ||
| `${storageKey}/navigation-supported`, | ||
| typeof navigation !== 'undefined' | ||
| ) | ||
| if (!listening && typeof navigation !== 'undefined') { | ||
| listening = true | ||
| navigation.addEventListener('navigate', (event) => { | ||
| let key = `${storageKey}/navigate-${event.destination.url}` | ||
| let count = +sessionStorage.getItem(key) | ||
| sessionStorage.setItem(key, count + 1) | ||
| }) | ||
| } | ||
|
|
||
| startedNavigating = true | ||
| startTransition(() => { | ||
| router.push('https://example.vercel.sh/stuff?abc=123') | ||
| }) | ||
| }} | ||
| > | ||
| go | ||
| </button> | ||
| <pre id="storage">{JSON.stringify(storage, null, 2)}</pre> | ||
| </> | ||
| ) | ||
| } |
45 changes: 45 additions & 0 deletions
45
test/e2e/app-dir/navigation/app/redirect/external-log/[storageKey]/page.js
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,45 @@ | ||
| /*global navigation*/ | ||
| 'use client' | ||
|
|
||
| import { redirect, useSearchParams } from 'next/navigation' | ||
|
|
||
| let listening = false | ||
|
|
||
| export default function Page({ params: { storageKey } }) { | ||
| if (typeof window === 'undefined') { | ||
| throw new Error('Client render only') | ||
| } | ||
|
|
||
| let searchParams = useSearchParams() | ||
|
|
||
| if (searchParams.get('read')) { | ||
| // Read all matching logs and print them | ||
| let storage = Object.fromEntries( | ||
| Object.entries(sessionStorage).flatMap(([key, value]) => | ||
| key.startsWith(`${storageKey}/`) | ||
| ? [[key.slice(storageKey.length + 1), value]] | ||
| : [] | ||
| ) | ||
| ) | ||
|
|
||
| return <pre id="storage">{JSON.stringify(storage, null, 2)}</pre> | ||
| } else { | ||
| // Count number of navigations triggered (in browsers that support it) | ||
| sessionStorage.setItem( | ||
| `${storageKey}/navigation-supported`, | ||
| typeof navigation !== 'undefined' | ||
| ) | ||
| if (!listening && typeof navigation !== 'undefined') { | ||
| listening = true | ||
| navigation.addEventListener('navigate', (event) => { | ||
| if (!event.destination.sameDocument) { | ||
| let key = `${storageKey}/navigate-${event.destination.url}` | ||
| let count = +sessionStorage.getItem(key) | ||
| sessionStorage.setItem(key, count + 1) | ||
| } | ||
| }) | ||
| } | ||
|
|
||
| redirect('https://example.vercel.sh/') | ||
| } | ||
| } |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters