Fix static data fetching when using absolute assetprefix (#15287)

Fixes https://github.com/vercel/next.js/issues/15188

`parseRelativeUrl` was used on urls that weren't always relative. It was used to generate a cache key, but we actually don't need these cache keys to be relative if the urls aren't relative.

Also took a look at the overall static data fetching logic and found a few things:

- [x] cache key is unnecessarily transformed through `prepareRoute`, we can just cache by resolved `dataHref` and remove that function. Pretty sure that `prepareRoute` was also introducing edge cases with `assetPath` and `delBasePath`
- [x] there is [a bug in the caching logic](ebdfa2e7a3/packages/next/next-server/lib/router/router.ts (L898)) that made it fail on the second visit: it should be `Promise.resolve(this.sdc[pathname])` instead of `Promise.resolve(this.sdc[dataHref])`. Also added a test for this
- [x] ~converted to async await to improve stacktraces and readability.~ I assumed this was fine since I saw some async/awaits in that file already but it seems to just blow up the size of the non-modern bundle.
- [x] extracted nested `getResponse` function and define it top level. this should improve runtime performance
- [x] convert `_getStaticData` and `_getServerData` to class methods instead of properties. Not sure why they were defined as properties but I think they belong on the prototype instead.
- [x] remove `cb` property from `fetchNextData`, it's unnecessary and makes the async flow hard to understand.  The exact same logic can go in the `.then` instead.
- [ ] data fetching logic [retries on 5xx errors](ebdfa2e7a3/packages/next/next-server/lib/router/router.ts (L157)), but not on network level errors. It should also retry on those. It should also not retry on every 5xx, probably only makes sense on 502, 503 and 504. (e.g. 500 is a server error that I wouldn't expect to succeed on a retry)

The overall result also is a few bytes smaller in size

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

@@ -36,10 +36,6 @@ export function delBasePath(path: string): string {
   return path.slice(basePath.length) || '/'
 }
 
-function prepareRoute(path: string) {
-  return removePathTrailingSlash(delBasePath(path || '/'))
-}
-
 type Url = UrlObject | string
 
 /**
@@ -132,50 +128,42 @@ const manualScrollRestoration =
   typeof window !== 'undefined' &&
   'scrollRestoration' in window.history
 
-function fetchNextData(
-  dataHref: string,
-  isServerRender: boolean,
-  cb?: (...args: any) => any
-) {
-  let attempts = isServerRender ? 3 : 1
-  function getResponse(): Promise<any> {
-    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()
-        }
-        throw new Error(`Failed to load static props`)
+function fetchRetry(url: string, attempts: number): Promise<any> {
+  return fetch(url, {
+    // 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 > 1 && res.status >= 500) {
+        return fetchRetry(url, attempts - 1)
       }
-      return res.json()
-    })
-  }
+      throw new Error(`Failed to load static props`)
+    }
 
-  return getResponse()
-    .then((data) => {
-      return cb ? cb(data) : data
-    })
-    .catch((err: Error) => {
-      // We should only trigger a server-side transition if this was caused
-      // on a client-side transition. Otherwise, we'd get into an infinite
-      // loop.
-      if (!isServerRender) {
-        ;(err as any).code = 'PAGE_LOAD_ERROR'
-      }
-      throw err
-    })
+    return res.json()
+  })
+}
+
+function fetchNextData(dataHref: string, isServerRender: boolean) {
+  return fetchRetry(dataHref, isServerRender ? 3 : 1).catch((err: Error) => {
+    // We should only trigger a server-side transition if this was caused
+    // on a client-side transition. Otherwise, we'd get into an infinite
+    // loop.
+    if (!isServerRender) {
+      ;(err as any).code = 'PAGE_LOAD_ERROR'
+    }
+    throw err
+  })
 }
 
 export default class Router implements BaseRouter {
@@ -890,20 +878,18 @@ export default class Router implements BaseRouter {
     })
   }
 
-  _getStaticData = (dataHref: string): Promise<object> => {
-    let { pathname } = parseRelativeUrl(dataHref)
-    pathname = prepareRoute(pathname)
-
-    return process.env.NODE_ENV === 'production' && this.sdc[pathname]
-      ? Promise.resolve(this.sdc[dataHref])
-      : fetchNextData(
-          dataHref,
-          this.isSsr,
-          (data) => (this.sdc[pathname] = data)
-        )
+  _getStaticData(dataHref: string): Promise<object> {
+    const { href: cacheKey } = new URL(dataHref, window.location.href)
+    if (process.env.NODE_ENV === 'production' && this.sdc[cacheKey]) {
+      return Promise.resolve(this.sdc[cacheKey])
+    }
+    return fetchNextData(dataHref, this.isSsr).then((data) => {
+      this.sdc[cacheKey] = data
+      return data
+    })
   }
 
-  _getServerData = (dataHref: string): Promise<object> => {
+  _getServerData(dataHref: string): Promise<object> {
     return fetchNextData(dataHref, this.isSsr)
   }
 

test/integration/absolute-assetprefix/next.config.js

@@ -0,0 +1,3 @@
+module.exports = {
+  assetPrefix: 'http://localhost:__CDN_PORT__/path-prefix',
+}

test/integration/absolute-assetprefix/pages/about.js

@@ -0,0 +1,11 @@
+export async function getStaticProps() {
+  return { props: { prop: 'hello' } }
+}
+
+export default function About({ prop }) {
+  return (
+    <>
+      about <div id="prop">{prop}</div>
+    </>
+  )
+}

test/integration/absolute-assetprefix/pages/index.js

@@ -0,0 +1,11 @@
+import Link from 'next/link'
+
+export default function Page() {
+  return (
+    <>
+      <Link href="/about">
+        <a id="about-link">Click</a>
+      </Link>
+    </>
+  )
+}

test/integration/absolute-assetprefix/test/index.test.js

@@ -0,0 +1,74 @@
+/* eslint-env jest */
+
+import { findPort, killApp, nextBuild, nextStart, File } from 'next-test-utils'
+import * as http from 'http'
+import * as path from 'path'
+import webdriver from 'next-webdriver'
+import { join } from 'path'
+
+jest.setTimeout(1000 * 60 * 1)
+
+const appDir = join(__dirname, '..')
+
+let appPort
+let cdnPort
+let app
+let cdn
+
+const nextConfig = new File(path.resolve(__dirname, '../next.config.js'))
+
+describe('absolute assetPrefix with path prefix', () => {
+  beforeAll(async () => {
+    cdnPort = await findPort()
+    // lightweight http proxy
+    cdn = http.createServer((clientReq, clientRes) => {
+      const proxy = http.request(
+        {
+          hostname: 'localhost',
+          port: appPort,
+          path: clientReq.url.slice('/path-prefix'.length),
+          method: clientReq.method,
+          headers: clientReq.headers,
+        },
+        (res) => {
+          // cdn must be configured to allow requests from this origin
+          res.headers[
+            'Access-Control-Allow-Origin'
+          ] = `http://localhost:${appPort}`
+          clientRes.writeHead(res.statusCode, res.headers)
+          res.pipe(clientRes, { end: true })
+        }
+      )
+
+      clientReq.pipe(proxy, { end: true })
+    })
+    await new Promise((resolve) => cdn.listen(cdnPort, resolve))
+    nextConfig.replace('__CDN_PORT__', cdnPort)
+    await nextBuild(appDir)
+    appPort = await findPort()
+    app = await nextStart(appDir, appPort)
+  })
+
+  afterAll(() => killApp(app))
+  afterAll(() => cdn.close())
+  afterAll(() => nextConfig.restore())
+
+  it('should be able to fetch static data from a CDN', async () => {
+    const browser = await webdriver(appPort, '/')
+    await browser.waitForElementByCss('#about-link').click()
+    const prop = await browser.waitForElementByCss('#prop').text()
+    expect(prop).toBe('hello')
+  })
+
+  it('should fetch from cache correctly', async () => {
+    const browser = await webdriver(appPort, '/')
+    await browser.eval('window.clientSideNavigated = true')
+    await browser.waitForElementByCss('#about-link').click()
+    await browser.waitForElementByCss('#prop')
+    await browser.back()
+    await browser.waitForElementByCss('#about-link').click()
+    const prop = await browser.waitForElementByCss('#prop').text()
+    expect(prop).toBe('hello')
+    expect(await browser.eval('window.clientSideNavigated')).toBe(true)
+  })
+})