Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update routeKeys to handle non-word characters #12801

Merged
merged 11 commits into from
Jun 16, 2020
6 changes: 3 additions & 3 deletions packages/next/build/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -295,8 +295,8 @@ export default async function build(dir: string, conf = null): Promise<void> {
return {
page,
regex: routeRegex.re.source,
routeKeys: routeRegex.routeKeys,
namedRegex: routeRegex.namedRegex,
routeKeys: Object.keys(routeRegex.groups),
}
}),
}
Expand Down Expand Up @@ -640,8 +640,8 @@ export default async function build(dir: string, conf = null): Promise<void> {
)

let dataRouteRegex: string
let routeKeys: string[] | undefined
let namedDataRouteRegex: string | undefined
let routeKeys: { [named: string]: string } | undefined

if (isDynamicRoute(page)) {
const routeRegex = getRouteRegex(dataRoute.replace(/\.json$/, ''))
Expand All @@ -654,7 +654,7 @@ export default async function build(dir: string, conf = null): Promise<void> {
/\(\?:\/\)\?\$$/,
'\\.json$'
)
routeKeys = Object.keys(routeRegex.groups)
routeKeys = routeRegex.routeKeys
} else {
dataRouteRegex = new RegExp(
`^${path.posix.join(
Expand Down
13 changes: 11 additions & 2 deletions packages/next/next-server/lib/router/utils/route-regex.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ export function getRouteRegex(
): {
re: RegExp
namedRegex?: string
routeKeys?: { [named: string]: string }
groups: { [groupName: string]: { pos: number; repeat: boolean } }
} {
// Escape all characters that could be considered RegEx
Expand All @@ -33,6 +34,7 @@ export function getRouteRegex(
)

let namedParameterizedRoute: string | undefined
const routeKeys: { [named: string]: string } = {}

// dead code eliminate for browser since it's only needed
// while generating routes-manifest
Expand All @@ -46,9 +48,15 @@ export function getRouteRegex(
.replace(/\\([|\\{}()[\]^$+*?.-])/g, '$1')
.replace(/^\.{3}/, '')

// replace any non-word characters since they can break
// the named regex
const cleanedKey = key.replace(/\W/g, '')

routeKeys[cleanedKey] = key

return isCatchAll
? `/(?<${escapeRegex(key)}>.+?)`
: `/(?<${escapeRegex(key)}>[^/]+?)`
? `/(?<${cleanedKey}>.+?)`
: `/(?<${cleanedKey}>[^/]+?)`
}
)
}
Expand All @@ -58,6 +66,7 @@ export function getRouteRegex(
groups,
...(namedParameterizedRoute
? {
routeKeys,
namedRegex: `^${namedParameterizedRoute}(?:/)?$`,
}
: {}),
Expand Down
18 changes: 13 additions & 5 deletions packages/next/next-server/lib/router/utils/sorted-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -93,11 +93,19 @@ class UrlNode {
}
}

if (slugNames.indexOf(nextSlug) !== -1) {
throw new Error(
`You cannot have the same slug name "${nextSlug}" repeat within a single dynamic path`
)
}
slugNames.forEach(slug => {
if (slug === nextSlug) {
throw new Error(
`You cannot have the same slug name "${nextSlug}" repeat within a single dynamic path`
)
}

if (slug.replace(/\W/g, '') === nextSegment.replace(/\W/g, '')) {
throw new Error(
`You cannot have the slug names "${slug}" and "${nextSlug}" differ only by non-word symbols within a single dynamic path`
)
}
})

slugNames.push(nextSlug)
}
Expand Down
12 changes: 9 additions & 3 deletions test/integration/custom-routes/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -827,19 +827,25 @@ const runTests = (isDev = false) => {
namedRegex: '^/another/(?<id>[^/]+?)(?:/)?$',
page: '/another/[id]',
regex: normalizeRegEx('^\\/another\\/([^\\/]+?)(?:\\/)?$'),
routeKeys: ['id'],
routeKeys: {
id: 'id',
},
},
{
namedRegex: '^/api/dynamic/(?<slug>[^/]+?)(?:/)?$',
page: '/api/dynamic/[slug]',
regex: normalizeRegEx('^\\/api\\/dynamic\\/([^\\/]+?)(?:\\/)?$'),
routeKeys: ['slug'],
routeKeys: {
slug: 'slug',
},
},
{
namedRegex: '^/blog/(?<post>[^/]+?)(?:/)?$',
page: '/blog/[post]',
regex: normalizeRegEx('^\\/blog\\/([^\\/]+?)(?:\\/)?$'),
routeKeys: ['post'],
routeKeys: {
post: 'post',
},
},
],
})
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default () => 'hi'
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default () => 'hi'
78 changes: 65 additions & 13 deletions test/integration/dynamic-routing/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -517,10 +517,18 @@ function runTests(dev) {

for (const route of manifest.dynamicRoutes) {
route.regex = normalizeRegEx(route.regex)

// ensure regexes are valid
new RegExp(route.regex)
new RegExp(route.namedRegex)
}

for (const route of manifest.dataRoutes) {
route.dataRouteRegex = normalizeRegEx(route.dataRouteRegex)

// ensure regexes are valid
new RegExp(route.dataRouteRegex)
new RegExp(route.namedDataRouteRegex)
}

expect(manifest).toEqual({
Expand All @@ -541,7 +549,9 @@ function runTests(dev) {
)}\\/p1\\/p2\\/all\\-ssg\\/(.+?)\\.json$`
),
page: '/p1/p2/all-ssg/[...rest]',
routeKeys: ['rest'],
routeKeys: {
rest: 'rest',
},
},
{
namedDataRouteRegex: `^/_next/data/${escapeRegex(
Expand All @@ -553,7 +563,9 @@ function runTests(dev) {
)}\\/p1\\/p2\\/nested\\-all\\-ssg\\/(.+?)\\.json$`
),
page: '/p1/p2/nested-all-ssg/[...rest]',
routeKeys: ['rest'],
routeKeys: {
rest: 'rest',
},
},
{
namedDataRouteRegex: `^/_next/data/${escapeRegex(
Expand All @@ -565,7 +577,9 @@ function runTests(dev) {
)}\\/p1\\/p2\\/predefined\\-ssg\\/(.+?)\\.json$`
),
page: '/p1/p2/predefined-ssg/[...rest]',
routeKeys: ['rest'],
routeKeys: {
rest: 'rest',
},
},
],
dynamicRoutes: [
Expand All @@ -575,67 +589,105 @@ function runTests(dev) {
regex: normalizeRegEx(
'^\\/blog\\/([^\\/]+?)\\/comment\\/([^\\/]+?)(?:\\/)?$'
),
routeKeys: ['name', 'id'],
routeKeys: {
name: 'name',
id: 'id',
},
},
{
namedRegex: '^/catchall\\-dash/(?<helloworld>.+?)(?:/)?$',
page: '/catchall-dash/[...hello-world]',
regex: normalizeRegEx('^\\/catchall\\-dash\\/(.+?)(?:\\/)?$'),
routeKeys: {
helloworld: 'hello-world',
},
},
{
namedRegex: '^/dash/(?<helloworld>[^/]+?)(?:/)?$',
page: '/dash/[hello-world]',
regex: normalizeRegEx('^\\/dash\\/([^\\/]+?)(?:\\/)?$'),
routeKeys: {
helloworld: 'hello-world',
},
},
{
namedRegex: `^/on\\-mount/(?<post>[^/]+?)(?:/)?$`,
page: '/on-mount/[post]',
regex: normalizeRegEx('^\\/on\\-mount\\/([^\\/]+?)(?:\\/)?$'),
routeKeys: ['post'],
routeKeys: {
post: 'post',
},
},
{
namedRegex: `^/p1/p2/all\\-ssg/(?<rest>.+?)(?:/)?$`,
page: '/p1/p2/all-ssg/[...rest]',
regex: normalizeRegEx('^\\/p1\\/p2\\/all\\-ssg\\/(.+?)(?:\\/)?$'),
routeKeys: ['rest'],
routeKeys: {
rest: 'rest',
},
},
{
namedRegex: `^/p1/p2/all\\-ssr/(?<rest>.+?)(?:/)?$`,
page: '/p1/p2/all-ssr/[...rest]',
regex: normalizeRegEx('^\\/p1\\/p2\\/all\\-ssr\\/(.+?)(?:\\/)?$'),
routeKeys: ['rest'],
routeKeys: {
rest: 'rest',
},
},
{
namedRegex: `^/p1/p2/nested\\-all\\-ssg/(?<rest>.+?)(?:/)?$`,
page: '/p1/p2/nested-all-ssg/[...rest]',
regex: normalizeRegEx(
'^\\/p1\\/p2\\/nested\\-all\\-ssg\\/(.+?)(?:\\/)?$'
),
routeKeys: ['rest'],
routeKeys: {
rest: 'rest',
},
},
{
namedRegex: `^/p1/p2/predefined\\-ssg/(?<rest>.+?)(?:/)?$`,
page: '/p1/p2/predefined-ssg/[...rest]',
regex: normalizeRegEx(
'^\\/p1\\/p2\\/predefined\\-ssg\\/(.+?)(?:\\/)?$'
),
routeKeys: ['rest'],
routeKeys: {
rest: 'rest',
},
},
{
namedRegex: `^/(?<name>[^/]+?)(?:/)?$`,
page: '/[name]',
regex: normalizeRegEx('^\\/([^\\/]+?)(?:\\/)?$'),
routeKeys: ['name'],
routeKeys: {
name: 'name',
},
},
{
namedRegex: `^/(?<name>[^/]+?)/comments(?:/)?$`,
page: '/[name]/comments',
regex: normalizeRegEx('^\\/([^\\/]+?)\\/comments(?:\\/)?$'),
routeKeys: ['name'],
routeKeys: {
name: 'name',
},
},
{
namedRegex: `^/(?<name>[^/]+?)/on\\-mount\\-redir(?:/)?$`,
page: '/[name]/on-mount-redir',
regex: normalizeRegEx(
'^\\/([^\\/]+?)\\/on\\-mount\\-redir(?:\\/)?$'
),
routeKeys: ['name'],
routeKeys: {
name: 'name',
},
},
{
namedRegex: `^/(?<name>[^/]+?)/(?<comment>[^/]+?)(?:/)?$`,
page: '/[name]/[comment]',
regex: normalizeRegEx('^\\/([^\\/]+?)\\/([^\\/]+?)(?:\\/)?$'),
routeKeys: ['name', 'comment'],
routeKeys: {
name: 'name',
comment: 'comment',
},
},
],
})
Expand Down
17 changes: 13 additions & 4 deletions test/integration/getserversideprops/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,9 @@ const expectedManifestRoutes = () => [
`^\\/_next\\/data\\/${escapeRegex(buildId)}\\/blog\\/([^\\/]+?)\\.json$`
),
page: '/blog/[post]',
routeKeys: ['post'],
routeKeys: {
post: 'post',
},
},
{
namedDataRouteRegex: `^/_next/data/${escapeRegex(
Expand All @@ -67,7 +69,10 @@ const expectedManifestRoutes = () => [
)}\\/blog\\/([^\\/]+?)\\/([^\\/]+?)\\.json$`
),
page: '/blog/[post]/[comment]',
routeKeys: ['post', 'comment'],
routeKeys: {
post: 'post',
comment: 'comment',
},
},
{
namedDataRouteRegex: `^/_next/data/${escapeRegex(
Expand All @@ -77,7 +82,9 @@ const expectedManifestRoutes = () => [
`^\\/_next\\/data\\/${escapeRegex(buildId)}\\/catchall\\/(.+?)\\.json$`
),
page: '/catchall/[...path]',
routeKeys: ['path'],
routeKeys: {
path: 'path',
},
},
{
dataRouteRegex: normalizeRegEx(
Expand Down Expand Up @@ -125,7 +132,9 @@ const expectedManifestRoutes = () => [
)}\\/user\\/([^\\/]+?)\\/profile\\.json$`
),
page: '/user/[user]/profile',
routeKeys: ['user'],
routeKeys: {
user: 'user',
},
},
]

Expand Down
Loading