From cfa9326c63d1c139eb25ca86ccadfb74a2c31cde Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Sat, 16 Jan 2021 09:46:21 -0600 Subject: [PATCH] Minimal mode normalizing (#21083) Co-authored-by: kodiakhq[bot] <49736102+kodiakhq[bot]@users.noreply.github.com> --- .../next/next-server/server/next-server.ts | 185 ++++++++++-------- .../required-server-files/next.config.js | 10 + .../pages/catch-all/[[...rest]].js | 29 +++ .../required-server-files/test/index.test.js | 129 ++++++++++++ 4 files changed, 269 insertions(+), 84 deletions(-) create mode 100644 test/integration/required-server-files/next.config.js create mode 100644 test/integration/required-server-files/pages/catch-all/[[...rest]].js diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 3bbef11aa9186..185b17e62323f 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -314,6 +314,7 @@ export default class Server { const url: any = req.url parsedUrl = parseUrl(url, true) } + const { basePath, i18n } = this.nextConfig // Parse the querystring ourselves if the user doesn't handle querystring parsing if (typeof parsedUrl.query === 'string') { @@ -321,8 +322,6 @@ export default class Server { } ;(req as any).__NEXT_INIT_QUERY = Object.assign({}, parsedUrl.query) - const { basePath, i18n } = this.nextConfig - if (basePath && req.url?.startsWith(basePath)) { // store original URL to allow checking if basePath was // provided or not @@ -330,7 +329,101 @@ export default class Server { req.url = req.url!.replace(basePath, '') || '/' } - if (i18n && !req.url?.startsWith('/_next')) { + if ( + this.minimalMode && + req.headers['x-matched-path'] && + typeof req.headers['x-matched-path'] === 'string' + ) { + const reqUrlIsDataUrl = req.url?.includes('/_next/data') + const matchedPathIsDataUrl = req.headers['x-matched-path']?.includes( + '/_next/data' + ) + const isDataUrl = reqUrlIsDataUrl || matchedPathIsDataUrl + + let parsedPath = parseUrl( + isDataUrl ? req.url! : (req.headers['x-matched-path'] as string), + true + ) + const { pathname, query } = parsedPath + let matchedPathname = pathname as string + + const matchedPathnameNoExt = isDataUrl + ? matchedPathname.replace(/\.json$/, '') + : matchedPathname + + if (i18n) { + const localePathResult = normalizeLocalePath( + matchedPathname || '/', + i18n.locales + ) + + if (localePathResult.detectedLocale) { + parsedUrl.query.__nextLocale = localePathResult.detectedLocale + } + } + + const pageIsDynamic = isDynamicRoute(matchedPathnameNoExt) + const utils = getUtils({ + pageIsDynamic, + page: matchedPathnameNoExt, + i18n: this.nextConfig.i18n, + basePath: this.nextConfig.basePath, + rewrites: this.customRoutes.rewrites, + }) + + utils.handleRewrites(parsedUrl) + + // interpolate dynamic params and normalize URL if needed + if (pageIsDynamic) { + let params: ParsedUrlQuery | false = {} + const paramsResult = utils.normalizeDynamicRouteParams({ + ...parsedUrl.query, + ...query, + }) + + if (paramsResult.hasValidParams) { + params = paramsResult.params + } else if (req.headers['x-now-route-matches']) { + const opts: Record = {} + params = utils.getParamsFromRouteMatches( + req, + opts, + (parsedUrl.query.__nextLocale as string | undefined) || '' + ) + + if (opts.locale) { + parsedUrl.query.__nextLocale = opts.locale + } + } else { + params = utils.dynamicRouteMatcher!(matchedPathnameNoExt) + } + + if (params) { + params = utils.normalizeDynamicRouteParams(params).params + + matchedPathname = utils.interpolateDynamicPath( + matchedPathname, + params + ) + req.url = utils.interpolateDynamicPath(req.url!, params) + } + + if (reqUrlIsDataUrl && matchedPathIsDataUrl) { + req.url = formatUrl({ + ...parsedPath, + pathname: matchedPathname, + }) + } + Object.assign(parsedUrl.query, params) + utils.normalizeVercelUrl(req, true) + } + + parsedUrl.pathname = `${basePath || ''}${ + matchedPathname === '/' && basePath ? '' : matchedPathname + }` + } + + if (i18n) { // get pathname from URL with basePath stripped for locale detection let { pathname, ...parsed } = parseUrl(req.url || '/') pathname = pathname || '/' @@ -462,88 +555,12 @@ export default class Server { parsedUrl.query.__nextDefaultLocale = detectedDomain?.defaultLocale || i18n.defaultLocale - parsedUrl.query.__nextLocale = - localePathResult.detectedLocale || - detectedDomain?.defaultLocale || - defaultLocale - } - - if ( - this.minimalMode && - req.headers['x-matched-path'] && - typeof req.headers['x-matched-path'] === 'string' - ) { - const reqUrlIsDataUrl = req.url?.includes('/_next/data') - const matchedPathIsDataUrl = req.headers['x-matched-path']?.includes( - '/_next/data' - ) - const isDataUrl = reqUrlIsDataUrl || matchedPathIsDataUrl - - let parsedPath = parseUrl( - isDataUrl ? req.url! : (req.headers['x-matched-path'] as string), - true - ) - const { pathname, query } = parsedPath - let matchedPathname = pathname as string - - const matchedPathnameNoExt = isDataUrl - ? matchedPathname.replace(/\.json$/, '') - : matchedPathname - - // interpolate dynamic params and normalize URL if needed - if (isDynamicRoute(matchedPathnameNoExt)) { - const utils = getUtils({ - pageIsDynamic: true, - page: matchedPathnameNoExt, - i18n: this.nextConfig.i18n, - basePath: this.nextConfig.basePath, - rewrites: this.customRoutes.rewrites, - }) - - let params: ParsedUrlQuery | false = {} - const paramsResult = utils.normalizeDynamicRouteParams({ - ...parsedUrl.query, - ...query, - }) - - if (paramsResult.hasValidParams) { - params = paramsResult.params - } else if (req.headers['x-now-route-matches']) { - const opts: Record = {} - params = utils.getParamsFromRouteMatches( - req, - opts, - (parsedUrl.query.__nextLocale as string | undefined) || '' - ) - - if (opts.locale) { - parsedUrl.query.__nextLocale = opts.locale - } - } else { - params = utils.dynamicRouteMatcher!(matchedPathname) - } - - if (params) { - matchedPathname = utils.interpolateDynamicPath( - matchedPathname, - params - ) - - req.url = utils.interpolateDynamicPath(req.url!, params) - } - - if (reqUrlIsDataUrl && matchedPathIsDataUrl) { - req.url = formatUrl({ - ...parsedPath, - pathname: matchedPathname, - }) - } - Object.assign(parsedUrl.query, params) - utils.normalizeVercelUrl(req, true) + if (!this.minimalMode || !parsedUrl.query.__nextLocale) { + parsedUrl.query.__nextLocale = + localePathResult.detectedLocale || + detectedDomain?.defaultLocale || + defaultLocale } - parsedUrl.pathname = `${basePath || ''}${ - parsedUrl.query.__nextLocale || '' - }${matchedPathname}` } res.statusCode = 200 diff --git a/test/integration/required-server-files/next.config.js b/test/integration/required-server-files/next.config.js new file mode 100644 index 0000000000000..1214e33ad2f06 --- /dev/null +++ b/test/integration/required-server-files/next.config.js @@ -0,0 +1,10 @@ +module.exports = { + rewrites() { + return [ + { + source: '/some-catch-all/:path*', + destination: '/', + }, + ] + }, +} diff --git a/test/integration/required-server-files/pages/catch-all/[[...rest]].js b/test/integration/required-server-files/pages/catch-all/[[...rest]].js new file mode 100644 index 0000000000000..c93573368f0e0 --- /dev/null +++ b/test/integration/required-server-files/pages/catch-all/[[...rest]].js @@ -0,0 +1,29 @@ +import { useRouter } from 'next/router' + +export const getStaticProps = ({ params }) => { + return { + props: { + hello: 'world', + params: params || null, + random: Math.random(), + }, + } +} + +export const getStaticPaths = () => { + return { + paths: ['/catch-all/hello'], + fallback: true, + } +} + +export default function Page(props) { + const router = useRouter() + return ( + <> +

optional catch-all page

+

{JSON.stringify(router)}

+

{JSON.stringify(props)}

+ + ) +} diff --git a/test/integration/required-server-files/test/index.test.js b/test/integration/required-server-files/test/index.test.js index 4ea34ab10b8fb..97a1fc7e12d8f 100644 --- a/test/integration/required-server-files/test/index.test.js +++ b/test/integration/required-server-files/test/index.test.js @@ -273,6 +273,116 @@ describe('Required Server Files', () => { expect(data2.hello).toBe('world') }) + it('should render fallback optional catch-all route correctly with x-matched-path and routes-matches', async () => { + const html = await renderViaHTTP( + appPort, + '/catch-all/[[...rest]]', + undefined, + { + headers: { + 'x-matched-path': '/catch-all/[[...rest]]', + 'x-now-route-matches': '', + }, + } + ) + const $ = cheerio.load(html) + const data = JSON.parse($('#props').text()) + + expect($('#catch-all').text()).toBe('optional catch-all page') + expect(data.params).toEqual({}) + expect(data.hello).toBe('world') + + const html2 = await renderViaHTTP( + appPort, + '/catch-all/[[...rest]]', + undefined, + { + headers: { + 'x-matched-path': '/catch-all/[[...rest]]', + 'x-now-route-matches': '1=hello&catchAll=hello', + }, + } + ) + const $2 = cheerio.load(html2) + const data2 = JSON.parse($2('#props').text()) + + expect($2('#catch-all').text()).toBe('optional catch-all page') + expect(data2.params).toEqual({ rest: ['hello'] }) + expect(isNaN(data2.random)).toBe(false) + expect(data2.random).not.toBe(data.random) + + const html3 = await renderViaHTTP( + appPort, + '/catch-all/[[..rest]]', + undefined, + { + headers: { + 'x-matched-path': '/catch-all/[[...rest]]', + 'x-now-route-matches': '1=hello/world&catchAll=hello/world', + }, + } + ) + const $3 = cheerio.load(html3) + const data3 = JSON.parse($3('#props').text()) + + expect($3('#catch-all').text()).toBe('optional catch-all page') + expect(data3.params).toEqual({ rest: ['hello', 'world'] }) + expect(isNaN(data3.random)).toBe(false) + expect(data3.random).not.toBe(data.random) + }) + + it('should return data correctly with x-matched-path for optional catch-all route', async () => { + const res = await fetchViaHTTP( + appPort, + `/_next/data/${buildId}/catch-all.json`, + undefined, + { + headers: { + 'x-matched-path': '/catch-all/[[...rest]]', + }, + } + ) + + const { pageProps: data } = await res.json() + + expect(data.params).toEqual({}) + expect(data.hello).toBe('world') + + const res2 = await fetchViaHTTP( + appPort, + `/_next/data/${buildId}/catch-all/[[...rest]].json`, + undefined, + { + headers: { + 'x-matched-path': `/_next/data/${buildId}/catch-all/[[...rest]].json`, + 'x-now-route-matches': '1=hello&rest=hello', + }, + } + ) + + const { pageProps: data2 } = await res2.json() + + expect(data2.params).toEqual({ rest: ['hello'] }) + expect(data2.hello).toBe('world') + + const res3 = await fetchViaHTTP( + appPort, + `/_next/data/${buildId}/catch-all/[[...rest]].json`, + undefined, + { + headers: { + 'x-matched-path': `/_next/data/${buildId}/catch-all/[[...rest]].json`, + 'x-now-route-matches': '1=hello/world&rest=hello/world', + }, + } + ) + + const { pageProps: data3 } = await res3.json() + + expect(data3.params).toEqual({ rest: ['hello', 'world'] }) + expect(data3.hello).toBe('world') + }) + it('should not apply trailingSlash redirect', async () => { for (const path of [ '/', @@ -290,4 +400,23 @@ describe('Required Server Files', () => { expect(res.status).toBe(200) } }) + + it('should normalize catch-all rewrite query values correctly', async () => { + const html = await renderViaHTTP( + appPort, + '/some-catch-all/hello/world', + { + path: 'hello/world', + }, + { + headers: { + 'x-matched-path': '/', + }, + } + ) + const $ = cheerio.load(html) + expect(JSON.parse($('#router').text()).query).toEqual({ + path: ['hello', 'world'], + }) + }) })