fix: skipTrailingSlashRedirect
being ignored in pages
(#55067)
This moves `resolve-href` into `next/src/client` to make sure that when it calls `normalizeTrailingSlash`, that function has access to `process.env.__NEXT_MANUAL_TRAILING_SLASH` (inlined via `DefinePlugin`). Closes NEXT-1599 Fixes #54984
This commit is contained in:
parent
15980a66d4
commit
1f797faaa6
8 changed files with 192 additions and 95 deletions
|
@ -7,7 +7,7 @@ import type {
|
|||
|
||||
import React from 'react'
|
||||
import { UrlObject } from 'url'
|
||||
import { resolveHref } from '../shared/lib/router/utils/resolve-href'
|
||||
import { resolveHref } from './resolve-href'
|
||||
import { isLocalURL } from '../shared/lib/router/utils/is-local-url'
|
||||
import { formatUrl } from '../shared/lib/router/utils/format-url'
|
||||
import { isAbsoluteUrl } from '../shared/lib/utils'
|
||||
|
|
|
@ -1,13 +1,13 @@
|
|||
import type { NextRouter, Url } from '../router'
|
||||
import type { NextRouter, Url } from '../shared/lib/router/router'
|
||||
|
||||
import { searchParamsToUrlQuery } from './querystring'
|
||||
import { formatWithValidation } from './format-url'
|
||||
import { omit } from './omit'
|
||||
import { normalizeRepeatedSlashes } from '../../utils'
|
||||
import { normalizePathTrailingSlash } from '../../../../client/normalize-trailing-slash'
|
||||
import { isLocalURL } from './is-local-url'
|
||||
import { isDynamicRoute } from './is-dynamic'
|
||||
import { interpolateAs } from './interpolate-as'
|
||||
import { searchParamsToUrlQuery } from '../shared/lib/router/utils/querystring'
|
||||
import { formatWithValidation } from '../shared/lib/router/utils/format-url'
|
||||
import { omit } from '../shared/lib/router/utils/omit'
|
||||
import { normalizeRepeatedSlashes } from '../shared/lib/utils'
|
||||
import { normalizePathTrailingSlash } from './normalize-trailing-slash'
|
||||
import { isLocalURL } from '../shared/lib/router/utils/is-local-url'
|
||||
import { isDynamicRoute } from '../shared/lib/router/utils'
|
||||
import { interpolateAs } from '../shared/lib/router/utils/interpolate-as'
|
||||
|
||||
/**
|
||||
* Resolves a given hyperlink with a certain router state (basePath not included).
|
|
@ -40,6 +40,7 @@ import { removeLocale } from '../../../client/remove-locale'
|
|||
import { removeBasePath } from '../../../client/remove-base-path'
|
||||
import { addBasePath } from '../../../client/add-base-path'
|
||||
import { hasBasePath } from '../../../client/has-base-path'
|
||||
import { resolveHref } from '../../../client/resolve-href'
|
||||
import { isAPIRoute } from '../../../lib/is-api-route'
|
||||
import { getNextPathnameInfo } from './utils/get-next-pathname-info'
|
||||
import { formatNextPathnameInfo } from './utils/format-next-pathname-info'
|
||||
|
@ -47,7 +48,6 @@ import { compareRouterStates } from './utils/compare-states'
|
|||
import { isLocalURL } from './utils/is-local-url'
|
||||
import { isBot } from './utils/is-bot'
|
||||
import { omit } from './utils/omit'
|
||||
import { resolveHref } from './utils/resolve-href'
|
||||
import { interpolateAs } from './utils/interpolate-as'
|
||||
import { handleSmoothScroll } from './utils/handle-smooth-scroll'
|
||||
|
||||
|
|
7
test/e2e/skip-trailing-slash-redirect/app/app/layout.js
Normal file
7
test/e2e/skip-trailing-slash-redirect/app/app/layout.js
Normal file
|
@ -0,0 +1,7 @@
|
|||
export default function RootLayout({ children }) {
|
||||
return (
|
||||
<html>
|
||||
<body>{children}</body>
|
||||
</html>
|
||||
)
|
||||
}
|
|
@ -0,0 +1,13 @@
|
|||
import Link from 'next/link'
|
||||
|
||||
export default function Page(props) {
|
||||
return (
|
||||
<>
|
||||
<p id="another">another page</p>
|
||||
<Link href="/with-app-dir" id="to-index">
|
||||
to index
|
||||
</Link>
|
||||
<br />
|
||||
</>
|
||||
)
|
||||
}
|
|
@ -0,0 +1,13 @@
|
|||
import Link from 'next/link'
|
||||
|
||||
export default function Page(props) {
|
||||
return (
|
||||
<>
|
||||
<p id="blog">blog page</p>
|
||||
<Link href="/with-app-dir" id="to-index">
|
||||
to index
|
||||
</Link>
|
||||
<br />
|
||||
</>
|
||||
)
|
||||
}
|
|
@ -0,0 +1,21 @@
|
|||
import Link from 'next/link'
|
||||
|
||||
export default function Page(props) {
|
||||
return (
|
||||
<>
|
||||
<p id="index">index page</p>
|
||||
<Link href="/with-app-dir/another" id="to-another">
|
||||
to another
|
||||
</Link>
|
||||
<br />
|
||||
<Link href="/with-app-dir/another/" id="to-another-with-slash">
|
||||
to another
|
||||
</Link>
|
||||
<br />
|
||||
<Link href="/with-app-dir/blog/first" id="to-blog-first">
|
||||
to /blog/first
|
||||
</Link>
|
||||
<br />
|
||||
</>
|
||||
)
|
||||
}
|
|
@ -16,6 +16,129 @@ describe('skip-trailing-slash-redirect', () => {
|
|||
})
|
||||
afterAll(() => next.destroy())
|
||||
|
||||
// the tests below are run in both pages and app dir to ensure the behavior is the same
|
||||
// the other cases aren't added to this block since they are either testing pages-specific behavior
|
||||
// or aren't specific to either router implementation
|
||||
async function runSharedTests(basePath: string) {
|
||||
it('should not apply trailing slash redirect (with slash)', async () => {
|
||||
const res = await fetchViaHTTP(
|
||||
next.url,
|
||||
`${basePath}another/`,
|
||||
undefined,
|
||||
{
|
||||
redirect: 'manual',
|
||||
}
|
||||
)
|
||||
expect(res.status).toBe(200)
|
||||
expect(await res.text()).toContain('another page')
|
||||
})
|
||||
|
||||
it('should not apply trailing slash redirect (without slash)', async () => {
|
||||
const res = await fetchViaHTTP(
|
||||
next.url,
|
||||
`${basePath}another`,
|
||||
undefined,
|
||||
{
|
||||
redirect: 'manual',
|
||||
}
|
||||
)
|
||||
expect(res.status).toBe(200)
|
||||
expect(await res.text()).toContain('another page')
|
||||
})
|
||||
|
||||
it('should preserve original trailing slashes to links on client', async () => {
|
||||
const browser = await webdriver(next.url, basePath)
|
||||
await browser.eval('window.beforeNav = 1')
|
||||
|
||||
expect(
|
||||
new URL(
|
||||
await browser.elementByCss('#to-another').getAttribute('href'),
|
||||
'http://n'
|
||||
).pathname
|
||||
).toBe(`${basePath}another`)
|
||||
|
||||
expect(
|
||||
new URL(
|
||||
await browser
|
||||
.elementByCss('#to-another-with-slash')
|
||||
.getAttribute('href'),
|
||||
'http://n'
|
||||
).pathname
|
||||
).toBe(`${basePath}another/`)
|
||||
|
||||
await browser.elementByCss('#to-another').click()
|
||||
await browser.waitForElementByCss('#another')
|
||||
|
||||
expect(await browser.eval('window.location.pathname')).toBe(
|
||||
`${basePath}another`
|
||||
)
|
||||
|
||||
await browser.back().waitForElementByCss('#to-another')
|
||||
|
||||
expect(
|
||||
new URL(
|
||||
await browser
|
||||
.elementByCss('#to-another-with-slash')
|
||||
.getAttribute('href'),
|
||||
'http://n'
|
||||
).pathname
|
||||
).toBe(`${basePath}another/`)
|
||||
|
||||
await browser.elementByCss('#to-another-with-slash').click()
|
||||
await browser.waitForElementByCss('#another')
|
||||
|
||||
expect(await browser.eval('window.location.pathname')).toBe(
|
||||
`${basePath}another/`
|
||||
)
|
||||
|
||||
await browser.back().waitForElementByCss('#to-another')
|
||||
expect(await browser.eval('window.beforeNav')).toBe(1)
|
||||
})
|
||||
|
||||
it('should respond to index correctly', async () => {
|
||||
const res = await fetchViaHTTP(next.url, basePath, undefined, {
|
||||
redirect: 'manual',
|
||||
})
|
||||
expect(res.status).toBe(200)
|
||||
expect(await res.text()).toContain('index page')
|
||||
})
|
||||
|
||||
it('should respond to dynamic route correctly', async () => {
|
||||
const res = await fetchViaHTTP(
|
||||
next.url,
|
||||
`${basePath}blog/first`,
|
||||
undefined,
|
||||
{
|
||||
redirect: 'manual',
|
||||
}
|
||||
)
|
||||
expect(res.status).toBe(200)
|
||||
expect(await res.text()).toContain('blog page')
|
||||
})
|
||||
|
||||
it('should navigate client side correctly', async () => {
|
||||
const browser = await webdriver(next.url, basePath)
|
||||
|
||||
expect(await browser.eval('location.pathname')).toBe(basePath)
|
||||
|
||||
await browser.elementByCss('#to-another').click()
|
||||
await browser.waitForElementByCss('#another')
|
||||
|
||||
expect(await browser.eval('location.pathname')).toBe(`${basePath}another`)
|
||||
await browser.back()
|
||||
await browser.waitForElementByCss('#index')
|
||||
|
||||
expect(await browser.eval('location.pathname')).toBe(basePath)
|
||||
|
||||
await browser.elementByCss('#to-blog-first').click()
|
||||
await browser.waitForElementByCss('#blog')
|
||||
|
||||
expect(await browser.eval('location.pathname')).toBe(
|
||||
`${basePath}blog/first`
|
||||
)
|
||||
})
|
||||
}
|
||||
|
||||
it('should parse locale info for data request correctly', async () => {
|
||||
const pathname = `/_next/data/${next.buildId}/ja-jp/locale-test.json`
|
||||
const res = await next.fetch(pathname)
|
||||
|
@ -228,58 +351,6 @@ describe('skip-trailing-slash-redirect', () => {
|
|||
expect(await res.text()).toContain('another page')
|
||||
})
|
||||
|
||||
it('should not apply trailing slash redirect (with slash)', async () => {
|
||||
const res = await fetchViaHTTP(next.url, '/another/', undefined, {
|
||||
redirect: 'manual',
|
||||
})
|
||||
expect(res.status).toBe(200)
|
||||
expect(await res.text()).toContain('another page')
|
||||
})
|
||||
|
||||
it('should not apply trailing slash redirect (without slash)', async () => {
|
||||
const res = await fetchViaHTTP(next.url, '/another', undefined, {
|
||||
redirect: 'manual',
|
||||
})
|
||||
expect(res.status).toBe(200)
|
||||
expect(await res.text()).toContain('another page')
|
||||
})
|
||||
|
||||
it('should not apply trailing slash to links on client', async () => {
|
||||
const browser = await webdriver(next.url, '/')
|
||||
await browser.eval('window.beforeNav = 1')
|
||||
|
||||
expect(
|
||||
new URL(
|
||||
await browser.elementByCss('#to-another').getAttribute('href'),
|
||||
'http://n'
|
||||
).pathname
|
||||
).toBe('/another')
|
||||
|
||||
await browser.elementByCss('#to-another').click()
|
||||
await browser.waitForElementByCss('#another')
|
||||
|
||||
expect(await browser.eval('window.location.pathname')).toBe('/another')
|
||||
|
||||
await browser.back().waitForElementByCss('#to-another')
|
||||
|
||||
expect(
|
||||
new URL(
|
||||
await browser
|
||||
.elementByCss('#to-another-with-slash')
|
||||
.getAttribute('href'),
|
||||
'http://n'
|
||||
).pathname
|
||||
).toBe('/another/')
|
||||
|
||||
await browser.elementByCss('#to-another-with-slash').click()
|
||||
await browser.waitForElementByCss('#another')
|
||||
|
||||
expect(await browser.eval('window.location.pathname')).toBe('/another/')
|
||||
|
||||
await browser.back().waitForElementByCss('#to-another')
|
||||
expect(await browser.eval('window.beforeNav')).toBe(1)
|
||||
})
|
||||
|
||||
it('should not apply trailing slash on load on client', async () => {
|
||||
let browser = await webdriver(next.url, '/another')
|
||||
await check(() => browser.eval('next.router.isReady ? "yes": "no"'), 'yes')
|
||||
|
@ -292,39 +363,11 @@ describe('skip-trailing-slash-redirect', () => {
|
|||
expect(await browser.eval('location.pathname')).toBe('/another/')
|
||||
})
|
||||
|
||||
it('should respond to index correctly', async () => {
|
||||
const res = await fetchViaHTTP(next.url, '/', undefined, {
|
||||
redirect: 'manual',
|
||||
})
|
||||
expect(res.status).toBe(200)
|
||||
expect(await res.text()).toContain('index page')
|
||||
describe('pages dir', () => {
|
||||
runSharedTests('/')
|
||||
})
|
||||
|
||||
it('should respond to dynamic route correctly', async () => {
|
||||
const res = await fetchViaHTTP(next.url, '/blog/first', undefined, {
|
||||
redirect: 'manual',
|
||||
})
|
||||
expect(res.status).toBe(200)
|
||||
expect(await res.text()).toContain('blog page')
|
||||
})
|
||||
|
||||
it('should navigate client side correctly', async () => {
|
||||
const browser = await webdriver(next.url, '/')
|
||||
|
||||
expect(await browser.eval('location.pathname')).toBe('/')
|
||||
|
||||
await browser.elementByCss('#to-another').click()
|
||||
await browser.waitForElementByCss('#another')
|
||||
|
||||
expect(await browser.eval('location.pathname')).toBe('/another')
|
||||
await browser.back()
|
||||
await browser.waitForElementByCss('#index')
|
||||
|
||||
expect(await browser.eval('location.pathname')).toBe('/')
|
||||
|
||||
await browser.elementByCss('#to-blog-first').click()
|
||||
await browser.waitForElementByCss('#blog')
|
||||
|
||||
expect(await browser.eval('location.pathname')).toBe('/blog/first')
|
||||
describe('app dir', () => {
|
||||
runSharedTests('/with-app-dir/')
|
||||
})
|
||||
})
|
||||
|
|
Loading…
Reference in a new issue