Skip to content

Commit

Permalink
Fix trailing slash redirect applying for data request (#45417)
Browse files Browse the repository at this point in the history
This ensures we don't apply the trailing slash redirect for `_next/data`
requests as it can cause props to fail to resolve on client transition.
This also fixes `missing` fields not being applied correctly for
`headers` and `redirects` as the field wasn't being passed through.

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a helpful link attached, see
[`contributing.md`](https://github.com/vercel/next.js/blob/canary/contributing.md)

Closes: #45398
Fixes: #45393
x-ref: #45340
  • Loading branch information
ijjk authored Jan 30, 2023
1 parent 9e290f9 commit d3a9f5a
Show file tree
Hide file tree
Showing 8 changed files with 398 additions and 9 deletions.
7 changes: 7 additions & 0 deletions packages/next/src/lib/load-custom-routes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -668,6 +668,13 @@ export default async function loadCustomRoutes(
permanent: true,
locale: config.i18n ? false : undefined,
internal: true,
// don't run this redirect for _next/data requests
missing: [
{
type: 'header',
key: 'x-nextjs-data',
},
],
} as Redirect,
{
source: '/:notfile((?!\\.well-known(?:/.*)?)(?:[^/]+/)*[^/\\.]+)',
Expand Down
2 changes: 2 additions & 0 deletions packages/next/src/server/server-route-utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,7 @@ export const createHeaderRoute = ({
matchesLocaleAPIRoutes: true,
matchesTrailingSlash: true,
has: headerRoute.has,
missing: headerRoute.missing,
type: headerRoute.type,
name: `${headerRoute.type} ${headerRoute.source} header route`,
fn: async (_req, res, params, _parsedUrl) => {
Expand Down Expand Up @@ -146,6 +147,7 @@ export const createRedirectRoute = ({
matchesLocaleAPIRoutes: true,
matchesTrailingSlash: true,
has: redirectRoute.has,
missing: redirectRoute.missing,
statusCode: redirectRoute.statusCode,
name: `Redirect route ${redirectRoute.source}`,
fn: async (req, res, params, parsedUrl) => {
Expand Down
24 changes: 24 additions & 0 deletions test/e2e/middleware-trailing-slash/app/pages/html-links.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
import Link from 'next/link'

export default function Page() {
return (
<ul>
<li>
<Link
id="with-html"
href="/product/shirts_and_tops/mens_ua_playoff_polo_2.0/1327037.html"
>
Does not work
</Link>
</li>
<li>
<Link
id="without-html"
href="/product/shirts_and_tops/mens_ua_playoff_polo_2.0/1327037"
>
Works
</Link>
</li>
</ul>
)
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
import { GetServerSideProps } from 'next'
import React from 'react'

export interface ProductPageProps {
test: string
}

const ProductPage = (params: ProductPageProps) => {
return (
<>
<h1 id="text">Param found: {params.test}</h1>
</>
)
}

export const getServerSideProps: GetServerSideProps = async ({ params }) => {
const joined = Array.isArray(params['product-params'])
? params['product-params'].join(', ')
: params['product-params']
return {
props: {
test: joined ? joined : 'Not Found',
},
}
}

export default ProductPage
56 changes: 56 additions & 0 deletions test/e2e/middleware-trailing-slash/test/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,62 @@ describe('Middleware Runtime trailing slash', () => {
})

function runTests() {
describe('with .html extension', () => {
it('should work when requesting the page directly', async () => {
const $ = await next.render$(
'/product/shirts_and_tops/mens_ua_playoff_polo_2.0/1327037.html'
)
expect($('#text').text()).toBe(
'Param found: shirts_and_tops, mens_ua_playoff_polo_2.0, 1327037.html'
)
})

it('should work using browser', async () => {
const browser = await next.browser(
'/product/shirts_and_tops/mens_ua_playoff_polo_2.0/1327037.html'
)
expect(await browser.elementByCss('#text').text()).toBe(
'Param found: shirts_and_tops, mens_ua_playoff_polo_2.0, 1327037.html'
)
})

it('should work when navigating', async () => {
const browser = await next.browser('/html-links')
await browser.elementByCss('#with-html').click()
expect(await browser.waitForElementByCss('#text').text()).toBe(
'Param found: shirts_and_tops, mens_ua_playoff_polo_2.0, 1327037.html'
)
})
})

describe('without .html extension', () => {
it('should work when requesting the page directly', async () => {
const $ = await next.render$(
'/product/shirts_and_tops/mens_ua_playoff_polo_2.0/1327037'
)
expect($('#text').text()).toBe(
'Param found: shirts_and_tops, mens_ua_playoff_polo_2.0, 1327037'
)
})

it('should work using browser', async () => {
const browser = await next.browser(
'/product/shirts_and_tops/mens_ua_playoff_polo_2.0/1327037'
)
expect(await browser.elementByCss('#text').text()).toBe(
'Param found: shirts_and_tops, mens_ua_playoff_polo_2.0, 1327037'
)
})

it('should work when navigating', async () => {
const browser = await next.browser('/html-links')
await browser.elementByCss('#without-html').click()
expect(await browser.waitForElementByCss('#text').text()).toBe(
'Param found: shirts_and_tops, mens_ua_playoff_polo_2.0, 1327037'
)
})
})

if ((global as any).isNextDev) {
it('refreshes the page when middleware changes ', async () => {
const browser = await webdriver(next.url, `/about/`)
Expand Down
82 changes: 82 additions & 0 deletions test/integration/custom-routes/next.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,41 @@ module.exports = {
},
async redirects() {
return [
{
source: '/missing-redirect-1',
missing: [
{
type: 'header',
key: 'x-my-header',
value: '(?<myHeader>.*)',
},
],
destination: '/with-params',
permanent: false,
},
{
source: '/missing-redirect-2',
missing: [
{
type: 'query',
key: 'my-query',
},
],
destination: '/with-params',
permanent: false,
},
{
source: '/missing-redirect-3',
missing: [
{
type: 'cookie',
key: 'loggedIn',
value: '(?<loggedIn>true)',
},
],
destination: '/with-params?authorized=1',
permanent: false,
},
{
source: '/redirect/me/to-about/:lang',
destination: '/:lang/about',
Expand Down Expand Up @@ -465,6 +500,53 @@ module.exports = {

async headers() {
return [
{
source: '/missing-headers-1',
missing: [
{
type: 'header',
key: 'x-my-header',
value: '(?<myHeader>.*)',
},
],
headers: [
{
key: 'x-new-header',
value: 'new-value',
},
],
},
{
source: '/missing-headers-2',
missing: [
{
type: 'query',
key: 'my-query',
},
],
headers: [
{
key: 'x-new-header',
value: 'new-value',
},
],
},
{
source: '/missing-headers-3',
missing: [
{
type: 'cookie',
key: 'loggedIn',
value: '(?<loggedIn>true)',
},
],
headers: [
{
key: 'x-new-header',
value: 'new-value',
},
],
},
{
source: '/add-header',
headers: [
Expand Down
Loading

0 comments on commit d3a9f5a

Please sign in to comment.