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

Normalize trailing slash #6421

Closed
wants to merge 23 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
23 commits
Select commit Hold shift + click to select a range
126d3aa
Normalize trailing slash
Janpot Feb 23, 2019
6300eda
didn't mean to commit
Janpot Feb 23, 2019
a5348cb
Merge branch 'canary' into normalize-trailing-slash
Janpot Feb 23, 2019
ea1f230
replace client navigation test
Janpot Feb 23, 2019
b35e510
Merge branch 'normalize-trailing-slash' of https://github.com/Janpot/…
Janpot Feb 23, 2019
b1306f8
actually we shuld keep this test, but in other form
Janpot Feb 23, 2019
dc46920
make test for 404
Janpot Feb 23, 2019
7b92a1c
smoke check
Janpot Feb 23, 2019
d1aee79
Merge remote-tracking branch 'upstream/canary' into normalize-trailin…
Janpot Feb 25, 2019
3168e4c
Merge branch 'canary' into normalize-trailing-slash
Janpot Feb 25, 2019
d8ed050
Merge branch 'canary' into normalize-trailing-slash
Janpot Feb 26, 2019
b7ceb71
Merge branch 'canary' into normalize-trailing-slash
Janpot Feb 27, 2019
cf1c2ad
Merge branch 'canary' into normalize-trailing-slash
Janpot Mar 1, 2019
339bfd6
Merge branch 'canary' into normalize-trailing-slash
Janpot Mar 2, 2019
30deee4
Merge branch 'canary' into normalize-trailing-slash
timneutkens Mar 5, 2019
56ac9ff
Merge branch 'canary' into normalize-trailing-slash
Janpot Mar 9, 2019
1803120
Add production client-side test
Janpot Mar 9, 2019
3b11937
production test
Janpot Mar 9, 2019
500d4e4
Put require folder resolution behavior in tests
Janpot Mar 10, 2019
3ec42c2
Fix serverside failing page resolution
Janpot Mar 10, 2019
756dbf8
Add same production tests
Janpot Mar 10, 2019
6db9061
Merge branch 'canary' into normalize-trailing-slash
Janpot Mar 10, 2019
23eeefb
Merge branch 'canary' into normalize-trailing-slash
dav-is Mar 13, 2019
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: 2 additions & 0 deletions packages/next-server/server/normalize-page-path.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@ export function normalizePagePath(page: string): string {
if (page[0] !== '/') {
page = `/${page}`
}
// Strip trailing slash
page = page.replace(/\/$/, '')
// Throw when using ../ etc in the pathname
const resolvedPage = posix.normalize(page)
if (page !== resolvedPage) {
Expand Down
6 changes: 4 additions & 2 deletions packages/next-server/server/require.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,11 +20,13 @@ export function getPagePath(page: string, distDir: string): string {
throw pageNotFoundError(page)
}

if (!pagesManifest[page]) {
const buildPath = pagesManifest[page] || pagesManifest[page.replace(/\/index$/, '')]

if (!buildPath) {
throw pageNotFoundError(page)
}

return join(serverBuildPath, pagesManifest[page])
return join(serverBuildPath, buildPath)
}

export function requirePage(page: string, distDir: string): any {
Expand Down
2 changes: 1 addition & 1 deletion packages/next/pages/_document.js
Original file line number Diff line number Diff line change
Expand Up @@ -447,5 +447,5 @@ function getPagePathname(page) {
return '/index.js'
}

return `${page}.js`
return `${page.replace(/\/$/, '')}.js`
}
2 changes: 1 addition & 1 deletion packages/next/server/on-demand-entry-handler.js
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ export function normalizePage (page) {
if (unixPagePath === '/index' || unixPagePath === '/') {
return '/'
}
return unixPagePath.replace(/\/index$/, '')
return unixPagePath.replace(/\/(?:index)?$/, '')
}

// Make sure only one invalidation happens at a time
Expand Down
11 changes: 11 additions & 0 deletions test/integration/basic/pages/nav/trailing-slash-link.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import Link from 'next/link'

export default () => (
<div className='trailing-slash-link'>
<Link href='/nav/'>
<a id='home-link'>Go Back</a>
</Link>

<p>This page has a link to an index.js page, called with a trailing /.</p>
</div>
)
24 changes: 24 additions & 0 deletions test/integration/basic/pages/resolution/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import Link from 'next/link'

export default () => (
<div className='trailing-slash-link'>
<Link href='/resolution/subfolder1'>
<a id='subfolder1'>subfolder 1</a>
</Link>
<Link href='/resolution/subfolder1/'>
<a id='subfolder1-slash'>subfolder 1 with slash</a>
</Link>
<Link href='/resolution/subfolder1/index'>
<a id='subfolder1-index'>subfolder 1 with index</a>
</Link>
<Link href='/resolution/subfolder2'>
<a id='subfolder2'>subfolder 2</a>
</Link>
<Link href='/resolution/subfolder2/'>
<a id='subfolder2-slash'>subfolder 2 with slash</a>
</Link>
<Link href='/resolution/subfolder2/index'>
<a id='subfolder2-index'>subfolder 2 with index</a>
</Link>
</div>
)
5 changes: 5 additions & 0 deletions test/integration/basic/pages/resolution/subfolder1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default () => (
<div>
<p className='marker'>This is subfolder1.js</p>
</div>
)
5 changes: 5 additions & 0 deletions test/integration/basic/pages/resolution/subfolder1/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default () => (
<div>
<p className='marker'>This is subfolder1/index.js</p>
</div>
)
5 changes: 5 additions & 0 deletions test/integration/basic/pages/resolution/subfolder2/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default () => (
<div>
<p className='marker'>This is subfolder2/index.js</p>
</div>
)
22 changes: 18 additions & 4 deletions test/integration/basic/test/client-navigation.js
Original file line number Diff line number Diff line change
Expand Up @@ -70,6 +70,17 @@ export default (context) => {
expect(counterText).toBe('Counter: 1')
browser.close()
})

it('should navigate the page when href has trailing slash', async () => {
const browser = await webdriver(context.appPort, '/nav/trailing-slash-link')
const text = await browser
.elementByCss('#home-link').click()
.waitForElementByCss('.nav-home')
.elementByCss('p').text()

expect(text).toBe('This is the home.')
browser.quit()
})
})

