Update middleware query hydration handling (#41243)

This updates to skip the data request done during query hydration when middleware is present as it was mainly to gather query params from any potential rewrites in middleware although this is usually not needed for static pages and the context can be gathered in different ways on the client. x-ref: [slack thread](https://vercel.slack.com/archives/C045FKE5P51/p1665082474010149) ## Bug - [x] Related issues linked using `fixes #number` - [x] Integration tests added - [ ] Errors have a helpful link attached, see `contributing.md`
vercel · Oct 10, 2022 · 35308c6 · 35308c6
1 parent 05498a0
commit 35308c6
Show file tree

Hide file tree

Showing 13 changed files with 99 additions and 62 deletions.
diff --git a/packages/next/build/webpack-config.ts b/packages/next/build/webpack-config.ts
@@ -253,6 +253,9 @@ export function getDefineEnv({
     'process.env.__NEXT_I18N_SUPPORT': JSON.stringify(!!config.i18n),
     'process.env.__NEXT_I18N_DOMAINS': JSON.stringify(config.i18n?.domains),
     'process.env.__NEXT_ANALYTICS_ID': JSON.stringify(config.analyticsId),
+    'process.env.__NEXT_ALLOW_MIDDLEWARE_RESPONSE_BODY': JSON.stringify(
+      config.experimental.dangerouslyAllowMiddlewareResponseBody
+    ),
     'process.env.__NEXT_NO_MIDDLEWARE_URL_NORMALIZE': JSON.stringify(
       config.experimental.skipMiddlewareUrlNormalize
     ),
@@ -1909,6 +1912,8 @@ export default async function getBaseWebpackConfig(
           dev,
           sriEnabled: !dev && !!config.experimental.sri?.algorithm,
           hasFontLoaders: !!config.experimental.fontLoaders,
+          dangerouslyAllowMiddlewareResponseBody:
+            !!config.experimental.dangerouslyAllowMiddlewareResponseBody,
         }),
       isClient &&
         new BuildManifestPlugin({

diff --git a/packages/next/build/webpack/plugins/middleware-plugin.ts b/packages/next/build/webpack/plugins/middleware-plugin.ts
@@ -340,12 +340,14 @@ function getCodeAnalyzer(params: {
   dev: boolean
   compiler: webpack.Compiler
   compilation: webpack.Compilation
+  dangerouslyAllowMiddlewareResponseBody: boolean
 }) {
   return (parser: webpack.javascript.JavascriptParser) => {
     const {
       dev,
       compiler: { webpack: wp },
       compilation,
+      dangerouslyAllowMiddlewareResponseBody,
     } = params
     const { hooks } = parser
 
@@ -560,8 +562,11 @@ Learn More: https://nextjs.org/docs/messages/node-module-in-edge-runtime`,
         .for(`${prefix}WebAssembly.instantiate`)
         .tap(NAME, handleWrapWasmInstantiateExpression)
     }
-    hooks.new.for('Response').tap(NAME, handleNewResponseExpression)
-    hooks.new.for('NextResponse').tap(NAME, handleNewResponseExpression)
+
+    if (!dangerouslyAllowMiddlewareResponseBody) {
+      hooks.new.for('Response').tap(NAME, handleNewResponseExpression)
+      hooks.new.for('NextResponse').tap(NAME, handleNewResponseExpression)
+    }
     hooks.callMemberChain.for('process').tap(NAME, handleCallMemberChain)
     hooks.expressionMemberChain.for('process').tap(NAME, handleCallMemberChain)
     hooks.importCall.tap(NAME, handleImport)
@@ -801,19 +806,24 @@ export default class MiddlewarePlugin {
   private readonly dev: boolean
   private readonly sriEnabled: boolean
   private readonly hasFontLoaders: boolean
+  private readonly dangerouslyAllowMiddlewareResponseBody: boolean
 
   constructor({
     dev,
     sriEnabled,
     hasFontLoaders,
+    dangerouslyAllowMiddlewareResponseBody,
   }: {
     dev: boolean
     sriEnabled: boolean
     hasFontLoaders: boolean
+    dangerouslyAllowMiddlewareResponseBody: boolean
   }) {
     this.dev = dev
     this.sriEnabled = sriEnabled
     this.hasFontLoaders = hasFontLoaders
+    this.dangerouslyAllowMiddlewareResponseBody =
+      dangerouslyAllowMiddlewareResponseBody
   }
 
   public apply(compiler: webpack.Compiler) {
@@ -826,6 +836,8 @@ export default class MiddlewarePlugin {
         dev: this.dev,
         compiler,
         compilation,
+        dangerouslyAllowMiddlewareResponseBody:
+          this.dangerouslyAllowMiddlewareResponseBody,
       })
       hooks.parser.for('javascript/auto').tap(NAME, codeAnalyzer)
       hooks.parser.for('javascript/dynamic').tap(NAME, codeAnalyzer)

diff --git a/packages/next/server/config-schema.ts b/packages/next/server/config-schema.ts
@@ -268,6 +268,9 @@ const configSchema = {
         appDir: {
           type: 'boolean',
         },
+        dangerouslyAllowMiddlewareResponseBody: {
+          type: 'boolean',
+        },
         externalDir: {
           type: 'boolean',
         },
@@ -323,12 +326,6 @@ const configSchema = {
         optimisticClientCache: {
           type: 'boolean',
         },
-        serverComponentsExternalPackages: {
-          items: {
-            type: 'string',
-          },
-          type: 'array',
-        },
         outputFileTracingRoot: {
           minLength: 1,
           type: 'string',
@@ -348,6 +345,12 @@ const configSchema = {
           enum: ['experimental-edge', 'nodejs'] as any,
           type: 'string',
         },
+        serverComponentsExternalPackages: {
+          items: {
+            type: 'string',
+          },
+          type: 'array',
+        },
         scrollRestoration: {
           type: 'boolean',
         },

diff --git a/packages/next/server/config-shared.ts b/packages/next/server/config-shared.ts
@@ -79,6 +79,7 @@ export interface NextJsWebpackConfig {
 }
 
 export interface ExperimentalConfig {
+  dangerouslyAllowMiddlewareResponseBody?: boolean
   skipMiddlewareUrlNormalize?: boolean
   skipTrailingSlashRedirect?: boolean
   optimisticClientCache?: boolean

diff --git a/packages/next/server/web/adapter.ts b/packages/next/server/web/adapter.ts
@@ -186,6 +186,10 @@ export async function adapter(params: {
 export function blockUnallowedResponse(
   promise: Promise<FetchEventResult>
 ): Promise<FetchEventResult> {
+  if (process.env.__NEXT_ALLOW_MIDDLEWARE_RESPONSE_BODY) {
+    return promise
+  }
+
   return promise.then((result) => {
     if (result.response?.body) {
       console.error(

diff --git a/packages/next/shared/lib/router/router.ts b/packages/next/shared/lib/router/router.ts
@@ -1187,7 +1187,7 @@ export default class Router implements BaseRouter {
     // hydration. Your app should _never_ use this property. It may change at
     // any time without notice.
     const isQueryUpdating = (options as any)._h
-    const shouldResolveHref =
+    let shouldResolveHref =
       isQueryUpdating ||
       (options as any)._shouldResolveHref ||
       parsePath(url).pathname === parsePath(as).pathname
@@ -1418,6 +1418,10 @@ export default class Router implements BaseRouter {
       pathname = this.pathname
     }
 
+    if (isQueryUpdating && isMiddlewareMatch) {
+      shouldResolveHref = false
+    }
+
     if (shouldResolveHref && pathname !== '/_error') {
       ;(options as any)._shouldResolveHref = true
 
@@ -1955,12 +1959,14 @@ export default class Router implements BaseRouter {
         isBackground: isQueryUpdating,
       }
 
-      const data = await withMiddlewareEffects({
-        fetchData: () => fetchNextData(fetchNextDataParams),
-        asPath: resolvedAs,
-        locale: locale,
-        router: this,
-      })
+      const data = isQueryUpdating
+        ? ({} as any)
+        : await withMiddlewareEffects({
+            fetchData: () => fetchNextData(fetchNextDataParams),
+            asPath: resolvedAs,
+            locale: locale,
+            router: this,
+          })
 
       if (isQueryUpdating && data) {
         data.json = self.__NEXT_DATA__.props
@@ -2079,7 +2085,8 @@ export default class Router implements BaseRouter {
       if (
         !this.isPreview &&
         routeInfo.__N_SSG &&
-        process.env.NODE_ENV !== 'development'
+        process.env.NODE_ENV !== 'development' &&
+        !isQueryUpdating
       ) {
         fetchNextData(
           Object.assign({}, fetchNextDataParams, {

diff --git a/test/e2e/middleware-general/test/index.test.ts b/test/e2e/middleware-general/test/index.test.ts
@@ -224,14 +224,13 @@ describe('Middleware Runtime', () => {
 
       await check(
         () => browser.eval('document.documentElement.innerHTML'),
-        /"from":"middleware"/
+        /"slug":"hello"/
       )
 
       await check(() => browser.elementByCss('body').text(), /\/to-ssg/)
 
       expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({
         slug: 'hello',
-        from: 'middleware',
       })
       expect(
         JSON.parse(await browser.elementByCss('#props').text()).params

diff --git a/test/e2e/middleware-trailing-slash/test/index.test.ts b/test/e2e/middleware-trailing-slash/test/index.test.ts
@@ -121,7 +121,6 @@ describe('Middleware Runtime trailing slash', () => {
 
       expect(JSON.parse(await browser.elementByCss('#query').text())).toEqual({
         slug: 'hello',
-        from: 'middleware',
       })
       expect(
         JSON.parse(await browser.elementByCss('#props').text()).params

diff --git a/test/e2e/skip-trailing-slash-redirect/app/middleware.js b/test/e2e/skip-trailing-slash-redirect/app/middleware.js
@@ -23,5 +23,13 @@ export default function handler(req) {
     return res
   }
 
+  if (req.nextUrl.pathname === '/middleware-response-body') {
+    return new Response('hello from middleware', {
+      headers: {
+        'x-from-middleware': 'true',
+      },
+    })
+  }
+
   return NextResponse.next()
 }
diff --git a/test/e2e/skip-trailing-slash-redirect/app/next.config.js b/test/e2e/skip-trailing-slash-redirect/app/next.config.js
@@ -0,0 +1,27 @@
+/** @type {import('next').NextConfig} */
+const nextConfig = {
+  experimental: {
+    skipTrailingSlashRedirect: true,
+    skipMiddlewareUrlNormalize: true,
+    dangerouslyAllowMiddlewareResponseBody: true,
+  },
+  async redirects() {
+    return [
+      {
+        source: '/redirect-me',
+        destination: '/another',
+        permanent: false,
+      },
+    ]
+  },
+  async rewrites() {
+    return [
+      {
+        source: '/rewrite-me',
+        destination: '/another',
+      },
+    ]
+  },
+}
+
+module.exports = nextConfig
diff --git a/test/e2e/skip-trailing-slash-redirect/index.test.ts b/test/e2e/skip-trailing-slash-redirect/index.test.ts
@@ -11,33 +11,17 @@ describe('skip-trailing-slash-redirect', () => {
     next = await createNext({
       files: new FileRef(join(__dirname, 'app')),
       dependencies: {},
-      nextConfig: {
-        experimental: {
-          skipTrailingSlashRedirect: true,
-          skipMiddlewareUrlNormalize: true,
-        },
-        async redirects() {
-          return [
-            {
-              source: '/redirect-me',
-              destination: '/another',
-              permanent: false,
-            },
-          ]
-        },
-        async rewrites() {
-          return [
-            {
-              source: '/rewrite-me',
-              destination: '/another',
-            },
-          ]
-        },
-      },
     })
   })
   afterAll(() => next.destroy())
 
+  it('should allow response body from middleware with flag', async () => {
+    const res = await fetchViaHTTP(next.url, '/middleware-response-body')
+    expect(res.status).toBe(200)
+    expect(res.headers.get('x-from-middleware')).toBe('true')
+    expect(await res.text()).toBe('hello from middleware')
+  })
+
   it('should merge cookies from middleware and API routes correctly', async () => {
     const res = await fetchViaHTTP(next.url, '/api/test-cookie', undefined, {
       redirect: 'manual',

diff --git a/test/integration/dynamic-routing/test/index.test.js b/test/integration/dynamic-routing/test/index.test.js
@@ -47,11 +47,6 @@ function runTests({ dev, serverless }) {
 
       const cacheKeys = await getCacheKeys()
       expect(cacheKeys).toEqual([
-        ...(process.env.__MIDDLEWARE_TEST
-          ? // data route is fetched with middleware due to query hydration
-            // since middleware matches the index route
-            ['/_next/data/BUILD_ID/index.json']
-          : []),
         '/_next/data/BUILD_ID/p1/p2/all-ssg/hello.json?rest=hello',
         '/_next/data/BUILD_ID/p1/p2/all-ssg/hello1/hello2.json?rest=hello1&rest=hello2',
         '/_next/data/BUILD_ID/p1/p2/nested-all-ssg/hello.json?rest=hello',
@@ -97,7 +92,14 @@ function runTests({ dev, serverless }) {
         await browser.waitForElementByCss(linkSelector)
       }
       const newCacheKeys = await getCacheKeys()
-      expect(newCacheKeys).toEqual(cacheKeys)
+      expect(newCacheKeys).toEqual([
+        ...(process.env.__MIDDLEWARE_TEST
+          ? // data route is fetched with middleware due to query hydration
+            // since middleware matches the index route
+            ['/_next/data/BUILD_ID/index.json']
+          : []),
+        ...cacheKeys,
+      ])
     })
   }
 

diff --git a/test/integration/middleware-prefetch/tests/index.test.js b/test/integration/middleware-prefetch/tests/index.test.js
@@ -3,7 +3,6 @@
 import { join } from 'path'
 import fs from 'fs-extra'
 import webdriver from 'next-webdriver'
-import assert from 'assert'
 import { check, findPort, killApp, nextBuild, nextStart } from 'next-test-utils'
 
 jest.setTimeout(1000 * 60 * 2)
@@ -62,19 +61,6 @@ describe('Middleware Production Prefetch', () => {
     }, 'yes')
   })
 
-  it(`prefetches data when it is ssg`, async () => {
-    const browser = await webdriver(context.appPort, `/`)
-    await browser.elementByCss('#made-up-link').moveTo()
-    await check(async () => {
-      const hrefs = await browser.eval(`Object.keys(window.next.router.sdc)`)
-      const mapped = hrefs.map((href) =>
-        new URL(href).pathname.replace(/^\/_next\/data\/[^/]+/, '')
-      )
-      assert.deepEqual(mapped, ['/index.json'])
-      return 'yes'
-    }, 'yes')
-  })
-
   it(`prefetches provided path even if it will be rewritten`, async () => {
     const browser = await webdriver(context.appPort, `/`)
     await browser.elementByCss('#ssg-page-2').moveTo()