Ensure query is provided correctly with middleware rewrites (#42818)

x-ref: [slack
thread](https://vercel.slack.com/archives/C035J346QQL/p1668195556550659)
Fixes: #42463

## Bug

- [x] Related issues linked using `fixes #number`
- [x] Integration tests added
- [ ] Errors have a 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 a helpful link attached, see `contributing.md`

## Documentation / Examples

- [ ] Make sure the linting passes by running `pnpm build && pnpm lint`
- [ ] The "examples guidelines" are followed from [our contributing
doc](https://github.com/vercel/next.js/blob/canary/contributing/examples/adding-examples.md)

packages/next/shared/lib/router/router.ts

@@ -1415,6 +1415,16 @@ export default class Router implements BaseRouter {
       ? removeTrailingSlash(removeBasePath(pathname))
       : pathname
 
+    let route = removeTrailingSlash(pathname)
+    const parsedAsPathname = as.startsWith('/') && parseRelativeUrl(as).pathname
+
+    const isMiddlewareRewrite = !!(
+      parsedAsPathname &&
+      route !== parsedAsPathname &&
+      (!isDynamicRoute(route) ||
+        !getRouteMatcher(getRouteRegex(route))(parsedAsPathname))
+    )
+
     // we don't attempt resolve asPath when we need to execute
     // middleware as the resolving will occur server-side
     const isMiddlewareMatch = await matchesMiddleware({
@@ -1489,7 +1499,7 @@ export default class Router implements BaseRouter {
 
     resolvedAs = removeLocale(removeBasePath(resolvedAs), nextState.locale)
 
-    let route = removeTrailingSlash(pathname)
+    route = removeTrailingSlash(pathname)
     let routeMatch: { [paramName: string]: string | string[] } | false = false
 
     if (isDynamicRoute(route)) {
@@ -1565,6 +1575,7 @@ export default class Router implements BaseRouter {
         hasMiddleware: isMiddlewareMatch,
         unstable_skipClientCache: options.unstable_skipClientCache,
         isQueryUpdating: isQueryUpdating && !this.isFallback,
+        isMiddlewareRewrite,
       })
 
       if ('route' in routeInfo && isMiddlewareMatch) {
@@ -1913,6 +1924,7 @@ export default class Router implements BaseRouter {
     isPreview,
     unstable_skipClientCache,
     isQueryUpdating,
+    isMiddlewareRewrite,
   }: {
     route: string
     pathname: string
@@ -1925,6 +1937,7 @@ export default class Router implements BaseRouter {
     isPreview: boolean
     unstable_skipClientCache?: boolean
     isQueryUpdating?: boolean
+    isMiddlewareRewrite?: boolean
   }) {
     /**
      * This `route` binding can change if there's a rewrite
@@ -1970,16 +1983,26 @@ export default class Router implements BaseRouter {
         isBackground: isQueryUpdating,
       }
 
-      const data = isQueryUpdating
-        ? ({} as any)
-        : await withMiddlewareEffects({
-            fetchData: () => fetchNextData(fetchNextDataParams),
-            asPath: resolvedAs,
-            locale: locale,
-            router: this,
-          })
+      const data =
+        isQueryUpdating && !isMiddlewareRewrite
+          ? ({} as any)
+          : await withMiddlewareEffects({
+              fetchData: () => fetchNextData(fetchNextDataParams),
+              asPath: resolvedAs,
+              locale: locale,
+              router: this,
+            }).catch((err) => {
+              // we don't hard error during query updating
+              // as it's un-necessary and doesn't need to be fatal
+              // unless it is a fallback route and the props can't
+              // be loaded
+              if (isQueryUpdating) {
+                return {} as any
+              }
+              throw err
+            })
 
-      if (isQueryUpdating && data) {
+      if (isQueryUpdating) {
         data.json = self.__NEXT_DATA__.props
       }
       handleCancelled()
@@ -1992,26 +2015,35 @@ export default class Router implements BaseRouter {
       }
 
       if (data?.effect?.type === 'rewrite') {
-        route = removeTrailingSlash(data.effect.resolvedHref)
-        pathname = data.effect.resolvedHref
-        query = { ...query, ...data.effect.parsedAs.query }
-        resolvedAs = removeBasePath(
-          normalizeLocalePath(data.effect.parsedAs.pathname, this.locales)
-            .pathname
-        )
+        const resolvedRoute = removeTrailingSlash(data.effect.resolvedHref)
+        const pages = await this.pageLoader.getPageList()
+
+        // during query updating the page must match although during
+        // client-transition a redirect that doesn't match a page
+        // can be returned and this should trigger a hard navigation
+        // which is valid for incremental migration
+        if (!isQueryUpdating || pages.includes(resolvedRoute)) {
+          route = resolvedRoute
+          pathname = data.effect.resolvedHref
+          query = { ...query, ...data.effect.parsedAs.query }
+          resolvedAs = removeBasePath(
+            normalizeLocalePath(data.effect.parsedAs.pathname, this.locales)
+              .pathname
+          )
 
-        // Check again the cache with the new destination.
-        existingInfo = this.components[route]
-        if (
-          routeProps.shallow &&
-          existingInfo &&
-          this.route === route &&
-          !hasMiddleware
-        ) {
-          // If we have a match with the current route due to rewrite,
-          // we can copy the existing information to the rewritten one.
-          // Then, we return the information along with the matched route.
-          return { ...existingInfo, route }
+          // Check again the cache with the new destination.
+          existingInfo = this.components[route]
+          if (
+            routeProps.shallow &&
+            existingInfo &&
+            this.route === route &&
+            !hasMiddleware
+          ) {
+            // If we have a match with the current route due to rewrite,
+            // we can copy the existing information to the rewritten one.
+            // Then, we return the information along with the matched route.
+            return { ...existingInfo, route }
+          }
         }
       }
 

test/e2e/middleware-general/test/index.test.ts

@@ -230,6 +230,7 @@ describe('Middleware Runtime', () => {
       await check(() => browser.elementByCss('body').text(), /\/to-ssg/)
 
       expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({
+        from: 'middleware',
         slug: 'hello',
       })
       expect(
@@ -278,7 +279,11 @@ describe('Middleware Runtime', () => {
     })
 
     it('should have correct dynamic route params for middleware rewrite to dynamic route', async () => {
-      const browser = await webdriver(next.url, '/')
+      const browser = await webdriver(next.url, '/404')
+      await check(
+        () => browser.eval('next.router.isReady ? "yes" : "no"'),
+        'yes'
+      )
       await browser.eval('window.beforeNav = 1')
       await browser.eval('window.next.router.push("/rewrite-to-dynamic")')
       await browser.waitForElementByCss('#blog')
@@ -301,7 +306,11 @@ describe('Middleware Runtime', () => {
     })
 
     it('should have correct route params for chained rewrite from middleware to config rewrite', async () => {
-      const browser = await webdriver(next.url, '/')
+      const browser = await webdriver(next.url, '/404')
+      await check(
+        () => browser.eval('next.router.isReady ? "yes" : "no"'),
+        'yes'
+      )
       await browser.eval('window.beforeNav = 1')
       await browser.eval(
         'window.next.router.push("/rewrite-to-config-rewrite")'
@@ -327,7 +336,7 @@ describe('Middleware Runtime', () => {
     })
 
     it('should have correct route params for rewrite from config dynamic route', async () => {
-      const browser = await webdriver(next.url, '/')
+      const browser = await webdriver(next.url, '/404')
       await browser.eval('window.beforeNav = 1')
       await browser.eval('window.next.router.push("/rewrite-3")')
       await browser.waitForElementByCss('#blog')

test/e2e/middleware-rewrites/app/middleware.js

@@ -19,6 +19,11 @@ export async function middleware(request) {
     })
   }
 
+  if (url.pathname.includes('/rewrite-to-static')) {
+    request.nextUrl.pathname = '/static-ssg/post-1'
+    return NextResponse.rewrite(request.nextUrl)
+  }
+
   if (url.pathname.includes('/fallback-true-blog/rewritten')) {
     request.nextUrl.pathname = '/about'
     return NextResponse.rewrite(request.nextUrl)

test/e2e/middleware-rewrites/app/next.config.js

@@ -20,6 +20,10 @@ module.exports = {
           source: '/afterfiles-rewrite-ssg',
           destination: '/fallback-true-blog/first',
         },
+        {
+          source: '/config-rewrite-to-dynamic-static/:rewriteSlug',
+          destination: '/ssg',
+        },
       ],
       fallback: [],
     }

test/e2e/middleware-rewrites/app/pages/static-ssg/[slug].js

@@ -0,0 +1,14 @@
+import { useRouter } from 'next/router'
+
+export default function Page() {
+  const router = useRouter()
+
+  return (
+    <>
+      <p id="page">/static-ssg/[slug]</p>
+      <p id="query">{JSON.stringify(router.query)}</p>
+      <p id="pathname">{router.pathname}</p>
+      <p id="as-path">{router.asPath}</p>
+    </>
+  )
+}

test/e2e/middleware-rewrites/test/index.test.ts

@@ -23,6 +23,42 @@ describe('Middleware Rewrite', () => {
   })
 
   function tests() {
+    it('should handle static dynamic rewrite from middleware correctly', async () => {
+      const browser = await webdriver(next.url, '/rewrite-to-static')
+
+      await check(() => browser.eval('next.router.query.slug'), 'post-1')
+      expect(await browser.elementByCss('#page').text()).toBe(
+        '/static-ssg/[slug]'
+      )
+      expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({
+        slug: 'post-1',
+      })
+      expect(await browser.elementByCss('#pathname').text()).toBe(
+        '/static-ssg/[slug]'
+      )
+      expect(await browser.elementByCss('#as-path').text()).toBe(
+        '/rewrite-to-static'
+      )
+    })
+
+    it('should handle static rewrite from next.config.js correctly', async () => {
+      const browser = await webdriver(
+        next.url,
+        '/config-rewrite-to-dynamic-static/post-2'
+      )
+
+      await check(() => browser.eval('next.router.query.rewriteSlug'), 'post-2')
+      expect(
+        JSON.parse(await browser.eval('JSON.stringify(next.router.query)'))
+      ).toEqual({
+        rewriteSlug: 'post-2',
+      })
+      expect(await browser.eval('next.router.pathname')).toBe('/ssg')
+      expect(await browser.eval('next.router.asPath')).toBe(
+        '/config-rewrite-to-dynamic-static/post-2'
+      )
+    })
+
     it('should not have un-necessary data request on rewrite', async () => {
       const browser = await webdriver(next.url, '/to-blog/first', {
         waitHydration: false,

test/e2e/middleware-trailing-slash/test/index.test.ts

@@ -120,6 +120,7 @@ describe('Middleware Runtime trailing slash', () => {
       await check(() => browser.elementByCss('body').text(), /\/to-ssg/)
 
       expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({
+        from: 'middleware',
         slug: 'hello',
       })
       expect(
@@ -133,6 +134,10 @@ describe('Middleware Runtime trailing slash', () => {
 
     it('should have correct dynamic route params on client-transition to dynamic route', async () => {
       const browser = await webdriver(next.url, '/')
+      await check(
+        () => browser.eval('next.router.isReady ? "yes" : "no"'),
+        'yes'
+      )
       await browser.eval('window.beforeNav = 1')
       await browser.eval('window.next.router.push("/blog/first")')
       await browser.waitForElementByCss('#blog')
@@ -170,7 +175,11 @@ describe('Middleware Runtime trailing slash', () => {
     })
 
     it('should have correct dynamic route params for middleware rewrite to dynamic route', async () => {
-      const browser = await webdriver(next.url, '/')
+      const browser = await webdriver(next.url, '/404')
+      await check(
+        () => browser.eval('next.router.isReady ? "yes" : "no"'),
+        'yes'
+      )
       await browser.eval('window.beforeNav = 1')
       await browser.eval('window.next.router.push("/rewrite-to-dynamic")')
       await browser.waitForElementByCss('#blog')
@@ -193,7 +202,11 @@ describe('Middleware Runtime trailing slash', () => {
     })
 
     it('should have correct route params for chained rewrite from middleware to config rewrite', async () => {
-      const browser = await webdriver(next.url, '/')
+      const browser = await webdriver(next.url, '/404')
+      await check(
+        () => browser.eval('next.router.isReady ? "yes" : "no"'),
+        'yes'
+      )
       await browser.eval('window.beforeNav = 1')
       await browser.eval(
         'window.next.router.push("/rewrite-to-config-rewrite")'
@@ -219,7 +232,11 @@ describe('Middleware Runtime trailing slash', () => {
     })
 
     it('should have correct route params for rewrite from config dynamic route', async () => {
-      const browser = await webdriver(next.url, '/')
+      const browser = await webdriver(next.url, '/404')
+      await check(
+        () => browser.eval('next.router.isReady ? "yes" : "no"'),
+        'yes'
+      )
       await browser.eval('window.beforeNav = 1')
       await browser.eval('window.next.router.push("/rewrite-3")')
       await browser.waitForElementByCss('#blog')