describe('with unexpected <a/> nested tag', () => {
Expand Down Expand Up @@ -719,10 +730,13 @@ export default (context) => {
browser.close()
})

it('should 404 for <page>/', async () => {
const browser = await webdriver(context.appPort, '/nav/about/')
expect(await browser.elementByCss('h1').text()).toBe('404')
expect(await browser.elementByCss('h2').text()).toBe('This page could not be found.')
it('should not 404 for <page>/', async () => {
const browser = await webdriver(context.appPort, '/nav/')
const text = await browser
.waitForElementByCss('.nav-home')
.elementByCss('p').text()

expect(text).toBe('This is the home.')
browser.close()
})

Expand Down
2 changes: 2 additions & 0 deletions test/integration/basic/test/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import hmr from './hmr'
import errorRecovery from './error-recovery'
import dynamic from './dynamic'
import processEnv from './process-env'
import resolution from './resolution'

const context = {}
jasmine.DEFAULT_TIMEOUT_INTERVAL = 1000 * 60 * 5
Expand Down Expand Up @@ -72,4 +73,5 @@ describe('Basic Features', () => {
hmr(context, (p, q) => renderViaHTTP(context.appPort, p, q))
errorRecovery(context, (p, q) => renderViaHTTP(context.appPort, p, q))
processEnv(context)
resolution(context)
})
9 changes: 5 additions & 4 deletions test/integration/basic/test/rendering.js
Original file line number Diff line number Diff line change
Expand Up @@ -259,13 +259,14 @@ export default function ({ app }, suiteName, render, fetch) {
expect($('h2').text()).toBe('This page could not be found.')
})

it('should 404 for <page>/', async () => {
it('should not 404 for <page>/', async () => {
const $ = await get$('/nav/about/')
expect($('h1').text()).toBe('404')
expect($('h2').text()).toBe('This page could not be found.')
expect($('p').text()).toBe('This is the about page.')
// Make sure "about.js" got included and not "about/.js"
expect($('script[src*="/nav/about.js"]').length).toBe(1)
})

it('should should not contain a page script in a 404 page', async () => {
it('should not contain a page script in a 404 page', async () => {
const $ = await get$('/non-existent')
$('script[src]').each((index, element) => {
const src = $(element).attr('src')
Expand Down
137 changes: 137 additions & 0 deletions test/integration/basic/test/resolution.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,137 @@
/* eslint-env jest */

import webdriver from 'next-webdriver'

export default (context) => {
describe('page resolution', () => {
describe('client side', () => {
it('should resolve to js file when no slash', async () => {
const browser = await webdriver(context.appPort, '/resolution')
const text = await browser
.elementByCss('#subfolder1').click()
.waitForElementByCss('.marker')
.elementByCss('.marker').text()

expect(text).toBe('This is subfolder1.js')
browser.quit()
})

it('should resolve to folder index when slash', async () => {
const browser = await webdriver(context.appPort, '/resolution')
const text = await browser
.elementByCss('#subfolder1-slash').click()
.waitForElementByCss('.marker')
.elementByCss('.marker').text()

expect(text).toBe('This is subfolder1/index.js')
browser.quit()
})

it('should resolve to folder index when index', async () => {
const browser = await webdriver(context.appPort, '/resolution')
const text = await browser
.elementByCss('#subfolder1-index').click()
.waitForElementByCss('.marker')
.elementByCss('.marker').text()

expect(text).toBe('This is subfolder1/index.js')
browser.quit()
})

it('should resolve to folder when no slash and no js file', async () => {
const browser = await webdriver(context.appPort, '/resolution')
const text = await browser
.elementByCss('#subfolder2').click()
.waitForElementByCss('.marker')
.elementByCss('.marker').text()

expect(text).toBe('This is subfolder2/index.js')
browser.quit()
})

it('should resolve to folder index when slash and no js file', async () => {
const browser = await webdriver(context.appPort, '/resolution')
const text = await browser
.elementByCss('#subfolder2-slash').click()
.waitForElementByCss('.marker')
.elementByCss('.marker').text()

expect(text).toBe('This is subfolder2/index.js')
browser.quit()
})

it('should resolve to folder index when index and no js file', async () => {
const browser = await webdriver(context.appPort, '/resolution')
const text = await browser
.elementByCss('#subfolder2-index').click()
.waitForElementByCss('.marker')
.elementByCss('.marker').text()

expect(text).toBe('This is subfolder2/index.js')
browser.quit()
})
})

describe('server side ', () => {
it('should resolve to js file when no slash', async () => {
const browser = await webdriver(context.appPort, '/resolution/subfolder1')
const text = await browser
.waitForElementByCss('.marker')
.elementByCss('.marker').text()

expect(text).toBe('This is subfolder1.js')
browser.close()
})

it('should resolve folder index when slash', async () => {
const browser = await webdriver(context.appPort, '/resolution/subfolder1/')
const text = await browser
.waitForElementByCss('.marker')
.elementByCss('.marker').text()

expect(text).toBe('This is subfolder1/index.js')
browser.close()
})

it('should resolve folder index when index', async () => {
const browser = await webdriver(context.appPort, '/resolution/subfolder1/index')
const text = await browser
.waitForElementByCss('.marker')
.elementByCss('.marker').text()

expect(text).toBe('This is subfolder1/index.js')
browser.close()
})

it('should resolve to js file when no slash and no js file', async () => {
const browser = await webdriver(context.appPort, '/resolution/subfolder2')
const text = await browser
.waitForElementByCss('.marker')
.elementByCss('.marker').text()

expect(text).toBe('This is subfolder2/index.js')
browser.close()
})

it('should resolve to js file when slash and no js file', async () => {
const browser = await webdriver(context.appPort, '/resolution/subfolder2/')
const text = await browser
.waitForElementByCss('.marker')
.elementByCss('.marker').text()

expect(text).toBe('This is subfolder2/index.js')
browser.close()
})

it('should resolve folder index when index and no js file', async () => {
const browser = await webdriver(context.appPort, '/resolution/subfolder2/index')
const text = await browser
.waitForElementByCss('.marker')
.elementByCss('.marker').text()

expect(text).toBe('This is subfolder2/index.js')
browser.close()
})
})
})
}
24 changes: 24 additions & 0 deletions test/integration/production/pages/resolution/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import Link from 'next/link'

export default () => (
<div className='trailing-slash-link'>
<Link href='/resolution/subfolder1'>
<a id='subfolder1'>subfolder 1</a>
</Link>
<Link href='/resolution/subfolder1/'>
<a id='subfolder1-slash'>subfolder 1 with slash</a>
</Link>
<Link href='/resolution/subfolder1/index'>
<a id='subfolder1-index'>subfolder 1 with index</a>
</Link>
<Link href='/resolution/subfolder2'>
<a id='subfolder2'>subfolder 2</a>
</Link>
<Link href='/resolution/subfolder2/'>
<a id='subfolder2-slash'>subfolder 2 with slash</a>
</Link>
<Link href='/resolution/subfolder2/index'>
<a id='subfolder2-index'>subfolder 2 with index</a>
</Link>
</div>
)
5 changes: 5 additions & 0 deletions test/integration/production/pages/resolution/subfolder1.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default () => (
<div>
<p className='marker'>This is subfolder1.js</p>
</div>
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default () => (
<div>
<p className='marker'>This is subfolder1/index.js</p>
</div>
)
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default () => (
<div>
<p className='marker'>This is subfolder2/index.js</p>
</div>
)
5 changes: 5 additions & 0 deletions test/integration/production/pages/subfolder/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
export default () => (
<div>
<p className='subfolder-index-page'>Hello World</p>
</div>
)
11 changes: 11 additions & 0 deletions test/integration/production/pages/trailing-slash-link.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,11 @@
import Link from 'next/link'

export default () => (
<div className='trailing-slash-link'>
<Link href='/subfolder/'>
<a id='home-link'>Go Back</a>
</Link>

<p>This page has a link to an index.js page, called with a trailing /.</p>
</div>
)
Loading