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

Fix inconsistent handling for /index #11643

Merged
merged 7 commits into from
Apr 7, 2020
Merged
Show file tree
Hide file tree
Changes from 6 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion packages/next/next-server/server/require.ts
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ export function getPagePath(
}

if (!pagesManifest[page]) {
const cleanedPage = page.replace(/\/index$/, '') || '/'
const cleanedPage = page.replace(/^\/index$/, '') || '/'
if (!pagesManifest[cleanedPage]) {
throw pageNotFoundError(page)
} else {
Expand Down
11 changes: 9 additions & 2 deletions packages/next/server/lib/find-page-file.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,15 @@ export async function findPageFile(
const relativePagePath = `${normalizedPagePath}.${extension}`
const pagePath = join(rootDir, relativePagePath)

if (await isWriteable(pagePath)) {
foundPagePaths.push(relativePagePath)
// only /index and /sub/index when /sub/index/index.js is allowed
// see test/integration/route-indexes for expected index handling
if (
normalizedPagePath.startsWith('/index') ||
!normalizedPagePath.endsWith('/index')
) {
if (await isWriteable(pagePath)) {
foundPagePaths.push(relativePagePath)
}
}

const relativePagePathWithIndex = join(
Expand Down
2 changes: 1 addition & 1 deletion test/integration/client-navigation/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ describe('Client Navigation', () => {
'/nav/as-path-using-router',
'/nav/url-prop-change',

'/nested-cdm/index',
'/nested-cdm',
]
await Promise.all(
prerender.map(route => renderViaHTTP(context.appPort, route))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ describe('Errors on conflict between public file and page file', () => {
it('Throws error during development', async () => {
const appPort = await findPort()
const app = await launchApp(appDir, appPort)
const conflicts = ['/another/conflict', '/another/index', '/hello']
const conflicts = ['/another/conflict', '/hello']

for (const conflict of conflicts) {
const html = await renderViaHTTP(appPort, conflict)
Expand Down
2 changes: 1 addition & 1 deletion test/integration/custom-server/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ app.prepare().then(() => {
}

if (/dashboard/.test(req.url)) {
return app.render(req, res, '/dashboard/index')
return app.render(req, res, '/dashboard')
}

if (/static\/hello\.text/.test(req.url)) {
Expand Down
2 changes: 2 additions & 0 deletions test/integration/route-index/pages/index/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const page = () => 'hello from index'
export default page
60 changes: 60 additions & 0 deletions test/integration/route-index/test/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,60 @@
/* eslint-env jest */
/* global jasmine */
import { join } from 'path'
import {
findPort,
launchApp,
killApp,
nextBuild,
nextStart,
fetchViaHTTP,
} from 'next-test-utils'

jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 2

let app
let appPort
const appDir = join(__dirname, '../')

const runTests = () => {
it('should handle / correctly', async () => {
const res = await fetchViaHTTP(appPort, '/')
expect(res.status).toBe(200)
expect(await res.text()).toContain('hello from index')
})

it('should handle /index correctly', async () => {
const res = await fetchViaHTTP(appPort, '/index')
expect(res.status).toBe(200)
expect(await res.text()).toContain('hello from index')
})

it('should handle /index/index correctly', async () => {
const res = await fetchViaHTTP(appPort, '/index/index')
expect(res.status).toBe(404)
expect(await res.text()).toContain('page could not be found')
})
}

describe('Route index handling', () => {
describe('dev mode', () => {
beforeAll(async () => {
appPort = await findPort()
app = await launchApp(appDir, appPort)
})
afterAll(() => killApp(app))

runTests()
})

describe('production mode', () => {
beforeAll(async () => {
await nextBuild(appDir)
appPort = await findPort()
app = await nextStart(appDir, appPort)
})
afterAll(() => killApp(app))

runTests()
})
})
1 change: 1 addition & 0 deletions test/integration/route-indexes/pages/api/sub/[id].js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default (req, res) => res.end('hi from sub id')
1 change: 1 addition & 0 deletions test/integration/route-indexes/pages/api/sub/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default (req, res) => res.end('hi from sub index')
2 changes: 2 additions & 0 deletions test/integration/route-indexes/pages/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
const page = () => 'hello from index'
export default page
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
export default () => 'hello from nested index'
3 changes: 3 additions & 0 deletions test/integration/route-indexes/pages/sub/[id].js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const page = () => 'hello from sub id'
page.getInitialProps = () => ({ hello: 'hi' })
export default page
3 changes: 3 additions & 0 deletions test/integration/route-indexes/pages/sub/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
const page = () => 'hello from sub index'
page.getInitialProps = () => ({ hello: 'hi' })
export default page
138 changes: 138 additions & 0 deletions test/integration/route-indexes/test/index.test.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,138 @@
/* eslint-env jest */
/* global jasmine */
import { join } from 'path'
import {
findPort,
launchApp,
killApp,
nextBuild,
nextStart,
fetchViaHTTP,
} from 'next-test-utils'

jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 2

let app
let appPort
const appDir = join(__dirname, '../')

const runTests = () => {
it('should handle / correctly', async () => {
const res = await fetchViaHTTP(appPort, '/')
expect(res.status).toBe(200)
expect(await res.text()).toContain('hello from index')
})

it('should handle /index correctly', async () => {
const res = await fetchViaHTTP(appPort, '/index')
expect(res.status).toBe(200)
expect(await res.text()).toContain('hello from index')
})

it('should handle /index/index correctly', async () => {
const res = await fetchViaHTTP(appPort, '/index/index')
expect(res.status).toBe(404)
expect(await res.text()).toContain('page could not be found')
})

it('should handle /nested-index correctly', async () => {
const res = await fetchViaHTTP(appPort, '/nested-index')
expect(res.status).toBe(404)
expect(await res.text()).toContain('page could not be found')
})

it('should handle /nested-index/index correctly', async () => {
ijjk marked this conversation as resolved.
Show resolved Hide resolved
const res = await fetchViaHTTP(appPort, '/nested-index/index')
expect(res.status).toBe(200)
expect(await res.text()).toContain('hello from nested index')
})

it('should handle /nested-index/index/index correctly', async () => {
const res = await fetchViaHTTP(appPort, '/nested-index/index/index')
expect(res.status).toBe(404)
expect(await res.text()).toContain('page could not be found')
})

it('should handle /sub correctly', async () => {
const res = await fetchViaHTTP(appPort, '/sub')
expect(res.status).toBe(200)
expect(await res.text()).toContain('hello from sub index')
})

it('should handle /sub/index correctly', async () => {
const res = await fetchViaHTTP(appPort, '/sub/index')
expect(res.status).toBe(200)
expect(await res.text()).toContain('hello from sub id')
})

it('should handle /sub/index/index correctly', async () => {
const res = await fetchViaHTTP(appPort, '/sub/index/index')
expect(res.status).toBe(404)
expect(await res.text()).toContain('page could not be found')
})

it('should handle /sub/another correctly', async () => {
const res = await fetchViaHTTP(appPort, '/sub/another')
expect(res.status).toBe(200)
expect(await res.text()).toContain('hello from sub id')
})

it('should handle /sub/another/index correctly', async () => {
const res = await fetchViaHTTP(appPort, '/sub/another/index')
expect(res.status).toBe(404)
expect(await res.text()).toContain('page could not be found')
})

it('should handle /api/sub correctly', async () => {
const res = await fetchViaHTTP(appPort, '/api/sub')
expect(res.status).toBe(200)
expect(await res.text()).toContain('hi from sub index')
})

it('should handle /api/sub/index correctly', async () => {
const res = await fetchViaHTTP(appPort, '/api/sub/index')
expect(res.status).toBe(200)
expect(await res.text()).toContain('hi from sub id')
})

it('should handle /api/sub/index/index correctly', async () => {
const res = await fetchViaHTTP(appPort, '/api/sub/index/index')
expect(res.status).toBe(404)
expect(await res.text()).toContain('page could not be found')
})

it('should handle /api/sub/another correctly', async () => {
const res = await fetchViaHTTP(appPort, '/api/sub/another')
expect(res.status).toBe(200)
expect(await res.text()).toContain('hi from sub id')
})

it('should handle /api/sub/another/index correctly', async () => {
const res = await fetchViaHTTP(appPort, '/api/sub/another/index')
expect(res.status).toBe(404)
expect(await res.text()).toContain('page could not be found')
})
}

describe('Route indexes handling', () => {
describe('dev mode', () => {
beforeAll(async () => {
appPort = await findPort()
app = await launchApp(appDir, appPort)
})
afterAll(() => killApp(app))

runTests()
})

describe('production mode', () => {
beforeAll(async () => {
await nextBuild(appDir)
appPort = await findPort()
app = await nextStart(appDir, appPort)
})
afterAll(() => killApp(app))

runTests()
})
})