Normalize asPath for GS(S)P pages (#17081)

This normalizes the `asPath` for `getServerSideProps` and `getStaticProps` pages to ensure it matches the value that would show on the client instead of a) the output pathname when revalidating or generating a fallback or b) the `_next/data` URL on client transition. 

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

packages/next/build/webpack/loaders/next-serverless-loader.ts

@@ -299,6 +299,7 @@ const nextServerlessLoader: loader.Loader = function () {
     const {parse: parseQs} = require('querystring')
     const { renderToHTML } = require('next/dist/next-server/server/render');
     const { tryGetPreviewData } = require('next/dist/next-server/server/api-utils');
+    const { denormalizePagePath } = require('next/dist/next-server/server/denormalize-page-path')
     const {sendPayload} = require('next/dist/next-server/server/send-payload');
     const buildManifest = require('${buildManifest}');
     const reactLoadableManifest = require('${reactLoadableManifest}');
@@ -465,8 +466,8 @@ const nextServerlessLoader: loader.Loader = function () {
             : ''
         }
 
-        // normalize request URL/asPath for fallback pages since the proxy
-        // sets the request URL to the output's path for fallback pages
+        // normalize request URL/asPath for fallback/revalidate pages since the
+        // proxy sets the request URL to the output's path for fallback pages
         ${
           pageIsDynamicRoute
             ? `
@@ -482,12 +483,44 @@ const nextServerlessLoader: loader.Loader = function () {
                     _parsedUrl.pathname.substr(paramIdx + param.length + 2)
                 }
               }
+              parsedUrl.pathname = _parsedUrl.pathname
               req.url = formatUrl(_parsedUrl)
             }
           `
             : ``
         }
 
+        // make sure to normalize asPath for revalidate and _next/data requests
+        // since the asPath should match what is shown on the client
+        if (
+          !fromExport &&
+          (getStaticProps || getServerSideProps)
+        ) {
+          const curQuery = {...parsedUrl.query}
+
+          ${
+            pageIsDynamicRoute
+              ? `
+              // don't include dynamic route params in query while normalizing
+              // asPath
+              if (trustQuery) {
+                delete parsedUrl.search
+
+                for (const param of Object.keys(defaultRouteRegex.groups)) {
+                  delete curQuery[param]
+                }
+              }
+            `
+              : ``
+          }
+
+          parsedUrl.pathname = denormalizePagePath(parsedUrl.pathname)
+          renderOpts.normalizedAsPath = formatUrl({
+            ...parsedUrl,
+            query: curQuery
+          })
+        }
+
         const isFallback = parsedUrl.query.__nextFallback
 
         const previewData = tryGetPreviewData(req, res, options.previewProps)

packages/next/next-server/server/next-server.ts

@@ -64,6 +64,7 @@ import { PagesManifest } from '../../build/webpack/plugins/pages-manifest-plugin
 import { removePathTrailingSlash } from '../../client/normalize-trailing-slash'
 import getRouteFromAssetPath from '../lib/router/utils/get-route-from-asset-path'
 import { FontManifest } from './font-utils'
+import { denormalizePagePath } from './denormalize-page-path'
 
 const getCustomRouteMatcher = pathMatch(true)
 
@@ -1025,9 +1026,9 @@ export default class Server {
     // remove /_next/data prefix from urlPathname so it matches
     // for direct page visit and /_next/data visit
     if (isDataReq && urlPathname.includes(this.buildId)) {
-      urlPathname = (urlPathname.split(this.buildId).pop() || '/')
-        .replace(/\.json$/, '')
-        .replace(/\/index$/, '/')
+      urlPathname = denormalizePagePath(
+        (urlPathname.split(this.buildId).pop() || '/').replace(/\.json$/, '')
+      )
     }
 
     const ssgCacheKey =
@@ -1110,7 +1111,15 @@ export default class Server {
             ...components,
             ...opts,
             isDataReq,
+            normalizedAsPath: isDataReq
+              ? formatUrl({
+                  pathname: urlPathname,
+                  // make sure to only add query values from original URL
+                  query: parseUrl(req.url!, true).query,
+                })
+              : undefined,
           }
+
           renderResult = await renderToHTML(
             req,
             res,

packages/next/next-server/server/render.tsx

@@ -154,6 +154,7 @@ export type RenderOptsPartial = {
   fontManifest?: FontManifest
   optimizeImages: boolean
   devOnlyCacheBusterQueryString?: string
+  normalizedAsPath?: string
 }
 
 export type RenderOpts = LoadComponentsReturnType & RenderOptsPartial
@@ -488,7 +489,7 @@ export async function renderToHTML(
   await Loadable.preloadAll() // Make sure all dynamic imports are loaded
 
   // url will always be set
-  const asPath: string = req.url as string
+  const asPath: string = renderOpts.normalizedAsPath || (req.url as string)
   const router = new ServerRouter(
     pathname,
     query,

test/integration/getserversideprops/test/index.test.js

@@ -300,7 +300,7 @@ const runTests = (dev = false) => {
     expect(appProps).toEqual({
       url: curUrl,
       query: { post: 'post-1' },
-      asPath: curUrl,
+      asPath: '/blog/post-1',
       pathname: '/blog/[post]',
     })
   })
@@ -313,7 +313,7 @@ const runTests = (dev = false) => {
     expect(appProps).toEqual({
       url: curUrl,
       query: {},
-      asPath: curUrl,
+      asPath: '/something',
       pathname: '/something',
     })
   })

test/integration/revalidate-as-path/pages/_app.js

+import { useRouter } from 'next/router'
+
+export default function App({ Component, pageProps }) {
+  // we log the asPath during rendering to make sure the value is
+  // correct during the /_next/data request since they are kept in sync
+  console.log(`asPath: ${useRouter().asPath}`)
+  return <Component {...pageProps} />
+}

test/integration/revalidate-as-path/pages/another/index/index.js

+import { useRouter } from 'next/router'
+
+export default function Another() {
+  const router = useRouter()
+
+  return (
+    <>
+      <p id="as-path">{router.asPath}</p>
+      <p id="another">another</p>
+    </>
+  )
+}
+
+export const getStaticProps = () => {
+  return {
+    props: {
+      hello: 'world',
+    },
+    revalidate: 1,
+  }
+}

test/integration/revalidate-as-path/pages/index.js

+import { useRouter } from 'next/router'
+
+export default function Index() {
+  const router = useRouter()
+
+  return (
+    <>
+      <p id="as-path">{router.asPath}</p>
+      <p id="index">index</p>
+    </>
+  )
+}
+
+export const getStaticProps = () => {
+  return {
+    props: {
+      hello: 'world',
+    },
+    revalidate: 1,
+  }
+}

test/integration/revalidate-as-path/server.js

+const http = require('http')
+const url = require('url')
+
+const render = (pagePath, req, res) => {
+  const mod = require(`./.next/serverless/pages/${pagePath}`)
+  return (mod.render || mod.default || mod)(req, res)
+}
+
+const { BUILD_ID } = process.env
+
+const server = http.createServer(async (req, res) => {
+  try {
+    const { pathname } = url.parse(req.url)
+
+    switch (pathname) {
+      case '/index':
+      case '/':
+      case `/_next/data/${BUILD_ID}/index.json`:
+        return render('index.js', req, res)
+      case '/another/index':
+      case '/another':
+      case `/_next/data/${BUILD_ID}/another/index.json`:
+        return render('another/index.js', req, res)
+      default: {
+        res.statusCode = 404
+        res.end('not found')
+      }
+    }
+  } catch (err) {
+    console.error('failed to render', err)
+    res.statusCode = 500
+    res.end(err.message)
+  }
+})
+
+server.listen(process.env.PORT, () => {
+  console.log('ready on', process.env.PORT)
+})

test/integration/revalidate-as-path/test/index.test.js

+/* eslint-env jest */
+
+import fs from 'fs-extra'
+import { join } from 'path'
+import cheerio from 'cheerio'
+import {
+  killApp,
+  findPort,
+  nextBuild,
+  initNextServerScript,
+  renderViaHTTP,
+  nextStart,
+  waitFor,
+  check,
+} from 'next-test-utils'
+
+jest.setTimeout(1000 * 60 * 2)
+
+const appDir = join(__dirname, '../')
+const nextConfigPath = join(appDir, 'next.config.js')
+let appPort
+let buildId
+let app
+let stdout = ''
+
+const checkAsPath = async (urlPath, expectedAsPath) => {
+  const html = await renderViaHTTP(appPort, urlPath)
+  const $ = cheerio.load(html)
+  const asPath = $('#as-path').text()
+
+  expect(asPath).toBe(expectedAsPath)
+}
+
+const runTests = (isServerless) => {
+  if (isServerless) {
+    it('should render with correct asPath with /index requested', async () => {
+      await checkAsPath('/index', '/')
+    })
+  }
+
+  it('should render with correct asPath with /_next/data /index requested', async () => {
+    stdout = ''
+    const path = `/_next/data/${buildId}/index.json`
+    await renderViaHTTP(appPort, path)
+    await waitFor(1000)
+    const data = await renderViaHTTP(appPort, path)
+
+    expect(JSON.parse(data).pageProps).toEqual({
+      hello: 'world',
+    })
+
+    await check(() => stdout, /asPath/)
+    const asPath = stdout.split('asPath: ').pop().split('\n').shift()
+    expect(asPath).toBe('/')
+  })
+
+  it('should render with correct asPath with / requested', async () => {
+    await checkAsPath('/', '/')
+  })
+
+  it('should render with correct asPath with /another/index requested', async () => {
+    await checkAsPath('/another/index', '/another/index')
+  })
+
+  it('should render with correct asPath with /_next/data /another/index requested', async () => {
+    stdout = ''
+    const path = `/_next/data/${buildId}/another/index.json`
+    await renderViaHTTP(appPort, path)
+    await waitFor(1000)
+    const data = await renderViaHTTP(appPort, path)
+
+    expect(JSON.parse(data).pageProps).toEqual({
+      hello: 'world',
+    })
+
+    await check(() => stdout, /asPath/)
+    const asPath = stdout.split('asPath: ').pop().split('\n').shift()
+    expect(asPath).toBe('/another/index')
+  })
+}
+
+describe('Revalidate asPath Normalizing', () => {
+  describe('raw serverless mode', () => {
+    beforeAll(async () => {
+      await fs.remove(join(appDir, '.next'))
+      await fs.writeFile(
+        nextConfigPath,
+        `
+          module.exports = {
+            target: 'experimental-serverless-trace'
+          }
+        `
+      )
+      appPort = await findPort()
+      await nextBuild(appDir)
+
+      buildId = await fs.readFile(join(appDir, '.next/BUILD_ID'), 'utf8')
+
+      app = await initNextServerScript(
+        join(appDir, 'server.js'),
+        /ready on/,
+        {
+          ...process.env,
+          PORT: appPort,
+          BUILD_ID: buildId,
+        },
+        /error/,
+        {
+          onStdout(msg) {
+            stdout += msg || ''
+          },
+        }
+      )
+    })
+    afterAll(async () => {
+      await killApp(app)
+      await fs.remove(nextConfigPath)
+    })
+    runTests()
+  })
+
+  describe('server mode', () => {
+    beforeAll(async () => {
+      await fs.remove(join(appDir, '.next'))
+      appPort = await findPort()
+      await nextBuild(appDir)
+
+      buildId = await fs.readFile(join(appDir, '.next/BUILD_ID'), 'utf8')
+
+      app = await nextStart(appDir, appPort, {
+        onStdout(msg) {
+          console.log('got stdout', msg)
+          stdout += msg || ''
+        },
+      })
+    })
+    afterAll(() => killApp(app))
+    runTests()
+  })
+})