Preserve asPath while resolving rewrites (#21410)

* Preserve asPath while resolving rewrites

* remove un-needed alias

* Add new alias

* Add return type for resolve rewrites

packages/next/build/webpack-config.ts

@@ -213,7 +213,7 @@ export default async function getBaseWebpackConfig(
   let plugins: PluginMetaData[] = []
   let babelPresetPlugins: { dir: string; config: any }[] = []
 
-  const hasRewrites = rewrites.length > 0 || dev
+  const hasRewrites = rewrites.length > 0
 
   if (config.experimental.plugins) {
     plugins = await collectPlugins(dir, config.env, config.plugins)
@@ -344,6 +344,9 @@ export default async function getBaseWebpackConfig(
   const clientResolveRewrites = require.resolve(
     'next/dist/next-server/lib/router/utils/resolve-rewrites'
   )
+  const clientResolveRewritesNoop = require.resolve(
+    'next/dist/next-server/lib/router/utils/resolve-rewrites-noop'
+  )
 
   const resolveConfig = {
     // Disable .mjs for node_modules bundling
@@ -385,7 +388,7 @@ export default async function getBaseWebpackConfig(
       ...getReactProfilingInProduction(),
       [clientResolveRewrites]: hasRewrites
         ? clientResolveRewrites
-        : require.resolve('next/dist/client/dev/noop.js'),
+        : clientResolveRewritesNoop,
     },
     mainFields: isServer ? ['main', 'module'] : ['browser', 'module', 'main'],
     plugins: isWebpack5

packages/next/next-server/lib/router/router.ts

@@ -932,39 +932,23 @@ export default class Router implements BaseRouter {
     let resolvedAs = as
 
     if (process.env.__NEXT_HAS_REWRITES && as.startsWith('/')) {
-      resolvedAs = resolveRewrites(
-        addBasePath(
-          addLocale(delBasePath(parseRelativeUrl(as).pathname), this.locale)
-        ),
+      const rewritesResult = resolveRewrites(
+        addBasePath(addLocale(delBasePath(as), this.locale)),
         pages,
         rewrites,
         query,
         (p: string) => this._resolveHref({ pathname: p }, pages).pathname!,
         this.locales
       )
+      resolvedAs = rewritesResult.asPath
 
-      if (resolvedAs !== as) {
-        const potentialHref = removePathTrailingSlash(
-          this._resolveHref(
-            Object.assign({}, parsed, {
-              pathname: normalizeLocalePath(
-                hasBasePath(resolvedAs) ? delBasePath(resolvedAs) : resolvedAs,
-                this.locales
-              ).pathname,
-            }),
-            pages,
-            false
-          ).pathname!
-        )
-
+      if (rewritesResult.matchedPage && rewritesResult.resolvedHref) {
         // if this directly matches a page we need to update the href to
         // allow the correct page chunk to be loaded
-        if (pages.includes(potentialHref)) {
-          route = potentialHref
-          pathname = potentialHref
-          parsed.pathname = pathname
-          url = formatWithValidation(parsed)
-        }
+        route = rewritesResult.resolvedHref
+        pathname = rewritesResult.resolvedHref
+        parsed.pathname = pathname
+        url = formatWithValidation(parsed)
       }
     }
 
@@ -1045,7 +1029,8 @@ export default class Router implements BaseRouter {
         route,
         pathname,
         query,
-        addBasePath(addLocale(resolvedAs, this.locale)),
+        as,
+        resolvedAs,
         routeProps
       )
       let { error, props, __N_SSG, __N_SSP } = routeInfo
@@ -1092,6 +1077,7 @@ export default class Router implements BaseRouter {
             notFoundRoute,
             query,
             as,
+            resolvedAs,
             { shallow: false }
           )
         }
@@ -1259,6 +1245,7 @@ export default class Router implements BaseRouter {
     pathname: string,
     query: any,
     as: string,
+    resolvedAs: string,
     routeProps: RouteProperties
   ): Promise<PrivateRouteInfo> {
     try {
@@ -1298,7 +1285,7 @@ export default class Router implements BaseRouter {
       if (__N_SSG || __N_SSP) {
         dataHref = this.pageLoader.getDataHref(
           formatWithValidation({ pathname, query }),
-          delBasePath(as),
+          resolvedAs,
           __N_SSG,
           this.locale
         )

packages/next/next-server/lib/router/utils/resolve-rewrites-noop.ts

@@ -0,0 +1 @@
+export default function resolveRewrites() {}

packages/next/next-server/lib/router/utils/resolve-rewrites.ts

@@ -4,6 +4,8 @@ import prepareDestination from './prepare-destination'
 import { Rewrite } from '../../../../lib/load-custom-routes'
 import { removePathTrailingSlash } from '../../../../client/normalize-trailing-slash'
 import { normalizeLocalePath } from '../../i18n/normalize-locale-path'
+import { parseRelativeUrl } from './parse-relative-url'
+import { delBasePath } from '../router'
 
 const customRouteMatcher = pathMatch(true)
 
@@ -14,11 +16,23 @@ export default function resolveRewrites(
   query: ParsedUrlQuery,
   resolveHref: (path: string) => string,
   locales?: string[]
-) {
-  if (!pages.includes(normalizeLocalePath(asPath, locales).pathname)) {
+): {
+  matchedPage: boolean
+  parsedAs: ReturnType<typeof parseRelativeUrl>
+  asPath: string
+  resolvedHref?: string
+} {
+  let matchedPage = false
+  let parsedAs = parseRelativeUrl(asPath)
+  let fsPathname = removePathTrailingSlash(
+    normalizeLocalePath(delBasePath(parsedAs.pathname), locales).pathname
+  )
+  let resolvedHref
+
+  if (!pages.includes(fsPathname)) {
     for (const rewrite of rewrites) {
       const matcher = customRouteMatcher(rewrite.source)
-      const params = matcher(asPath)
+      const params = matcher(parsedAs.pathname)
 
       if (params) {
         if (!rewrite.destination) {
@@ -31,30 +45,36 @@ export default function resolveRewrites(
           query,
           true
         )
-        asPath = destRes.parsedDestination.pathname!
+        parsedAs = destRes.parsedDestination
+        asPath = destRes.newUrl
         Object.assign(query, destRes.parsedDestination.query)
 
-        const fsPathname = normalizeLocalePath(
-          removePathTrailingSlash(asPath),
-          locales
-        ).pathname
+        fsPathname = removePathTrailingSlash(
+          normalizeLocalePath(delBasePath(asPath), locales).pathname
+        )
 
         if (pages.includes(fsPathname)) {
-          asPath = fsPathname
           // check if we now match a page as this means we are done
           // resolving the rewrites
+          matchedPage = true
+          resolvedHref = fsPathname
           break
         }
 
         // check if we match a dynamic-route, if so we break the rewrites chain
-        const resolvedHref = resolveHref(fsPathname)
+        resolvedHref = resolveHref(fsPathname)
 
         if (resolvedHref !== asPath && pages.includes(resolvedHref)) {
-          asPath = fsPathname
+          matchedPage = true
           break
         }
       }
     }
   }
-  return asPath
+  return {
+    asPath,
+    parsedAs,
+    matchedPage,
+    resolvedHref,
+  }
 }

test/integration/custom-routes-i18n/test/index.test.js

@@ -94,7 +94,13 @@ const runTests = () => {
       await browser.elementByCss('#to-about').click()
 
       await check(async () => {
-        const data = JSON.parse(await browser.elementByCss('#data').text())
+        const data = JSON.parse(
+          cheerio
+            .load(await browser.eval('document.documentElement.innerHTML'))(
+              '#data'
+            )
+            .text()
+        )
         console.log(data)
         return data.url === `${expectedIndex ? '/fr' : ''}/about`
           ? 'success'
@@ -108,7 +114,13 @@ const runTests = () => {
         .click()
 
       await check(async () => {
-        const data = JSON.parse(await browser.elementByCss('#data').text())
+        const data = JSON.parse(
+          cheerio
+            .load(await browser.eval('document.documentElement.innerHTML'))(
+              '#data'
+            )
+            .text()
+        )
         console.log(data)
         return data.url === `${expectedIndex ? '/fr' : ''}/hello`
           ? 'success'

test/integration/production/test/index.test.js

@@ -10,6 +10,7 @@ import {
   stopApp,
   waitFor,
   getPageFileFromPagesManifest,
+  check,
 } from 'next-test-utils'
 import webdriver from 'next-webdriver'
 import {
@@ -860,6 +861,27 @@ describe('Production Usage', () => {
     expect(missing).toBe(false)
   })
 
+  it('should preserve query when hard navigating from page 404', async () => {
+    const browser = await webdriver(appPort, '/')
+    await browser.eval(`(function() {
+      window.beforeNav = 1
+      window.next.router.push({
+        pathname: '/non-existent',
+        query: { hello: 'world' }
+      })
+    })()`)
+
+    await check(
+      () => browser.eval('document.documentElement.innerHTML'),
+      /page could not be found/
+    )
+
+    expect(await browser.eval('window.beforeNav')).toBe(null)
+    expect(await browser.eval('window.location.hash')).toBe('')
+    expect(await browser.eval('window.location.search')).toBe('?hello=world')
+    expect(await browser.eval('window.location.pathname')).toBe('/non-existent')
+  })
+
   dynamicImportTests(context, (p, q) => renderViaHTTP(context.appPort, p, q))
 
   processEnv(context)