From 3c283f8638a2197a0fcc6d81cb2e44b673c56181 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 11 Jan 2021 13:03:34 -0600 Subject: [PATCH 1/6] add logs --- .../next/next-server/server/next-server.ts | 31 +++++++++++++++++++ 1 file changed, 31 insertions(+) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 3bbef11aa91869f..8f58017f2ed0d07 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -490,6 +490,17 @@ export default class Server { ? matchedPathname.replace(/\.json$/, '') : matchedPathname + console.log({ + reqUrlIsDataUrl, + matchedPathIsDataUrl, + isDataUrl, + parsedPath, + pathname, + query, + matchedPathname, + matchedPathnameNoExt, + }) + // interpolate dynamic params and normalize URL if needed if (isDynamicRoute(matchedPathnameNoExt)) { const utils = getUtils({ @@ -523,13 +534,24 @@ export default class Server { params = utils.dynamicRouteMatcher!(matchedPathname) } + console.log({ params }) + if (params) { + console.log('before interpolating', { + matchedPathname, + url: req.url, + }) matchedPathname = utils.interpolateDynamicPath( matchedPathname, params ) req.url = utils.interpolateDynamicPath(req.url!, params) + + console.log('after interpolating', { + matchedPathname, + url: req.url, + }) } if (reqUrlIsDataUrl && matchedPathIsDataUrl) { @@ -537,13 +559,22 @@ export default class Server { ...parsedPath, pathname: matchedPathname, }) + console.log('updated req.url for data path', { + url: req.url, + reqUrlIsDataUrl, + matchedPathIsDataUrl, + }) } Object.assign(parsedUrl.query, params) + console.log('before normalize URL', req.url) utils.normalizeVercelUrl(req, true) + console.log('after normalize URL', req.url) } parsedUrl.pathname = `${basePath || ''}${ parsedUrl.query.__nextLocale || '' }${matchedPathname}` + + console.log('parsedUrl.pathname', parsedUrl.pathname) } res.statusCode = 200 From b7813ed36ff23f3e6547852c9f9f79500fa7a618 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 11 Jan 2021 13:55:34 -0600 Subject: [PATCH 2/6] log route-matches --- packages/next/next-server/server/next-server.ts | 1 + 1 file changed, 1 insertion(+) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index 8f58017f2ed0d07..b587a82d1010fad 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -491,6 +491,7 @@ export default class Server { : matchedPathname console.log({ + routeMatches: req.headers['x-now-route-matches'], reqUrlIsDataUrl, matchedPathIsDataUrl, isDataUrl, From d0b621ec2ba28744a24d1dea298578eb24b7bdfc Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 12 Jan 2021 13:39:45 -0600 Subject: [PATCH 3/6] Save changes and test --- .../next/next-server/server/next-server.ts | 238 +++++++++--------- .../pages/catch-all/[[...rest]].js | 29 +++ .../required-server-files/test/index.test.js | 110 ++++++++ 3 files changed, 264 insertions(+), 113 deletions(-) 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 b587a82d1010fad..fc92463fec9b31e 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,126 @@ 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 + + console.log({ + url: req.url, + matchedPathnameHeader: req.headers['x-matched-path'], + routeMatches: req.headers['x-now-route-matches'], + reqUrlIsDataUrl, + matchedPathIsDataUrl, + isDataUrl, + parsedPath, + pathname, + query, + matchedPathname, + matchedPathnameNoExt, + }) + + // 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!(matchedPathnameNoExt) + } + + console.log('before normalize', { params }) + + if (params) { + params = utils.normalizeDynamicRouteParams(params).params + console.log('after normalize', { params }) + + console.log('before interpolating', { + matchedPathname, + url: req.url, + }) + matchedPathname = utils.interpolateDynamicPath( + matchedPathname, + params + ) + + req.url = utils.interpolateDynamicPath(req.url!, params) + + console.log('after interpolating', { + matchedPathname, + url: req.url, + interpolatedUrl: utils.interpolateDynamicPath(req.url!, params), + }) + } + + if (reqUrlIsDataUrl && matchedPathIsDataUrl) { + req.url = formatUrl({ + ...parsedPath, + pathname: matchedPathname, + }) + console.log('updated req.url for data path', { + url: req.url, + potentialUpdatedUrl: formatUrl({ + ...parsedPath, + pathname: matchedPathname, + }), + reqUrlIsDataUrl, + matchedPathIsDataUrl, + }) + } + Object.assign(parsedUrl.query, params) + console.log('before normalize URL', req.url) + utils.normalizeVercelUrl(req, true) + console.log('after normalize URL', req.url) + } + + 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 || '/' @@ -468,118 +586,12 @@ export default class Server { 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 - - console.log({ - routeMatches: req.headers['x-now-route-matches'], - reqUrlIsDataUrl, - matchedPathIsDataUrl, - isDataUrl, - parsedPath, - pathname, - query, - matchedPathname, - matchedPathnameNoExt, - }) - - // 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) - } - - console.log({ params }) - - if (params) { - console.log('before interpolating', { - matchedPathname, - url: req.url, - }) - matchedPathname = utils.interpolateDynamicPath( - matchedPathname, - params - ) - - req.url = utils.interpolateDynamicPath(req.url!, params) - - console.log('after interpolating', { - matchedPathname, - url: req.url, - }) - } - - if (reqUrlIsDataUrl && matchedPathIsDataUrl) { - req.url = formatUrl({ - ...parsedPath, - pathname: matchedPathname, - }) - console.log('updated req.url for data path', { - url: req.url, - reqUrlIsDataUrl, - matchedPathIsDataUrl, - }) - } - Object.assign(parsedUrl.query, params) - console.log('before normalize URL', req.url) - utils.normalizeVercelUrl(req, true) - console.log('after normalize URL', req.url) - } - parsedUrl.pathname = `${basePath || ''}${ - parsedUrl.query.__nextLocale || '' - }${matchedPathname}` - - console.log('parsedUrl.pathname', parsedUrl.pathname) - } - res.statusCode = 200 try { + console.log('run', { + url: req.url, + parsedUrl, + }) return await this.run(req, res, parsedUrl) } catch (err) { this.logError(err) 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 000000000000000..c93573368f0e052 --- /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 4ea34ab10b8fb15..7ffa7c62cf78b83 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 [ '/', From 5d8d0f1ba6a011d12b4f3c4c3ed4c9d736a08981 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Tue, 12 Jan 2021 17:19:18 -0600 Subject: [PATCH 4/6] Prioritize matched-path locale --- .../next/next-server/server/next-server.ts | 21 +++++++++++++++---- 1 file changed, 17 insertions(+), 4 deletions(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index fc92463fec9b31e..dcb4998796a2ebe 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -365,6 +365,17 @@ export default class Server { matchedPathnameNoExt, }) + if (i18n) { + const localePathResult = normalizeLocalePath( + matchedPathname || '/', + i18n.locales + ) + + if (localePathResult.detectedLocale) { + parsedUrl.query.__nextLocale = localePathResult.detectedLocale + } + } + // interpolate dynamic params and normalize URL if needed if (isDynamicRoute(matchedPathnameNoExt)) { const utils = getUtils({ @@ -580,10 +591,12 @@ export default class Server { parsedUrl.query.__nextDefaultLocale = detectedDomain?.defaultLocale || i18n.defaultLocale - parsedUrl.query.__nextLocale = - localePathResult.detectedLocale || - detectedDomain?.defaultLocale || - defaultLocale + if (!this.minimalMode || !parsedUrl.query.__nextLocale) { + parsedUrl.query.__nextLocale = + localePathResult.detectedLocale || + detectedDomain?.defaultLocale || + defaultLocale + } } res.statusCode = 200 From 6d7724726c23841576fcf5b4b4a30374d9747f25 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 13 Jan 2021 11:26:03 -0600 Subject: [PATCH 5/6] Ensure rewrite params are normalized --- .../next/next-server/server/next-server.ts | 23 +++++++++++-------- .../required-server-files/next.config.js | 10 ++++++++ .../required-server-files/test/index.test.js | 19 +++++++++++++++ 3 files changed, 43 insertions(+), 9 deletions(-) create mode 100644 test/integration/required-server-files/next.config.js diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index dcb4998796a2ebe..c989b2519b8c111 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -363,6 +363,7 @@ export default class Server { query, matchedPathname, matchedPathnameNoExt, + parsedUrl, }) if (i18n) { @@ -376,16 +377,20 @@ export default class Server { } } - // 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, - }) + 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) + console.log('after handleRewrites', parsedUrl) + // interpolate dynamic params and normalize URL if needed + if (pageIsDynamic) { let params: ParsedUrlQuery | false = {} const paramsResult = utils.normalizeDynamicRouteParams({ ...parsedUrl.query, 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 000000000000000..1214e33ad2f06af --- /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/test/index.test.js b/test/integration/required-server-files/test/index.test.js index 7ffa7c62cf78b83..97a1fc7e12d8f3d 100644 --- a/test/integration/required-server-files/test/index.test.js +++ b/test/integration/required-server-files/test/index.test.js @@ -400,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'], + }) + }) }) From 977d98a098cc171d435974dce1a30270d6be9985 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Wed, 13 Jan 2021 14:51:35 -0600 Subject: [PATCH 6/6] Remove extra logs --- .../next/next-server/server/next-server.ts | 45 ------------------- 1 file changed, 45 deletions(-) diff --git a/packages/next/next-server/server/next-server.ts b/packages/next/next-server/server/next-server.ts index c989b2519b8c111..185b17e62323f04 100644 --- a/packages/next/next-server/server/next-server.ts +++ b/packages/next/next-server/server/next-server.ts @@ -351,21 +351,6 @@ export default class Server { ? matchedPathname.replace(/\.json$/, '') : matchedPathname - console.log({ - url: req.url, - matchedPathnameHeader: req.headers['x-matched-path'], - routeMatches: req.headers['x-now-route-matches'], - reqUrlIsDataUrl, - matchedPathIsDataUrl, - isDataUrl, - parsedPath, - pathname, - query, - matchedPathname, - matchedPathnameNoExt, - parsedUrl, - }) - if (i18n) { const localePathResult = normalizeLocalePath( matchedPathname || '/', @@ -387,7 +372,6 @@ export default class Server { }) utils.handleRewrites(parsedUrl) - console.log('after handleRewrites', parsedUrl) // interpolate dynamic params and normalize URL if needed if (pageIsDynamic) { @@ -414,28 +398,14 @@ export default class Server { params = utils.dynamicRouteMatcher!(matchedPathnameNoExt) } - console.log('before normalize', { params }) - if (params) { params = utils.normalizeDynamicRouteParams(params).params - console.log('after normalize', { params }) - console.log('before interpolating', { - matchedPathname, - url: req.url, - }) matchedPathname = utils.interpolateDynamicPath( matchedPathname, params ) - req.url = utils.interpolateDynamicPath(req.url!, params) - - console.log('after interpolating', { - matchedPathname, - url: req.url, - interpolatedUrl: utils.interpolateDynamicPath(req.url!, params), - }) } if (reqUrlIsDataUrl && matchedPathIsDataUrl) { @@ -443,20 +413,9 @@ export default class Server { ...parsedPath, pathname: matchedPathname, }) - console.log('updated req.url for data path', { - url: req.url, - potentialUpdatedUrl: formatUrl({ - ...parsedPath, - pathname: matchedPathname, - }), - reqUrlIsDataUrl, - matchedPathIsDataUrl, - }) } Object.assign(parsedUrl.query, params) - console.log('before normalize URL', req.url) utils.normalizeVercelUrl(req, true) - console.log('after normalize URL', req.url) } parsedUrl.pathname = `${basePath || ''}${ @@ -606,10 +565,6 @@ export default class Server { res.statusCode = 200 try { - console.log('run', { - url: req.url, - parsedUrl, - }) return await this.run(req, res, parsedUrl) } catch (err) { this.logError(err)