Skip to content

Commit

Permalink
fix(31013): add base path to preflight request url (#31101)
Browse files Browse the repository at this point in the history
Fixes #31013

My understanding is that there are currently two functions `Router.change` and `Router.prefetch` leading to `Router._preflightRequest` and they pass `options.as` URL differently regarding base path.
In this PR, such difference will be handled in `Router._preflightRequest` to add base path before actually fetching.
Thanks for the review!

P.S.
Since middleware feature is a relatively new, official maintainers might not want external contributions around this area at this stage.
I totally understand such situation, so please let me know if that's the case. I can look for other issues to investigate instead.


## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have helpful link attached, see `contributing.md`

## Feature

- [ ] Implements an existing feature request or RFC. Make sure the feature request has been accepted for implementation before opening a PR.
- [ ] Related issues linked using `fixes #number`
- [ ] Integration tests added
- [ ] Documentation added
- [ ] Telemetry added. In case of a feature if it's used or not.
- [ ] Errors have helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `yarn lint`
  • Loading branch information
hi-ogawa committed Nov 9, 2021
1 parent 3e8b2dc commit 764e29c
Show file tree
Hide file tree
Showing 4 changed files with 60 additions and 7 deletions.
2 changes: 1 addition & 1 deletion packages/next/shared/lib/router/router.ts
Expand Up @@ -1697,7 +1697,7 @@ export default class Router implements BaseRouter {
}

const effects = await this._preflightRequest({
as: asPath,
as: addBasePath(asPath),
cache: true,
pages,
pathname,
Expand Down
10 changes: 10 additions & 0 deletions test/integration/middleware/with-base-path/pages/_middleware.js
Expand Up @@ -2,6 +2,16 @@ import { NextResponse } from 'next/server'

export async function middleware(request) {
const url = request.nextUrl

if (
request.method === 'HEAD' &&
url.basePath === '/root' &&
url.pathname === '/redirect-me-to-about'
) {
url.pathname = '/about'
return NextResponse.redirect(url)
}

if (url.pathname === '/redirect-with-basepath' && !url.basePath) {
url.basePath = '/root'
return NextResponse.redirect(url)
Expand Down
5 changes: 5 additions & 0 deletions test/integration/middleware/with-base-path/pages/index.js
Expand Up @@ -22,6 +22,11 @@ export default function Main({ message }) {
<a>Rewrite me to Vercel</a>
</Link>
</li>
<li>
<Link href="/redirect-me-to-about">
<a>redirect me to about</a>
</Link>
</li>
</ul>
</div>
)
Expand Down
50 changes: 44 additions & 6 deletions test/integration/middleware/with-base-path/test/index.test.js
Expand Up @@ -2,7 +2,14 @@

jest.setTimeout(1000 * 60 * 2)

import { fetchViaHTTP, findPort, killApp, launchApp } from 'next-test-utils'
import {
fetchViaHTTP,
findPort,
killApp,
launchApp,
nextBuild,
nextStart,
} from 'next-test-utils'
import { join } from 'path'
import cheerio from 'cheerio'
import webdriver from 'next-webdriver'
Expand All @@ -11,12 +18,25 @@ const context = {}
context.appDir = join(__dirname, '../')

describe('Middleware base tests', () => {
beforeAll(async () => {
context.appPort = await findPort()
context.app = await launchApp(context.appDir, context.appPort)
describe('dev mode', () => {
beforeAll(async () => {
context.appPort = await findPort()
context.app = await launchApp(context.appDir, context.appPort)
})
afterAll(() => killApp(context.app))
runTests()
})

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

function runTests() {
Expand All @@ -39,3 +59,21 @@ function runTests() {
expect($('.title').text()).toBe('About Page')
})
}

function runPreflightTests() {
it('should redirect via preflight middleware request', async () => {
const browser = await webdriver(context.appPort, '/root')

try {
await browser.waitForCondition(
'next.router && Object.keys(next.router.sde).length == 4'
)
const redirect = await browser.eval(
`next.router.sde["http://localhost:${context.appPort}/root/redirect-me-to-about"].redirect`
)
expect(redirect).toBe('/root/about')
} finally {
await browser.close()
}
})
}

0 comments on commit 764e29c

Please sign in to comment.