Ensure query for static pages with rewrites is updated correctly (#24189

) This updates the query refreshing on the client to also refresh when rewrites are used and the page is static since additional query values can be provided from rewrites that are relied on client-side. An additional test has been added in the custom-routes suite to ensure this is working correctly. ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added Fixes: #23490 Fixes: #22931 Fixes: #21062
vercel · Apr 18, 2021 · 6558808 · 6558808
1 parent 6e7245c
commit 6558808
Show file tree

Hide file tree

Showing 8 changed files with 73 additions and 9 deletions.
diff --git a/packages/next/client/index.tsx b/packages/next/client/index.tsx
@@ -186,10 +186,24 @@ class Container extends React.Component<{
     // - if it is a client-side skeleton (fallback render)
     if (
       router.isSsr &&
+      // We don't update for 404 requests as this can modify
+      // the asPath unexpectedly e.g. adding basePath when
+      // it wasn't originally present
+      page !== '/404' &&
+      !(
+        page === '/_error' &&
+        hydrateProps &&
+        hydrateProps.pageProps &&
+        hydrateProps.pageProps.statusCode === 404
+      ) &&
       (isFallback ||
         (data.nextExport &&
-          (isDynamicRoute(router.pathname) || location.search)) ||
-        (hydrateProps && hydrateProps.__N_SSG && location.search))
+          (isDynamicRoute(router.pathname) ||
+            location.search ||
+            process.env.__NEXT_HAS_REWRITES)) ||
+        (hydrateProps &&
+          hydrateProps.__N_SSG &&
+          (location.search || process.env.__NEXT_HAS_REWRITES)))
     ) {
       // update query on mount for exported pages
       router.replace(

diff --git a/packages/next/next-server/lib/router/router.ts b/packages/next/next-server/lib/router/router.ts
@@ -615,7 +615,9 @@ export default class Router implements BaseRouter {
     this.isReady = !!(
       self.__NEXT_DATA__.gssp ||
       self.__NEXT_DATA__.gip ||
-      (!autoExportDynamic && !self.location.search)
+      (!autoExportDynamic &&
+        !self.location.search &&
+        !process.env.__NEXT_HAS_REWRITES)
     )
     this.isPreview = !!isPreview
     this.isLocaleDomain = false

diff --git a/test/integration/build-output/test/index.test.js b/test/integration/build-output/test/index.test.js
@@ -99,7 +99,7 @@ describe('Build Output', () => {
       expect(indexSize.endsWith('B')).toBe(true)
 
       // should be no bigger than 64.8 kb
-      expect(parseFloat(indexFirstLoad)).toBeCloseTo(65.3, 1)
+      expect(parseFloat(indexFirstLoad)).toBeCloseTo(65.4, 1)
       expect(indexFirstLoad.endsWith('kB')).toBe(true)
 
       expect(parseFloat(err404Size)).toBeCloseTo(3.7, 1)
@@ -108,7 +108,7 @@ describe('Build Output', () => {
       expect(parseFloat(err404FirstLoad)).toBeCloseTo(68.5, 0)
       expect(err404FirstLoad.endsWith('kB')).toBe(true)
 
-      expect(parseFloat(sharedByAll)).toBeCloseTo(65, 1)
+      expect(parseFloat(sharedByAll)).toBeCloseTo(65.1, 1)
       expect(sharedByAll.endsWith('kB')).toBe(true)
 
       if (_appSize.endsWith('kB')) {

diff --git a/test/integration/custom-routes/next.config.js b/test/integration/custom-routes/next.config.js
@@ -13,7 +13,11 @@ module.exports = {
           : []),
         {
           source: '/rewriting-to-auto-export',
-          destination: '/auto-export/hello',
+          destination: '/auto-export/hello?rewrite=1',
+        },
+        {
+          source: '/rewriting-to-another-auto-export/:path*',
+          destination: '/auto-export/another?rewrite=1',
         },
         {
           source: '/to-another',

diff --git a/test/integration/custom-routes/pages/auto-export/[slug].js b/test/integration/custom-routes/pages/auto-export/[slug].js
@@ -1,5 +1,11 @@
 import { useRouter } from 'next/router'
 
 export default function Page() {
-  return <p id="auto-export">auto-export {useRouter().query.slug}</p>
+  const router = useRouter()
+  return (
+    <>
+      <p id="auto-export">auto-export {router.query.slug}</p>
+      <p id="query">{JSON.stringify(router.query)}</p>
+    </>
+  )
 }
diff --git a/test/integration/custom-routes/pages/auto-export/another.js b/test/integration/custom-routes/pages/auto-export/another.js
@@ -0,0 +1,11 @@
+import { useRouter } from 'next/router'
+
+export default function Page() {
+  const router = useRouter()
+  return (
+    <>
+      <p id="auto-export-another">auto-export another</p>
+      <p id="query">{JSON.stringify(router.query)}</p>
+    </>
+  )
+}
diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js
@@ -42,6 +42,25 @@ const runTests = (isDev = false) => {
     const browser = await webdriver(appPort, '/rewriting-to-auto-export')
     const text = await browser.eval(() => document.documentElement.innerHTML)
     expect(text).toContain('auto-export hello')
+    expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({
+      rewrite: '1',
+      slug: 'hello',
+    })
+  })
+
+  it('should provide params correctly for rewrite to auto-export non-dynamic page', async () => {
+    const browser = await webdriver(
+      appPort,
+      '/rewriting-to-another-auto-export/first'
+    )
+
+    expect(await browser.elementByCss('#auto-export-another').text()).toBe(
+      'auto-export another'
+    )
+    expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({
+      rewrite: '1',
+      path: ['first'],
+    })
   })
 
   it('should handle one-to-one rewrite successfully', async () => {
@@ -1377,10 +1396,17 @@ const runTests = (isDev = false) => {
           ],
           afterFiles: [
             {
-              destination: '/auto-export/hello',
+              destination: '/auto-export/hello?rewrite=1',
               regex: normalizeRegEx('^\\/rewriting-to-auto-export$'),
               source: '/rewriting-to-auto-export',
             },
+            {
+              destination: '/auto-export/another?rewrite=1',
+              regex: normalizeRegEx(
+                '^\\/rewriting-to-another-auto-export(?:\\/((?:[^\\/]+?)(?:\\/(?:[^\\/]+?))*))?$'
+              ),
+              source: '/rewriting-to-another-auto-export/:path*',
+            },
             {
               destination: '/another/one',
               regex: normalizeRegEx('^\\/to-another$'),

diff --git a/test/integration/production/test/index.test.js b/test/integration/production/test/index.test.js
@@ -803,7 +803,8 @@ describe('Production Usage', () => {
   it('should clear all core performance marks', async () => {
     let browser
     try {
-      browser = await webdriver(appPort, '/about')
+      browser = await webdriver(appPort, '/fully-dynamic')
+
       const currentPerfMarks = await browser.eval(
         `window.performance.getEntriesByType('mark')`
       )