Skip to content

Commit

Permalink
Fix inconsistent handling for /index (#11643)
Browse files Browse the repository at this point in the history
* Fix inconsistent handling for /index

* Add more test cases

* Add pages/index/index.js test

Co-authored-by: Joe Haddad <joe.haddad@zeit.co>
  • Loading branch information
ijjk and Timer authored Apr 7, 2020
1 parent bc7d183 commit cc96780
Show file tree
Hide file tree
Showing 14 changed files with 224 additions and 6 deletions.
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 () => {
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()
})
})

0 comments on commit cc96780

Please sign in to comment.