app router: fix double fetch on prefetch={false} (#51292)

This PR fixes a few reports that we were double fetching when navigating via a link that had prefetch false. ## Context The bug was happening because we were inadvertently eagerly fetching even if we potentially bailed out of the optimistic navigation, which would then trigger another fetch from going through the regular navigation path. There's potentially another bug here where we should potentially not bail out of optimistic navigation in the cases reported but we can fix that later. fix #49844 link NEXT-1287
vercel · Jun 14, 2023 · c6c4a3d · c6c4a3d
1 parent cbeecba
commit c6c4a3d
Show file tree

Hide file tree

Showing 8 changed files with 102 additions and 4 deletions.
diff --git a/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts b/packages/next/src/client/components/router-reducer/reducers/navigate-reducer.ts
@@ -157,9 +157,16 @@ export function navigateReducer(
     temporaryCacheNode.subTreeData = state.cache.subTreeData
     temporaryCacheNode.parallelRoutes = new Map(state.cache.parallelRoutes)
 
-    const data = createRecordFromThenable(
-      fetchServerResponse(url, optimisticTree, state.nextUrl, state.buildId)
-    )
+    let data: ReturnType<typeof createRecordFromThenable> | undefined
+
+    const fetchResponse = () => {
+      if (!data) {
+        data = createRecordFromThenable(
+          fetchServerResponse(url, optimisticTree, state.nextUrl, state.buildId)
+        )
+      }
+      return data
+    }
 
     // TODO-APP: segments.slice(1) strips '', we can get rid of '' altogether.
     // TODO-APP: re-evaluate if we need to strip the last segment
@@ -174,7 +181,7 @@ export function navigateReducer(
       temporaryCacheNode,
       state.cache,
       optimisticFlightSegmentPath,
-      () => data,
+      fetchResponse,
       true
     )
 

diff --git a/test/e2e/app-dir/app-prefetch-false/app-prefetch-false.test.ts b/test/e2e/app-dir/app-prefetch-false/app-prefetch-false.test.ts
@@ -0,0 +1,51 @@
+import { createNextDescribe } from 'e2e-utils'
+
+const getPathname = (url: string) => {
+  const urlObj = new URL(url)
+  return urlObj.pathname
+}
+
+const createRequestsListener = async (browser: BrowserInterface) => {
+  // wait for network idle
+  await browser.waitForIdleNetwork()
+
+  let requests = []
+
+  browser.on('request', (req: Request) => {
+    requests.push([req.url(), !!req.headers()['next-router-prefetch']])
+  })
+
+  await browser.refresh()
+
+  return {
+    getRequests: () => requests,
+    clearRequests: () => {
+      requests = []
+    },
+  }
+}
+
+createNextDescribe(
+  'app-prefetch-false',
+  {
+    files: __dirname,
+  },
+  ({ next, isNextDev }) => {
+    if (isNextDev) {
+      it.skip('should skip test in dev mode', () => {})
+    } else {
+      it('should avoid double-fetching when optimistic navigation fails', async () => {
+        const browser = await next.browser('/foo')
+        const { getRequests } = await createRequestsListener(browser)
+
+        await browser.elementByCss('[href="/foo"]').click()
+        await browser.elementByCss('[href="/foo/bar"]').click()
+        console.log('getRequests()', getRequests())
+        expect(
+          getRequests().filter(([req]) => getPathname(req) === '/foo/bar')
+            .length
+        ).toBe(1)
+      })
+    }
+  }
+)
diff --git a/test/e2e/app-dir/app-prefetch-false/app/[slugA]/[slugB]/page.tsx b/test/e2e/app-dir/app-prefetch-false/app/[slugA]/[slugB]/page.tsx
@@ -0,0 +1,3 @@
+export default function Page() {
+  return <p>hello world</p>
+}
diff --git a/test/e2e/app-dir/app-prefetch-false/app/[slugA]/layout.tsx b/test/e2e/app-dir/app-prefetch-false/app/[slugA]/layout.tsx
@@ -0,0 +1,3 @@
+export default function Layout({ children }) {
+  return children
+}
diff --git a/test/e2e/app-dir/app-prefetch-false/app/[slugA]/page.tsx b/test/e2e/app-dir/app-prefetch-false/app/[slugA]/page.tsx
@@ -0,0 +1,3 @@
+export default function Page() {
+  return <p>hello world</p>
+}
diff --git a/test/e2e/app-dir/app-prefetch-false/app/layout.tsx b/test/e2e/app-dir/app-prefetch-false/app/layout.tsx
@@ -0,0 +1,22 @@
+import Link from 'next/link'
+import React from 'react'
+
+export default function Root({ children }: { children: React.ReactNode }) {
+  return (
+    <html>
+      <body>
+        <div>
+          <Link prefetch={false} href="/foo">
+            foo
+          </Link>
+        </div>
+        <div>
+          <Link prefetch={false} href="/foo/bar">
+            foo/bar
+          </Link>
+        </div>
+        {children}
+      </body>
+    </html>
+  )
+}
diff --git a/test/e2e/app-dir/app-prefetch-false/app/page.tsx b/test/e2e/app-dir/app-prefetch-false/app/page.tsx
@@ -0,0 +1,3 @@
+export default function Page() {
+  return <p>hello world</p>
+}
diff --git a/test/e2e/app-dir/app-prefetch-false/next.config.js b/test/e2e/app-dir/app-prefetch-false/next.config.js
@@ -0,0 +1,6 @@
+/**
+ * @type {import('next').NextConfig}
+ */
+const nextConfig = {}
+
+module.exports = nextConfig