fix: prevent incorrect searchParams being applied on certain navs (#76914)

When the router returns an aliased prefetch for URLs to the same path
but with different search params, there's the possibility that we
trigger an MPA-style navigation if the path doesn't contain a valid
flight payload (eg: a route handler, a `pages` router page, or an
external URL). The logic to bail out of the alias handling was happening
_after_ this, which meant the incorrect searchParams would be applied to
the final URL.

This ensures that if we bail out of applying the alias (eg because it's
not a valid RSC payload, or the aliased entry has no reusable loading
segment), then we re-run the navigation with a non-aliased entry.

Fixes #75318
Closes NDX-804

Author: ztanner

packages/next/src/client/components/router-reducer/aliased-prefetch-navigations.ts

@@ -25,7 +25,7 @@ import type { Mutable, ReadonlyReducerState } from './router-reducer-types'
  */
 export function handleAliasedPrefetchEntry(
   state: ReadonlyReducerState,
-  flightData: NormalizedFlightData[],
+  flightData: string | NormalizedFlightData[],
   url: URL,
   mutable: Mutable
 ) {
@@ -34,6 +34,10 @@ export function handleAliasedPrefetchEntry(
   const href = createHrefFromUrl(url)
   let applied
 
+  if (typeof flightData === 'string') {
+    return false
+  }
+
   for (const normalizedFlightData of flightData) {
     // If the segment doesn't have a loading component, we don't need to do anything.
     if (!hasLoadingComponentInSeedData(normalizedFlightData.seedData)) {

packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts

@@ -234,6 +234,24 @@ export function navigateReducer(
         isFirstRead = true
       }
 
+      if (prefetchValues.aliased) {
+        const result = handleAliasedPrefetchEntry(
+          state,
+          flightData,
+          url,
+          mutable
+        )
+
+        // We didn't return new router state because we didn't apply the aliased entry for some reason.
+        // We'll re-invoke the navigation handler but ensure that we don't attempt to use the aliased entry. This
+        // will create an on-demand prefetch entry.
+        if (result === false) {
+          return navigateReducer(state, { ...action, allowAliasing: false })
+        }
+
+        return result
+      }
+
       // Handle case when navigating to page in `pages` from `app`
       if (typeof flightData === 'string') {
         return handleExternalUrl(state, mutable, flightData, pendingPush)
@@ -259,24 +277,6 @@ export function navigateReducer(
         return handleMutable(state, mutable)
       }
 
-      if (prefetchValues.aliased) {
-        const result = handleAliasedPrefetchEntry(
-          state,
-          flightData,
-          url,
-          mutable
-        )
-
-        // We didn't return new router state because we didn't apply the aliased entry for some reason.
-        // We'll re-invoke the navigation handler but ensure that we don't attempt to use the aliased entry. This
-        // will create an on-demand prefetch entry.
-        if (result === false) {
-          return navigateReducer(state, { ...action, allowAliasing: false })
-        }
-
-        return result
-      }
-
       let currentTree = state.tree
       let currentCache = state.cache
       let scrollableSegments: FlightSegmentPath[] = []

test/e2e/app-dir/searchparams-reuse-loading/app/mpa-navs/page.tsx

@@ -0,0 +1,26 @@
+import Link from 'next/link'
+
+export default function Page() {
+  return (
+    <ul>
+      <li>
+        <Link href="/non-existent-page?id=1">/non-existent-page?id=1</Link>
+      </li>
+      <li>
+        <Link href="/non-existent-page?id=2">/non-existent-page?id=2</Link>
+      </li>
+      <li>
+        <Link href="/pages-dir?param=1">/pages-dir?param=1</Link>
+      </li>
+      <li>
+        <Link href="/pages-dir?param=2">/pages-dir?param=2</Link>
+      </li>
+      <li>
+        <Link href="/route-handler?param=1">/route-handler?param=1</Link>
+      </li>
+      <li>
+        <Link href="/route-handler?param=2">/route-handler?param=2</Link>
+      </li>
+    </ul>
+  )
+}

test/e2e/app-dir/searchparams-reuse-loading/app/route-handler/route.ts

@@ -0,0 +1,6 @@
+import { NextRequest, NextResponse } from 'next/server'
+
+export async function GET(req: NextRequest) {
+  const param = req.nextUrl.searchParams.get('param') as string
+  return NextResponse.json({ param })
+}

test/e2e/app-dir/searchparams-reuse-loading/app/search/layout.tsx

@@ -9,5 +9,5 @@ export default function SearchLayout({
   children: React.ReactNode
 }) {
   let searchParams = useSearchParams()
-  return <Fragment key={searchParams.get('q')}>{children}</Fragment>
+  return <Fragment key={searchParams?.get('q')}>{children}</Fragment>
 }

test/e2e/app-dir/searchparams-reuse-loading/pages/pages-dir.tsx

@@ -0,0 +1,6 @@
+import { useRouter } from 'next/router'
+
+export default function Page() {
+  const router = useRouter()
+  return <div>Hello from pages dir! {router.query.param}</div>
+}

test/e2e/app-dir/searchparams-reuse-loading/searchparams-reuse-loading.test.ts

@@ -87,6 +87,75 @@ describe('searchparams-reuse-loading', () => {
     )
   })
 
+  describe('when aliasing is skipped', () => {
+    it('should work for not found pages', async () => {
+      const browser = await next.browser('/mpa-navs')
+      await browser.elementByCss("[href='/non-existent-page?id=1']").click()
+
+      // the first link would have been the "aliased" entry since it was prefetched first. Validate that it's the correct URL
+      await retry(async () => {
+        expect(await browser.url()).toContain('/non-existent-page?id=1')
+      })
+
+      expect(await browser.elementByCss('h2').text()).toBe(
+        'This page could not be found.'
+      )
+
+      // The other link would have attempted to use the aliased entry. Ensure the browser ends up on the correct page
+      await browser.loadPage(`${next.url}/mpa-navs`)
+      await retry(async () => {
+        expect(await browser.url()).toContain('/mpa-navs')
+      })
+      await browser.elementByCss("[href='/non-existent-page?id=2']").click()
+      await retry(async () => {
+        expect(await browser.url()).toContain('/non-existent-page?id=2')
+      })
+      expect(await browser.elementByCss('h2').text()).toBe(
+        'This page could not be found.'
+      )
+    })
+
+    it('should work for route handlers', async () => {
+      const browser = await next.browser('/mpa-navs')
+      await browser.elementByCss("[href='/route-handler?param=1']").click()
+      await retry(async () => {
+        expect(await browser.url()).toContain('/route-handler?param=1')
+      })
+
+      await browser.loadPage(`${next.url}/mpa-navs`)
+      await retry(async () => {
+        expect(await browser.url()).toContain('/mpa-navs')
+      })
+
+      await browser.elementByCss("[href='/route-handler?param=2']").click()
+      await retry(async () => {
+        expect(await browser.url()).toContain('/route-handler?param=2')
+      })
+    })
+
+    it('should work for navigating to pages dir', async () => {
+      const browser = await next.browser('/mpa-navs')
+      await browser.elementByCss("[href='/pages-dir?param=1']").click()
+
+      await retry(async () => {
+        expect(await browser.elementByCss('body').text()).toContain(
+          'Hello from pages dir! 1'
+        )
+        expect(await browser.url()).toContain('/pages-dir?param=1')
+      })
+
+      await browser.loadPage(`${next.url}/mpa-navs`)
+
+      await browser.elementByCss("[href='/pages-dir?param=2']").click()
+      await retry(async () => {
+        expect(await browser.elementByCss('body').text()).toContain(
+          'Hello from pages dir! 2'
+        )
+        expect(await browser.url()).toContain('/pages-dir?param=2')
+      })
+    })
+  })
+
   // Dev doesn't perform prefetching, so this test is skipped, as it relies on intercepting
   // prefetch network requests.
   if (!isNextDev) {