Update to use getDataHref in fetchNextData (#14667)

This updates `fetchNextData` to re-use the `getDataHref` function from `page-loader` which has more verbose handling to ensure the correct `/_next/data` URL is built. Re-using this logic ensures the `/_next/data` URL can still be built even when a mismatching `href` and `as` value is provided to `next/link`.

This also fixes a case in `getDataHref` where optional values that weren't provided would fail to build the data href since the check requiring the param be present while interpolating the route values hasn't been updated to allow missing params for optional values.

An additional test case has been added to the prerender suite to ensure the `/_next/data` URL is built correctly when mismatching `href` and `as` values are provided

x-ref: #14536
x-ref: #9081 (reply in thread)
Closes: #14668

Author: ijjk

packages/next/client/page-loader.js

@@ -109,17 +109,18 @@ export default class PageLoader {
    * @param {string} href the route href (file-system path)
    * @param {string} asPath the URL as shown in browser (virtual path); used for dynamic routes
    */
-  getDataHref(href, asPath) {
+  getDataHref(href, asPath, ssg) {
+    const { pathname: hrefPathname, query, search } = parse(href, true)
+    const { pathname: asPathname } = parse(asPath)
+    const route = normalizeRoute(hrefPathname)
+
     const getHrefForSlug = (/** @type string */ path) => {
       const dataRoute = getAssetPathFromRoute(path, '.json')
-      return `${this.assetPrefix}/_next/data/${this.buildId}${dataRoute}`
+      return `${this.assetPrefix}/_next/data/${this.buildId}${dataRoute}${
+        ssg ? '' : search || ''
+      }`
     }
 
-    const { pathname: hrefPathname, query } = parse(href, true)
-    const { pathname: asPathname } = parse(asPath)
-
-    const route = normalizeRoute(hrefPathname)
-
     let isDynamic = isDynamicRoute(route),
       interpolatedRoute
     if (isDynamic) {
@@ -135,19 +136,19 @@ export default class PageLoader {
       interpolatedRoute = route
       if (
         !Object.keys(dynamicGroups).every((param) => {
-          let value = dynamicMatches[param]
+          let value = dynamicMatches[param] || ''
           const { repeat, optional } = dynamicGroups[param]
 
           // support single-level catch-all
           // TODO: more robust handling for user-error (passing `/`)
-          if (repeat && !Array.isArray(value)) value = [value]
           let replaced = `[${repeat ? '...' : ''}${param}]`
           if (optional) {
-            replaced = `[${replaced}]`
+            replaced = `${!value ? '/' : ''}[${replaced}]`
           }
+          if (repeat && !Array.isArray(value)) value = [value]
 
           return (
-            param in dynamicMatches &&
+            (optional || param in dynamicMatches) &&
             // Interpolate group into data URL if present
             (interpolatedRoute = interpolatedRoute.replace(
               replaced,
@@ -182,7 +183,7 @@ export default class PageLoader {
         // Check if the route requires a data file
         s.has(route) &&
         // Try to generate data href, noop when falsy
-        (_dataHref = this.getDataHref(href, asPath)) &&
+        (_dataHref = this.getDataHref(href, asPath, true)) &&
         // noop when data has already been prefetched (dedupe)
         !document.querySelector(
           `link[rel="${relPrefetch}"][href^="${_dataHref}"]`

packages/next/next-server/lib/router/router.ts

@@ -16,7 +16,6 @@ import { isDynamicRoute } from './utils/is-dynamic'
 import { getRouteMatcher } from './utils/route-matcher'
 import { getRouteRegex } from './utils/route-regex'
 import { normalizeTrailingSlash } from './normalize-trailing-slash'
-import getAssetPathFromRoute from './utils/get-asset-path-from-route'
 
 const basePath = (process.env.__NEXT_ROUTER_BASEPATH as string) || ''
 
@@ -108,39 +107,26 @@ type ComponentLoadCancel = (() => void) | null
 type HistoryMethod = 'replaceState' | 'pushState'
 
 function fetchNextData(
-  pathname: string,
-  query: ParsedUrlQuery | null,
+  dataHref: string,
   isServerRender: boolean,
   cb?: (...args: any) => any
 ) {
   let attempts = isServerRender ? 3 : 1
   function getResponse(): Promise<any> {
-    return fetch(
-      formatWithValidation({
-        pathname: addBasePath(
-          // @ts-ignore __NEXT_DATA__
-          `/_next/data/${__NEXT_DATA__.buildId}${getAssetPathFromRoute(
-            pathname,
-            '.json'
-          )}`
-        ),
-        query,
-      }),
-      {
-        // Cookies are required to be present for Next.js' SSG "Preview Mode".
-        // Cookies may also be required for `getServerSideProps`.
-        //
-        // > `fetch` won’t send cookies, unless you set the credentials init
-        // > option.
-        // https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch
-        //
-        // > For maximum browser compatibility when it comes to sending &
-        // > receiving cookies, always supply the `credentials: 'same-origin'`
-        // > option instead of relying on the default.
-        // https://github.com/github/fetch#caveats
-        credentials: 'same-origin',
-      }
-    ).then((res) => {
+    return fetch(dataHref, {
+      // Cookies are required to be present for Next.js' SSG "Preview Mode".
+      // Cookies may also be required for `getServerSideProps`.
+      //
+      // > `fetch` won’t send cookies, unless you set the credentials init
+      // > option.
+      // https://developer.mozilla.org/en-US/docs/Web/API/Fetch_API/Using_Fetch
+      //
+      // > For maximum browser compatibility when it comes to sending &
+      // > receiving cookies, always supply the `credentials: 'same-origin'`
+      // > option instead of relying on the default.
+      // https://github.com/github/fetch#caveats
+      credentials: 'same-origin',
+    }).then((res) => {
       if (!res.ok) {
         if (--attempts > 0 && res.status >= 500) {
           return getResponse()
@@ -669,11 +655,21 @@ export default class Router implements BaseRouter {
           }
         }
 
+        let dataHref: string | undefined
+
+        if (__N_SSG || __N_SSP) {
+          dataHref = this.pageLoader.getDataHref(
+            formatWithValidation({ pathname, query }),
+            as,
+            __N_SSG
+          )
+        }
+
         return this._getData<RouteInfo>(() =>
           __N_SSG
-            ? this._getStaticData(as)
+            ? this._getStaticData(dataHref!)
             : __N_SSP
-            ? this._getServerData(as)
+            ? this._getServerData(dataHref!)
             : this.getInitialProps(
                 Component,
                 // we provide AppTree later so this needs to be `any`
@@ -843,23 +839,20 @@ export default class Router implements BaseRouter {
     })
   }
 
-  _getStaticData = (asPath: string): Promise<object> => {
-    const pathname = prepareRoute(parse(asPath).pathname!)
+  _getStaticData = (dataHref: string): Promise<object> => {
+    const pathname = prepareRoute(parse(dataHref).pathname!)
 
     return process.env.NODE_ENV === 'production' && this.sdc[pathname]
-      ? Promise.resolve(this.sdc[pathname])
+      ? Promise.resolve(this.sdc[dataHref])
       : fetchNextData(
-          pathname,
-          null,
+          dataHref,
           this.isSsr,
           (data) => (this.sdc[pathname] = data)
         )
   }
 
-  _getServerData = (asPath: string): Promise<object> => {
-    let { pathname, query } = parse(asPath, true)
-    pathname = prepareRoute(pathname!)
-    return fetchNextData(pathname, query, this.isSsr)
+  _getServerData = (dataHref: string): Promise<object> => {
+    return fetchNextData(dataHref, this.isSsr)
   }
 
   getInitialProps(

test/integration/build-output/test/index.test.js

@@ -113,7 +113,7 @@ describe('Build Output', () => {
       expect(parseFloat(webpackSize) - 775).toBeLessThanOrEqual(0)
       expect(webpackSize.endsWith('B')).toBe(true)
 
-      expect(parseFloat(mainSize) - 6.3).toBeLessThanOrEqual(0)
+      expect(parseFloat(mainSize) - 6.4).toBeLessThanOrEqual(0)
       expect(mainSize.endsWith('kB')).toBe(true)
 
       expect(parseFloat(frameworkSize) - 41).toBeLessThanOrEqual(0)

test/integration/prerender/pages/index.js

@@ -52,6 +52,10 @@ const Page = ({ world, time }) => {
         <a id="to-nested-index">to nested index</a>
       </Link>
       <br />
+      <Link href="/lang/[lang]/about?lang=en" as="/about">
+        <a id="to-rewritten-ssg">to rewritten static path page</a>
+      </Link>
+      <br />
       <Link href="/catchall-optional/[[...slug]]" as="/catchall-optional">
         <a id="catchall-optional-root">to optional catchall root</a>
       </Link>

test/integration/prerender/pages/lang/[lang]/about.js

@@ -1,4 +1,4 @@
-export default ({ lang }) => <p>About: {lang}</p>
+export default ({ lang }) => <p id="about">About: {lang}</p>
 
 export const getStaticProps = ({ params: { lang } }) => ({
   props: {

test/integration/prerender/test/index.test.js

@@ -25,6 +25,7 @@ import {
 } from 'next-test-utils'
 import webdriver from 'next-webdriver'
 import { dirname, join } from 'path'
+import url from 'url'
 
 jest.setTimeout(1000 * 60 * 2)
 const appDir = join(__dirname, '..')
@@ -601,6 +602,45 @@ const runTests = (dev = false, isEmulatedServerless = false) => {
       const html = await renderViaHTTP(appPort, '/about')
       expect(html).toMatch(/About:.*?en/)
     })
+
+    it('should fetch /_next/data correctly with mismatched href and as', async () => {
+      const browser = await webdriver(appPort, '/')
+
+      if (!dev) {
+        await browser.eval(() =>
+          document.querySelector('#to-rewritten-ssg').scrollIntoView()
+        )
+
+        await check(
+          async () => {
+            const links = await browser.elementsByCss('link[rel=prefetch]')
+            let found = false
+
+            for (const link of links) {
+              const href = await link.getAttribute('href')
+              const { pathname } = url.parse(href)
+
+              if (pathname.endsWith('/lang/en/about.json')) {
+                found = true
+                break
+              }
+            }
+            return found
+          },
+          {
+            test(result) {
+              return result === true
+            },
+          }
+        )
+      }
+      await browser.eval('window.beforeNav = "hi"')
+      await browser.elementByCss('#to-rewritten-ssg').click()
+      await browser.waitForElementByCss('#about')
+
+      expect(await browser.eval('window.beforeNav')).toBe('hi')
+      expect(await browser.elementByCss('#about').text()).toBe('About: en')
+    })
   }
 
   if (dev) {