Ensure we hard navigate for failed _next/data with middleware (#39148)

* Ensure we hard navigate for failed _next/data with middleware * fix-lint
vercel · Jul 28, 2022 · e3efc03 · e3efc03
1 parent a818606
commit e3efc03
Show file tree

Hide file tree

Showing 4 changed files with 30 additions and 12 deletions.
diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts
@@ -481,7 +481,7 @@ function fetchNextData({
               return { dataHref, response, text, json: {} }
             }
 
-            if (response.status === 404) {
+            if (!hasMiddleware && response.status === 404) {
               if (tryToParseAsJSON(text)?.notFound) {
                 return {
                   dataHref,
@@ -490,16 +490,6 @@ function fetchNextData({
                   text,
                 }
               }
-
-              /**
-               * If there is a 404 that is not for SSG we used to fail but if
-               * there is a middleware we must respond with an empty object.
-               * For now we will return the data when there is a middleware.
-               * TODO: Update the server to success on these requests.
-               */
-              if (hasMiddleware) {
-                return { dataHref, response, text, json: {} }
-              }
             }
 
             const error = new Error(`Failed to load static props`)
@@ -2341,7 +2331,14 @@ function getMiddlewareData<T extends FetchDataOutput>(
 
   const matchedPath = response.headers.get('x-matched-path')
 
-  if (!rewriteTarget && !matchedPath?.includes('__next_data_catchall')) {
+  if (
+    matchedPath &&
+    !rewriteTarget &&
+    !matchedPath.includes('__next_data_catchall') &&
+    !matchedPath.includes('/_error') &&
+    !matchedPath.includes('/404')
+  ) {
+    // leverage x-matched-path to detect next.config.js rewrites
     rewriteTarget = matchedPath
   }
 

diff --git a/test/e2e/middleware-rewrites/app/middleware.js b/test/e2e/middleware-rewrites/app/middleware.js
@@ -13,6 +13,12 @@ export async function middleware(request) {
     return NextResponse.next()
   }
 
+  if (url.pathname.includes('/to/some/404/path')) {
+    return NextResponse.next({
+      'x-matched-path': '/404',
+    })
+  }
+
   if (url.pathname.includes('/fallback-true-blog/rewritten')) {
     request.nextUrl.pathname = '/about'
     return NextResponse.rewrite(request.nextUrl)

diff --git a/test/e2e/middleware-rewrites/app/pages/404.js b/test/e2e/middleware-rewrites/app/pages/404.js
@@ -0,0 +1,3 @@
+export default function NotFound() {
+  return <p>custom 404 page</p>
+}
diff --git a/test/e2e/middleware-rewrites/test/index.test.ts b/test/e2e/middleware-rewrites/test/index.test.ts
@@ -27,6 +27,18 @@ describe('Middleware Rewrite', () => {
   testsWithLocale('/fr')
 
   function tests() {
+    it('should hard navigate on 404 for data request', async () => {
+      const browser = await webdriver(next.url, '/')
+      await browser.eval('window.beforeNav = 1')
+      await browser.eval(`next.router.push("/to/some/404/path")`)
+      await check(
+        () => browser.eval('document.documentElement.innerHTML'),
+        /custom 404 page/
+      )
+      expect(await browser.eval('location.pathname')).toBe('/to/some/404/path')
+      expect(await browser.eval('window.beforeNav')).not.toBe(1)
+    })
+
     // TODO: middleware effect headers aren't available here
     it.skip('includes the locale in rewrites by default', async () => {
       const res = await fetchViaHTTP(next.url, `/rewrite-me-to-about`)