Skip to content

Commit

Permalink
Support unnamed parameters in custom-routes correctly (#9920)
Browse files Browse the repository at this point in the history 
* Support unnamed parameters in custom-routes correctly

* Update unnamed params checking to be strict

* Removed un-used regexKeys

* Update test routes

Co-authored-by: Joe Haddad <timer150@gmail.com>
  • Loading branch information
@ijjk @Timer
ijjk and Timer committed Jan 16, 2020
1 parent 6804039 commit 22e015f
Show file tree
Hide file tree
Showing 4 changed files with 79 additions and 32 deletions.
1 change: 0 additions & 1 deletion packages/next/build/index.ts
View file
Expand Up @@ -251,7 +251,6 @@ export default async function build(dir: string, conf = null): Promise<void> {
}
: {}),
regex: routeRegex.source,
regexKeys: keys.map(k => k.name),
}
}

Expand Down
28 changes: 26 additions & 2 deletions packages/next/next-server/server/lib/path-match.ts
View file
Expand Up @@ -4,18 +4,42 @@ 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)
if (!res) {
return false
}

if (customRoute) {
const newParams: { [k: string]: string } = {}
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,
}
}

return { ...params, ...res.params }
}
}
Expand Down
10 changes: 10 additions & 0 deletions test/integration/custom-routes/next.config.js
View file
Expand Up @@ -124,6 +124,16 @@ module.exports = {
destination: '/with-params?first=:section&second=:name',
permanent: false,
},
{
source: '/unnamed/(first|second)/(.*)',
destination: '/:1/:2',
permanent: false,
},
{
source: '/named-like-unnamed/:0',
destination: '/:0',
permanent: false,
},
]
},

Expand Down
72 changes: 43 additions & 29 deletions test/integration/custom-routes/test/index.test.js
View file
Expand Up @@ -229,6 +229,29 @@ 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')
})

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')
})

it('should add refresh header for 308 redirect', async () => {
const res = await fetchViaHTTP(appPort, '/redirect4', undefined, {
redirect: 'manual',
Expand All @@ -243,7 +266,12 @@ const runTests = (isDev = false) => {
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)
}

Expand All @@ -256,82 +284,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,
},
Expand All @@ -340,10 +356,23 @@ const runTests = (isDev = false) => {
regex: normalizeRegEx(
'^\\/query-redirect(?:\\/([^\\/]+?))(?:\\/([^\\/]+?))$'
),
regexKeys: ['section', 'name'],
source: '/query-redirect/:section/:name',
statusCode: 307,
},
{
destination: '/:1/:2',
regex: normalizeRegEx(
'^\\/unnamed(?:\\/(first|second))(?:\\/(.*))$'
),
source: '/unnamed/(first|second)/(.*)',
statusCode: 307,
},
{
destination: '/:0',
regex: normalizeRegEx('^\\/named-like-unnamed(?:\\/([^\\/]+?))$'),
source: '/named-like-unnamed/:0',
statusCode: 307,
},
],
headers: [
{
Expand All @@ -358,7 +387,6 @@ const runTests = (isDev = false) => {
},
],
regex: normalizeRegEx('^\\/add-header$'),
regexKeys: [],
source: '/add-header',
},
{
Expand All @@ -373,93 +401,79 @@ const runTests = (isDev = false) => {
},
],
regex: normalizeRegEx('^\\/my-headers(?:\\/(.*))$'),
regexKeys: [0],
source: '/my-headers/(.*)',
},
],
rewrites: [
{
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',
destination: '/params/this-should-be-the-value',
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',
},
{
destination: '/_next/:path*',
regex: normalizeRegEx(
'^\\/hidden\\/_next(?:\\/((?:[^\\/]+?)(?:\\/(?:[^\\/]+?))*))?$'
),
regexKeys: ['path'],
source: '/hidden/_next/:path*',
},
],
Expand Down

0 comments on commit 22e015f

Please sign in to comment.