From 65588083db392712f842a67f3e63911a626e33ec Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Sun, 18 Apr 2021 05:00:04 -0500 Subject: [PATCH] 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: https://github.com/vercel/next.js/issues/23490 Fixes: https://github.com/vercel/next.js/issues/22931 Fixes: https://github.com/vercel/next.js/issues/21062 --- packages/next/client/index.tsx | 18 ++++++++++-- .../next/next-server/lib/router/router.ts | 4 ++- .../build-output/test/index.test.js | 4 +-- test/integration/custom-routes/next.config.js | 6 +++- .../custom-routes/pages/auto-export/[slug].js | 8 +++++- .../pages/auto-export/another.js | 11 ++++++++ .../custom-routes/test/index.test.js | 28 ++++++++++++++++++- .../integration/production/test/index.test.js | 3 +- 8 files changed, 73 insertions(+), 9 deletions(-) create mode 100644 test/integration/custom-routes/pages/auto-export/another.js diff --git a/packages/next/client/index.tsx b/packages/next/client/index.tsx index 25c559f9c543d..448a89608d97f 100644 --- 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 index 28e6d864c9ac4..785cbf0bbaf8b 100644 --- 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 index eb680f8b1010b..434ff9af58d4d 100644 --- 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 index 047b59c659f0d..c5ac688266816 100644 --- 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 index dd3efd7950a87..fe2b9fb3ea7c5 100644 --- 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

auto-export {useRouter().query.slug}

+ const router = useRouter() + return ( + <> +

auto-export {router.query.slug}

+

{JSON.stringify(router.query)}

+ + ) } diff --git a/test/integration/custom-routes/pages/auto-export/another.js b/test/integration/custom-routes/pages/auto-export/another.js new file mode 100644 index 0000000000000..d0a7267419dac --- /dev/null +++ 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 ( + <> +

auto-export another

+

{JSON.stringify(router.query)}

+ + ) +} diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js index d5bb4ed476695..725809849fc37 100644 --- 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 index 90ca62b5e7299..ec56edb6f2a09 100644 --- 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')` )