From 2e5218c7781ab98c636594e0f36a502425f88177 Mon Sep 17 00:00:00 2001
From: Javi Velasco <javier.velasco86@gmail.com>
Date: Fri, 26 Nov 2021 13:46:57 +0100
Subject: [PATCH] Fix hydration middleware effects (#31800)

Whenever we trigger a route change in the client we check if there route we are navigating to is affected by a middleware. When this is the case we run a preflight and in case there is an effect that tells us that the middleware is responding with content we force a _refresh_. This is fine for navigation in general but it is not ok when the change is triggered for hydration. For example, in cases where the rendered page is a dynamic page this triggers an infinite reload.

In this PR we add a test where we add a `_middleware` that proxies to a dynamic path. When making a request to `/interface/root-subrequest` there will be a middleware that simply performs a fetch against localhost for the same `/interface/root-subrequest`. The new request will skip the middleware to avoid loops and then render the dynamic page. Then client will force a change for hydration resulting in a preflight request that tells that the client must refresh because for that path there is a middleware writing content.

Then we add a fix which simply consist of checking the internal option that tells when a change is triggered for hydration and skip the preflight in such scenario.

## Bug

- [ ] 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`
---
 packages/next/shared/lib/router/router.ts     | 49 +++++++++++--------
 .../core/pages/interface/_middleware.js       |  6 +++
 .../middleware/core/test/index.test.js        |  9 ++++
 3 files changed, 43 insertions(+), 21 deletions(-)

diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts
index f855aa2fbbccc..2d4826e278fa8 100644
--- a/packages/next/shared/lib/router/router.ts
+++ b/packages/next/shared/lib/router/router.ts
@@ -1133,28 +1133,35 @@ export default class Router implements BaseRouter {
 
     resolvedAs = delLocale(delBasePath(resolvedAs), this.locale)
 
-    const effect = await this._preflightRequest({
-      as,
-      cache: process.env.NODE_ENV === 'production',
-      pages,
-      pathname,
-      query,
-    })
+    /**
+     * If the route update was triggered for client-side hydration then
+     * do not check the preflight request. Otherwise when rendering
+     * a page with refresh it might get into an infinite loop.
+     */
+    if ((options as any)._h !== 1) {
+      const effect = await this._preflightRequest({
+        as,
+        cache: process.env.NODE_ENV === 'production',
+        pages,
+        pathname,
+        query,
+      })
 
-    if (effect.type === 'rewrite') {
-      query = { ...query, ...effect.parsedAs.query }
-      resolvedAs = effect.asPath
-      pathname = effect.resolvedHref
-      parsed.pathname = effect.resolvedHref
-      url = formatWithValidation(parsed)
-    } else if (effect.type === 'redirect' && effect.newAs) {
-      return this.change(method, effect.newUrl, effect.newAs, options)
-    } else if (effect.type === 'redirect' && effect.destination) {
-      window.location.href = effect.destination
-      return new Promise(() => {})
-    } else if (effect.type === 'refresh') {
-      window.location.href = as
-      return new Promise(() => {})
+      if (effect.type === 'rewrite') {
+        query = { ...query, ...effect.parsedAs.query }
+        resolvedAs = effect.asPath
+        pathname = effect.resolvedHref
+        parsed.pathname = effect.resolvedHref
+        url = formatWithValidation(parsed)
+      } else if (effect.type === 'redirect' && effect.newAs) {
+        return this.change(method, effect.newUrl, effect.newAs, options)
+      } else if (effect.type === 'redirect' && effect.destination) {
+        window.location.href = effect.destination
+        return new Promise(() => {})
+      } else if (effect.type === 'refresh') {
+        window.location.href = as
+        return new Promise(() => {})
+      }
     }
 
     const route = removePathTrailingSlash(pathname)
diff --git a/test/integration/middleware/core/pages/interface/_middleware.js b/test/integration/middleware/core/pages/interface/_middleware.js
index b59a76d9eef45..a3ac20fd7c379 100644
--- a/test/integration/middleware/core/pages/interface/_middleware.js
+++ b/test/integration/middleware/core/pages/interface/_middleware.js
@@ -53,6 +53,12 @@ export async function middleware(request) {
     }
   }
 
+  if (url.pathname.endsWith('/root-subrequest')) {
+    return fetch(
+      `http://${request.headers.get('host')}/interface/root-subrequest`
+    )
+  }
+
   return new Response(null, {
     headers: {
       'req-url-basepath': request.nextUrl.basePath,
diff --git a/test/integration/middleware/core/test/index.test.js b/test/integration/middleware/core/test/index.test.js
index c92a60b8627f7..f0e10c2328414 100644
--- a/test/integration/middleware/core/test/index.test.js
+++ b/test/integration/middleware/core/test/index.test.js
@@ -438,6 +438,15 @@ function interfaceTests(locale = '') {
       expect(res.headers.get('req-url-locale')).toBe(locale.slice(1))
     }
   })
+
+  it(`${locale} renders correctly rewriting with a root subrequest`, async () => {
+    const browser = await webdriver(
+      context.appPort,
+      '/interface/root-subrequest'
+    )
+    const element = await browser.elementByCss('.title')
+    expect(await element.text()).toEqual('Dynamic route')
+  })
 }
 
 function getCookieFromResponse(res, cookieName) {