Skip to content

Commit

Permalink
Fix dynamic routes with pages under index folder (#32440)
Browse files Browse the repository at this point in the history
Fixes incorrect generated manifest and generated directory for `index/[...dynamic]` pages

Too much normalizing adding extra `index/` prefix to `index/[...dynamic]` routes which lead to the incorrected generated routes like `.next/server/pages/index/index/index/[...dynamic]`

## Bug

Fixes vercel/customer-issues#146

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`
  • Loading branch information
huozhi committed Dec 13, 2021
1 parent 48cee54 commit 10d814d
Show file tree
Hide file tree
Showing 6 changed files with 65 additions and 9 deletions.
6 changes: 4 additions & 2 deletions packages/next/export/worker.ts
Expand Up @@ -215,8 +215,10 @@ export default async function exportPage({
subFolders ? `${_path}${sep}index.html` : `${_path}.html`
let htmlFilename = getHtmlFilename(filePath)

const pageExt = extname(page)
const pathExt = extname(path)
// dynamic routes can provide invalid extensions e.g. /blog/[...slug] returns an
// extension of `.slug]`
const pageExt = isDynamic ? '' : extname(page)
const pathExt = isDynamic ? '' : extname(path)
// Make sure page isn't a folder with a dot in the name e.g. `v1.2`
if (pageExt !== pathExt && pathExt !== '') {
const isBuiltinPaths = ['/500', '/404'].some(
Expand Down
4 changes: 3 additions & 1 deletion packages/next/server/denormalize-page-path.ts
@@ -1,10 +1,12 @@
import { isDynamicRoute } from '../shared/lib/router/utils'

export function normalizePathSep(path: string): string {
return path.replace(/\\/g, '/')
}

export function denormalizePagePath(page: string) {
page = normalizePathSep(page)
if (page.startsWith('/index/')) {
if (page.startsWith('/index/') && !isDynamicRoute(page)) {
page = page.slice(6)
} else if (page === '/index') {
page = '/'
Expand Down
3 changes: 2 additions & 1 deletion packages/next/server/normalize-page-path.ts
@@ -1,12 +1,13 @@
import { posix } from 'path'
import { isDynamicRoute } from '../shared/lib/router/utils'

export { normalizePathSep, denormalizePagePath } from './denormalize-page-path'

export function normalizePagePath(page: string): string {
// If the page is `/` we need to append `/index`, otherwise the returned directory root will be bundles instead of pages
if (page === '/') {
page = '/index'
} else if (/^\/index(\/|$)/.test(page)) {
} else if (/^\/index(\/|$)/.test(page) && !isDynamicRoute(page)) {
page = `/index${page}`
}
// Resolve on anything that doesn't start with `/`
Expand Down
@@ -1,4 +1,7 @@
// Translate a pages asset path (relative from a common prefix) back into its logical route

import { isDynamicRoute } from './is-dynamic'

// "asset path" being its javascript file, data file, prerendered html,...
export default function getRouteFromAssetPath(
assetPath: string,
Expand All @@ -7,7 +10,7 @@ export default function getRouteFromAssetPath(
assetPath = assetPath.replace(/\\/g, '/')
assetPath =
ext && assetPath.endsWith(ext) ? assetPath.slice(0, -ext.length) : assetPath
if (assetPath.startsWith('/index/')) {
if (assetPath.startsWith('/index/') && !isDynamicRoute(assetPath)) {
assetPath = assetPath.slice(6)
} else if (assetPath === '/index') {
assetPath = '/'
Expand Down
3 changes: 3 additions & 0 deletions test/integration/dynamic-routing/pages/index/[...slug].js
@@ -0,0 +1,3 @@
export default function page() {
return 'index/[...slug]'
}
53 changes: 49 additions & 4 deletions test/integration/dynamic-routing/test/index.test.js
Expand Up @@ -27,7 +27,7 @@ let buildId
const appDir = join(__dirname, '../')
const buildIdPath = join(appDir, '.next/BUILD_ID')

function runTests(dev) {
function runTests({ dev, serverless }) {
if (dev) {
it('should not have error after pinging WebSocket', async () => {
const browser = await webdriver(appPort, '/')
Expand Down Expand Up @@ -1256,6 +1256,14 @@ function runTests(dev) {
helloworld: 'hello-world',
},
},
{
namedRegex: '^/index/(?<slug>.+?)(?:/)?$',
page: '/index/[...slug]',
regex: normalizeRegEx('^/index/(.+?)(?:/)?$'),
routeKeys: {
slug: 'slug',
},
},
{
namedRegex: `^/on\\-mount/(?<post>[^/]+?)(?:/)?$`,
page: '/on-mount/[post]',
Expand Down Expand Up @@ -1338,6 +1346,43 @@ function runTests(dev) {
],
})
})

if (!serverless) {
it('should output a pages-manifest correctly', async () => {
const manifest = await fs.readJson(
join(appDir, '.next/server/pages-manifest.json')
)

expect(manifest).toEqual({
'/[name]/[comment]': 'pages/[name]/[comment].js',
'/[name]/comments': 'pages/[name]/comments.js',
'/[name]': 'pages/[name].js',
'/[name]/on-mount-redir': 'pages/[name]/on-mount-redir.html',
'/another': 'pages/another.html',
'/b/[123]': 'pages/b/[123].js',
'/blog/[name]/comment/[id]': 'pages/blog/[name]/comment/[id].js',
'/c/[alongparamnameshouldbeallowedeventhoughweird]':
'pages/c/[alongparamnameshouldbeallowedeventhoughweird].js',
'/catchall-dash/[...hello-world]':
'pages/catchall-dash/[...hello-world].html',
'/d/[id]': 'pages/d/[id].html',
'/dash/[hello-world]': 'pages/dash/[hello-world].html',
'/': 'pages/index.html',
'/index/[...slug]': 'pages/index/[...slug].html',
'/on-mount/[post]': 'pages/on-mount/[post].html',
'/p1/p2/all-ssg/[...rest]': 'pages/p1/p2/all-ssg/[...rest].js',
'/p1/p2/all-ssr/[...rest]': 'pages/p1/p2/all-ssr/[...rest].js',
'/p1/p2/nested-all-ssg/[...rest]':
'pages/p1/p2/nested-all-ssg/[...rest].js',
'/p1/p2/predefined-ssg/[...rest]':
'pages/p1/p2/predefined-ssg/[...rest].js',
'/_app': 'pages/_app.js',
'/_error': 'pages/_error.js',
'/_document': 'pages/_document.js',
'/404': 'pages/404.html',
})
})
}
}
}

Expand All @@ -1354,7 +1399,7 @@ describe('Dynamic Routing', () => {
})
afterAll(() => killApp(app))

runTests(true)
runTests({ dev: true, serverless: false })
})

describe('production mode', () => {
Expand All @@ -1369,7 +1414,7 @@ describe('Dynamic Routing', () => {
})
afterAll(() => killApp(app))

runTests()
runTests({ dev: false, serverless: false })
})

describe('serverless mode', () => {
Expand All @@ -1389,6 +1434,6 @@ describe('Dynamic Routing', () => {
await killApp(app)
await fs.remove(nextConfig)
})
runTests()
runTests({ dev: false, serverless: true })
})
})

0 comments on commit 10d814d

Please sign in to comment.