From 172afb6c60db3a7c9ca788ec55b47566ec336b46 Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 2 Jan 2020 14:56:01 -0600 Subject: [PATCH 1/4] Support unnamed parameters in custom-routes correctly --- .../next/next-server/server/lib/path-match.ts | 14 ++++++++++++++ test/integration/custom-routes/next.config.js | 4 ++++ .../custom-routes/test/index.test.js | 18 ++++++++++++++++++ 3 files changed, 36 insertions(+) diff --git a/packages/next/next-server/server/lib/path-match.ts b/packages/next/next-server/server/lib/path-match.ts index 9e53f626f922c49..4ef6c3a60fdd0b3 100644 --- a/packages/next/next-server/server/lib/path-match.ts +++ b/packages/next/next-server/server/lib/path-match.ts @@ -16,6 +16,20 @@ export default (customRoute = false) => { if (!res) { return false } + + if (customRoute) { + const newParams: { [k: string]: string } = {} + Object.keys(res.params).forEach((key, idx) => { + if (key === idx + '') { + newParams[idx + 1 + ''] = (res.params as any)[key] + } + }) + res.params = { + ...res.params, + ...newParams, + } + } + return { ...params, ...res.params } } } diff --git a/test/integration/custom-routes/next.config.js b/test/integration/custom-routes/next.config.js index decf161877d2b94..2aab252eac30e34 100644 --- a/test/integration/custom-routes/next.config.js +++ b/test/integration/custom-routes/next.config.js @@ -120,6 +120,10 @@ module.exports = { source: '/query-redirect/:section/:name', destination: '/with-params?first=:section&second=:name', }, + { + source: '/unnamed/(first|second)/(.*)', + destination: '/:1/:2', + }, ] }, diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js index 269eded557397b7..23bc4ebec8bedbc 100644 --- a/test/integration/custom-routes/test/index.test.js +++ b/test/integration/custom-routes/test/index.test.js @@ -229,6 +229,15 @@ const runTests = (isDev = false) => { expect(res.headers.get('x-second-header')).toBe('second') }) + it('should support unnamed parameters correctly', async () => { + const res = await fetchViaHTTP(appPort, '/unnamed/first/final', undefined, { + redirect: 'manual', + }) + const { pathname } = url.parse(res.headers.get('location') || '') + expect(res.status).toBe(307) + expect(pathname).toBe('/first/final') + }) + if (!isDev) { it('should output routes-manifest successfully', async () => { const manifest = await fs.readJSON( @@ -336,6 +345,15 @@ const runTests = (isDev = false) => { source: '/query-redirect/:section/:name', statusCode: 307, }, + { + destination: '/:1/:2', + regex: normalizeRegEx( + '^\\/unnamed(?:\\/(first|second))(?:\\/(.*))$' + ), + regexKeys: [0, 1], + source: '/unnamed/(first|second)/(.*)', + statusCode: 307, + }, ], headers: [ { From 78ca12147e47ab48645e642c7abe2c2806c50fcd Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 6 Jan 2020 11:11:06 -0600 Subject: [PATCH 2/4] Update unnamed params checking to be strict --- .../next/next-server/server/lib/path-match.ts | 22 +++++++++++---- test/integration/custom-routes/next.config.js | 4 +++ .../custom-routes/test/index.test.js | 28 ++++++++++++++++++- .../dynamic-routing/test/index.test.js | 4 +++ 4 files changed, 51 insertions(+), 7 deletions(-) diff --git a/packages/next/next-server/server/lib/path-match.ts b/packages/next/next-server/server/lib/path-match.ts index 8cda44e26db9bff..98a86cfbe62ee48 100644 --- a/packages/next/next-server/server/lib/path-match.ts +++ b/packages/next/next-server/server/lib/path-match.ts @@ -4,12 +4,19 @@ export { pathToRegexp } export default (customRoute = false) => { return (path: string) => { - const matcher = pathToRegexp.match(path, { + const keys: pathToRegexp.Key[] = [] + const matcherOptions = { sensitive: false, delimiter: '/', ...(customRoute ? { strict: true } : undefined), decode: decodeParam, - }) + } + const matcherRegex = pathToRegexp.pathToRegexp(path, keys, matcherOptions) + const matcher = pathToRegexp.regexpToFunction( + matcherRegex, + keys, + matcherOptions + ) return (pathname: string | null | undefined, params?: any) => { const res = pathname == null ? false : matcher(pathname) @@ -19,11 +26,14 @@ export default (customRoute = false) => { if (customRoute) { const newParams: { [k: string]: string } = {} - Object.keys(res.params).forEach((key, idx) => { - if (key === idx + '') { - newParams[idx + 1 + ''] = (res.params as any)[key] + for (const key of keys) { + // unnamed matches should always be a number while named + // should be a string + if (typeof key.name === 'number') { + newParams[key.name + 1 + ''] = (res.params as any)[key.name + ''] + delete (res.params as any)[key.name + ''] } - }) + } res.params = { ...res.params, ...newParams, diff --git a/test/integration/custom-routes/next.config.js b/test/integration/custom-routes/next.config.js index 2aab252eac30e34..c3497f7758ebd13 100644 --- a/test/integration/custom-routes/next.config.js +++ b/test/integration/custom-routes/next.config.js @@ -124,6 +124,10 @@ module.exports = { source: '/unnamed/(first|second)/(.*)', destination: '/:1/:2', }, + { + source: '/named-like-unnamed/:0', + destination: '/:0', + }, ] }, diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js index 23bc4ebec8bedbc..649a6531b31d46a 100644 --- a/test/integration/custom-routes/test/index.test.js +++ b/test/integration/custom-routes/test/index.test.js @@ -238,13 +238,32 @@ const runTests = (isDev = false) => { expect(pathname).toBe('/first/final') }) + it('should support named like unnamed parameters correctly', async () => { + const res = await fetchViaHTTP( + appPort, + '/named-like-unnamed/first', + undefined, + { + redirect: 'manual', + } + ) + const { pathname } = url.parse(res.headers.get('location') || '') + expect(res.status).toBe(307) + expect(pathname).toBe('/first') + }) + if (!isDev) { it('should output routes-manifest successfully', async () => { const manifest = await fs.readJSON( join(appDir, '.next/routes-manifest.json') ) - for (const route of [...manifest.dynamicRoutes, ...manifest.rewrites]) { + for (const route of [ + ...manifest.dynamicRoutes, + ...manifest.rewrites, + ...manifest.redirects, + ...manifest.headers, + ]) { route.regex = normalizeRegEx(route.regex) } @@ -354,6 +373,13 @@ const runTests = (isDev = false) => { source: '/unnamed/(first|second)/(.*)', statusCode: 307, }, + { + destination: '/:0', + regex: normalizeRegEx('^\\/named-like-unnamed(?:\\/([^\\/]+?))$'), + regexKeys: ['0'], + source: '/named-like-unnamed/:0', + statusCode: 307, + }, ], headers: [ { diff --git a/test/integration/dynamic-routing/test/index.test.js b/test/integration/dynamic-routing/test/index.test.js index 492639cbd899493..a2cac95c722b4aa 100644 --- a/test/integration/dynamic-routing/test/index.test.js +++ b/test/integration/dynamic-routing/test/index.test.js @@ -389,6 +389,10 @@ function runTests(dev) { join(appDir, '.next/routes-manifest.json') ) + for (const route of manifest.dynamicRoutes) { + route.regex = normalizeRegEx(route.regex) + } + expect(manifest).toEqual({ version: 1, basePath: '', From 0bbb912bff4d6472cd3eed8c2d935cd98ed3d55f Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Mon, 6 Jan 2020 15:13:00 -0600 Subject: [PATCH 3/4] Removed un-used regexKeys --- packages/next/build/index.ts | 1 - .../custom-routes/test/index.test.js | 30 ------------------- 2 files changed, 31 deletions(-) diff --git a/packages/next/build/index.ts b/packages/next/build/index.ts index 3f231929b43fd92..3e9e6e5aa6b98b3 100644 --- a/packages/next/build/index.ts +++ b/packages/next/build/index.ts @@ -250,7 +250,6 @@ export default async function build(dir: string, conf = null): Promise { } : {}), regex: routeRegex.source, - regexKeys: keys.map(k => k.name), } } diff --git a/test/integration/custom-routes/test/index.test.js b/test/integration/custom-routes/test/index.test.js index 649a6531b31d46a..ebfcf4add0322c1 100644 --- a/test/integration/custom-routes/test/index.test.js +++ b/test/integration/custom-routes/test/index.test.js @@ -276,82 +276,70 @@ const runTests = (isDev = false) => { destination: '/docs/v2/network/status-codes#:code', statusCode: 301, regex: normalizeRegEx('^\\/docs\\/router-status(?:\\/([^\\/]+?))$'), - regexKeys: ['code'], }, { source: '/docs/github', destination: '/docs/v2/advanced/now-for-github', statusCode: 301, regex: normalizeRegEx('^\\/docs\\/github$'), - regexKeys: [], }, { source: '/docs/v2/advanced/:all(.*)', destination: '/docs/v2/more/:all', statusCode: 301, regex: normalizeRegEx('^\\/docs\\/v2\\/advanced(?:\\/(.*))$'), - regexKeys: ['all'], }, { source: '/hello/:id/another', destination: '/blog/:id', statusCode: 307, regex: normalizeRegEx('^\\/hello(?:\\/([^\\/]+?))\\/another$'), - regexKeys: ['id'], }, { source: '/redirect1', destination: '/', statusCode: 307, regex: normalizeRegEx('^\\/redirect1$'), - regexKeys: [], }, { source: '/redirect2', destination: '/', statusCode: 301, regex: normalizeRegEx('^\\/redirect2$'), - regexKeys: [], }, { source: '/redirect3', destination: '/another', statusCode: 302, regex: normalizeRegEx('^\\/redirect3$'), - regexKeys: [], }, { source: '/redirect4', destination: '/', statusCode: 308, regex: normalizeRegEx('^\\/redirect4$'), - regexKeys: [], }, { source: '/redir-chain1', destination: '/redir-chain2', statusCode: 301, regex: normalizeRegEx('^\\/redir-chain1$'), - regexKeys: [], }, { source: '/redir-chain2', destination: '/redir-chain3', statusCode: 302, regex: normalizeRegEx('^\\/redir-chain2$'), - regexKeys: [], }, { source: '/redir-chain3', destination: '/', statusCode: 303, regex: normalizeRegEx('^\\/redir-chain3$'), - regexKeys: [], }, { destination: 'https://google.com', regex: normalizeRegEx('^\\/to-external$'), - regexKeys: [], source: '/to-external', statusCode: 307, }, @@ -360,7 +348,6 @@ const runTests = (isDev = false) => { regex: normalizeRegEx( '^\\/query-redirect(?:\\/([^\\/]+?))(?:\\/([^\\/]+?))$' ), - regexKeys: ['section', 'name'], source: '/query-redirect/:section/:name', statusCode: 307, }, @@ -369,14 +356,12 @@ const runTests = (isDev = false) => { regex: normalizeRegEx( '^\\/unnamed(?:\\/(first|second))(?:\\/(.*))$' ), - regexKeys: [0, 1], source: '/unnamed/(first|second)/(.*)', statusCode: 307, }, { destination: '/:0', regex: normalizeRegEx('^\\/named-like-unnamed(?:\\/([^\\/]+?))$'), - regexKeys: ['0'], source: '/named-like-unnamed/:0', statusCode: 307, }, @@ -394,7 +379,6 @@ const runTests = (isDev = false) => { }, ], regex: normalizeRegEx('^\\/add-header$'), - regexKeys: [], source: '/add-header', }, { @@ -409,7 +393,6 @@ const runTests = (isDev = false) => { }, ], regex: normalizeRegEx('^\\/my-headers(?:\\/(.*))$'), - regexKeys: [0], source: '/my-headers/(.*)', }, ], @@ -417,56 +400,47 @@ const runTests = (isDev = false) => { { destination: '/another/one', regex: normalizeRegEx('^\\/to-another$'), - regexKeys: [], source: '/to-another', }, { source: '/hello-world', destination: '/static/hello.txt', regex: normalizeRegEx('^\\/hello-world$'), - regexKeys: [], }, { source: '/', destination: '/another', regex: normalizeRegEx('^\\/$'), - regexKeys: [], }, { source: '/another', destination: '/multi-rewrites', regex: normalizeRegEx('^\\/another$'), - regexKeys: [], }, { source: '/first', destination: '/hello', regex: normalizeRegEx('^\\/first$'), - regexKeys: [], }, { source: '/second', destination: '/hello-again', regex: normalizeRegEx('^\\/second$'), - regexKeys: [], }, { destination: '/hello', regex: normalizeRegEx('^\\/to-hello$'), - regexKeys: [], source: '/to-hello', }, { destination: '/blog/post-2', regex: normalizeRegEx('^\\/blog\\/post-1$'), - regexKeys: [], source: '/blog/post-1', }, { source: '/test/:path', destination: '/:path', regex: normalizeRegEx('^\\/test(?:\\/([^\\/]+?))$'), - regexKeys: ['path'], }, { source: '/test-overwrite/:something/:another', @@ -474,20 +448,17 @@ const runTests = (isDev = false) => { regex: normalizeRegEx( '^\\/test-overwrite(?:\\/([^\\/]+?))(?:\\/([^\\/]+?))$' ), - regexKeys: ['something', 'another'], }, { source: '/params/:something', destination: '/with-params', regex: normalizeRegEx('^\\/params(?:\\/([^\\/]+?))$'), - regexKeys: ['something'], }, { destination: '/with-params?first=:section&second=:name', regex: normalizeRegEx( '^\\/query-rewrite(?:\\/([^\\/]+?))(?:\\/([^\\/]+?))$' ), - regexKeys: ['section', 'name'], source: '/query-rewrite/:section/:name', }, { @@ -495,7 +466,6 @@ const runTests = (isDev = false) => { regex: normalizeRegEx( '^\\/hidden\\/_next(?:\\/((?:[^\\/]+?)(?:\\/(?:[^\\/]+?))*))?$' ), - regexKeys: ['path'], source: '/hidden/_next/:path*', }, ], From 9585c303f43237139b65bade70fc50131570b84b Mon Sep 17 00:00:00 2001 From: JJ Kasper Date: Thu, 16 Jan 2020 13:01:42 -0600 Subject: [PATCH 4/4] Update test routes --- test/integration/custom-routes/next.config.js | 2 ++ 1 file changed, 2 insertions(+) diff --git a/test/integration/custom-routes/next.config.js b/test/integration/custom-routes/next.config.js index 3f87da4c58512d0..3bff663bc6a378e 100644 --- a/test/integration/custom-routes/next.config.js +++ b/test/integration/custom-routes/next.config.js @@ -127,10 +127,12 @@ module.exports = { { source: '/unnamed/(first|second)/(.*)', destination: '/:1/:2', + permanent: false, }, { source: '/named-like-unnamed/:0', destination: '/:0', + permanent: false, }, ] },