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

[next] Fix to not call middleware with shallow push, fix middleware call wit… #35047

Merged
9 changes: 6 additions & 3 deletions packages/next/shared/lib/router/router.ts
Expand Up @@ -32,6 +32,7 @@ import {
} from '../utils'
import { isDynamicRoute } from './utils/is-dynamic'
import { parseRelativeUrl } from './utils/parse-relative-url'
import { parseUrl } from './utils/parse-url'
import { searchParamsToUrlQuery } from './utils/querystring'
import resolveRewrites from './utils/resolve-rewrites'
import { getRouteMatcher } from './utils/route-matcher'
Expand Down Expand Up @@ -1165,8 +1166,9 @@ export default class Router implements BaseRouter {
* request as it is not necessary.
*/
if (
(options as any)._h !== 1 ||
isDynamicRoute(removePathTrailingSlash(pathname))
(!options.shallow || (options as any)._h === 1) &&
((options as any)._h !== 1 ||
isDynamicRoute(removePathTrailingSlash(pathname)))
) {
const effect = await this._preflightRequest({
as,
Expand Down Expand Up @@ -1864,8 +1866,9 @@ export default class Router implements BaseRouter {
locale: string | undefined
isPreview: boolean
}): Promise<PreflightEffect> {
const asPathname = parseUrl(options.as).pathname
nkzawa marked this conversation as resolved.
Show resolved Hide resolved
const cleanedAs = delLocale(
hasBasePath(options.as) ? delBasePath(options.as) : options.as,
hasBasePath(asPathname) ? delBasePath(asPathname) : asPathname,
Copy link
Contributor Author

@nkzawa nkzawa Mar 4, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the fix for #35006
It's fixed at the same time since it's needed for the added test

options.locale
)

Expand Down
Expand Up @@ -65,7 +65,10 @@ export async function middleware(request) {
return NextResponse.rewrite(url)
}

if (url.pathname === '/rewrites/rewrite-me-without-hard-navigation') {
if (
url.pathname === '/rewrites/rewrite-me-without-hard-navigation' ||
url.searchParams.get('path') === 'rewrite-me-without-hard-navigation'
) {
url.searchParams.set('middleware', 'foo')
url.pathname =
request.cookies['about-bypass'] === '1'
Expand Down
17 changes: 17 additions & 0 deletions test/integration/middleware/core/pages/rewrites/index.js
@@ -1,6 +1,8 @@
import Link from 'next/link'
import { useRouter } from 'next/router'

export default function Home() {
const router = useRouter()
return (
<div>
<p className="title">Home Page</p>
Expand Down Expand Up @@ -32,6 +34,21 @@ export default function Home() {
<Link href="/rewrites/about?override=internal">
<a id="override-with-internal-rewrite">Rewrite me to internal path</a>
</Link>
<div />
<a
href=""
id="link-to-shallow-push"
onClick={(e) => {
e.preventDefault()
router.push(
'/rewrites?path=rewrite-me-without-hard-navigation&message=refreshed',
undefined,
{ shallow: true }
)
}}
>
Do not rewrite me
</a>
</div>
)
}
11 changes: 11 additions & 0 deletions test/integration/middleware/core/test/index.test.js
Expand Up @@ -484,6 +484,17 @@ function rewriteTests(log, locale = '') {
const element = await browser.elementByCss('.title')
expect(await element.text()).toEqual('About Bypassed Page')
})

it(`${locale} should not call middleware with shallow push`, async () => {
const browser = await webdriver(context.appPort, '/rewrites')
await browser.elementByCss('#link-to-shallow-push').click()
await browser.waitForCondition(
'new URL(window.location.href).searchParams.get("path") === "rewrite-me-without-hard-navigation"'
)
await expect(async () => {
await browser.waitForElementByCss('.refreshed', 500)
}).rejects.toThrow()
})
}

function redirectTests(locale = '') {
Expand Down