1b36f0c029
Disambiguate between pages/index.js and pages/index/index.js so that they resolve differently. It all started with a bug in pagesmanifest that propagated throughout the codebase. After fixing pagesmanifest I was able to remove a few hacks here and there and more logic is shared now. especially the logic that resolves an entrypoint back into a route path. To sum up what happened: - `getRouteFromEntrypoint` is the inverse operation of `getPageFile` that's under `pages/_document.tsx` - `denormalizePagePath` is the inverse operation of `normalizePagePath`. Everything is refactored in terms of these operations, that makes their behavior uniform and easier to update/patch in a central place. Before there were subtle differences between those that made `index/index.js` hard to handle. Some potential follow up on this PR: - [`hot-reloader`](https://github.com/vercel/next.js/pull/13699/files#diff-6161346d2c5f4b7abc87059d8768c44bR207) still has one place that does very similar behavior to `getRouteFromEntrypoint`. It can probably be rewritten in terms of `getRouteFromEntrypoint`. - There are a few places where `denormalizePagePath(normalizePagePath(...))` is happening. This is a sign that `normalizePagePath` is doing some validation that is independent of its rewriting logic. That should probably be factored out in its own function. after that I should probably investigate whether `normalizePagePath` is even still needed at all. - a lot of code is doing `.replace(/\\/g, '')`. If wanted, that could be replaced with `normalizePathSep`. - It looks to me like some logic that's spread across the project can be centralized in 4 functions - `getRouteFromEntrypoint` (part of this PR) - its inverse `getEntrypointFromRoute` (already exists in `_document.tsx` as `getPageFile`) - `getRouteFromPageFile` - its inverse `getPageFileFromRoute` (already exists as `findPageFile ` in `server/lib/find-page-file.ts`) It could be beneficial to structure the code to keep these fuctionalities close together and name them similarly. - revise `index.amp` handling in pagesmanifest. I left it alone in this PR to keep it scoped, but it may be broken wrt nested index files as well. It might even make sense to reshape the pagesmanifest altogether to handle html/json/amp/... better
119 lines
3.2 KiB
JavaScript
119 lines
3.2 KiB
JavaScript
/* eslint-env jest */
|
|
|
|
import { join } from 'path'
|
|
import { SERVER_DIRECTORY, CLIENT_STATIC_FILES_PATH } from 'next/constants'
|
|
import {
|
|
requirePage,
|
|
getPagePath,
|
|
pageNotFoundError,
|
|
} from 'next/dist/next-server/server/require'
|
|
import { normalizePagePath } from 'next/dist/next-server/server/normalize-page-path'
|
|
|
|
const sep = '/'
|
|
const distDir = join(__dirname, '_resolvedata')
|
|
const pathToBundles = join(
|
|
distDir,
|
|
SERVER_DIRECTORY,
|
|
CLIENT_STATIC_FILES_PATH,
|
|
'development',
|
|
'pages'
|
|
)
|
|
|
|
describe('pageNotFoundError', () => {
|
|
it('Should throw error with ENOENT code', () => {
|
|
expect.assertions(1)
|
|
try {
|
|
throw pageNotFoundError('test')
|
|
} catch (err) {
|
|
// eslint-disable-next-line jest/no-try-expect
|
|
expect(err.code).toBe('ENOENT')
|
|
}
|
|
})
|
|
})
|
|
|
|
describe('normalizePagePath', () => {
|
|
it('Should turn / into /index', () => {
|
|
expect(normalizePagePath('/')).toBe(`${sep}index`)
|
|
})
|
|
|
|
it('Should turn _error into /_error', () => {
|
|
expect(normalizePagePath('_error')).toBe(`${sep}_error`)
|
|
})
|
|
|
|
it('Should turn /abc into /abc', () => {
|
|
expect(normalizePagePath('/abc')).toBe(`${sep}abc`)
|
|
})
|
|
|
|
it('Should turn /abc/def into /abc/def', () => {
|
|
expect(normalizePagePath('/abc/def')).toBe(`${sep}abc${sep}def`)
|
|
})
|
|
|
|
it('Should throw on /../../test.js', () => {
|
|
expect(() => normalizePagePath('/../../test.js')).toThrow()
|
|
})
|
|
})
|
|
|
|
describe('getPagePath', () => {
|
|
it('Should not append /index to the / page', () => {
|
|
expect(() => getPagePath('/', distDir)).toThrow(
|
|
'Cannot find module for page: /'
|
|
)
|
|
})
|
|
|
|
it('Should prepend / when a page does not have it', () => {
|
|
const pagePath = getPagePath('_error', distDir)
|
|
expect(pagePath).toBe(join(pathToBundles, `${sep}_error.js`))
|
|
})
|
|
|
|
it('Should throw with paths containing ../', () => {
|
|
expect(() => getPagePath('/../../package.json', distDir)).toThrow()
|
|
})
|
|
})
|
|
|
|
describe('requirePage', () => {
|
|
it('Should not find page /index when using /', async () => {
|
|
await expect(() => requirePage('/', distDir)).toThrow(
|
|
'Cannot find module for page: /'
|
|
)
|
|
})
|
|
|
|
it('Should require /index.js when using /index', async () => {
|
|
const page = await requirePage('/index', distDir)
|
|
expect(page.test).toBe('hello')
|
|
})
|
|
|
|
it('Should require /world.js when using /world', async () => {
|
|
const page = await requirePage('/world', distDir)
|
|
expect(page.test).toBe('world')
|
|
})
|
|
|
|
it('Should throw when using /../../test.js', async () => {
|
|
expect.assertions(1)
|
|
try {
|
|
await requirePage('/../../test', distDir)
|
|
} catch (err) {
|
|
// eslint-disable-next-line jest/no-try-expect
|
|
expect(err.code).toBe('ENOENT')
|
|
}
|
|
})
|
|
|
|
it('Should throw when using non existent pages like /non-existent.js', async () => {
|
|
expect.assertions(1)
|
|
try {
|
|
await requirePage('/non-existent', distDir)
|
|
} catch (err) {
|
|
// eslint-disable-next-line jest/no-try-expect
|
|
expect(err.code).toBe('ENOENT')
|
|
}
|
|
})
|
|
|
|
it('Should bubble up errors in the child component', async () => {
|
|
expect.assertions(1)
|
|
try {
|
|
await requirePage('/non-existent-child', distDir)
|
|
} catch (err) {
|
|
// eslint-disable-next-line jest/no-try-expect
|
|
expect(err.code).toBe('MODULE_NOT_FOUND')
|
|
}
|
|
})
|
|
})
|