fix node.js module warning in middleware (#65112)

`server-utils` should not be imported into middleware as it contains imports that won't work in the edge runtime. Those imports aren't technically used, but it was causing a warning nonetheless. This adds a regression test to confirm the warning is only present when actually importing a Node.js module in userland. Closes NEXT-3246
vercel · Apr 27, 2024 · 3050f45 · 3050f45
1 parent 5897fc0
commit 3050f45
Show file tree

Hide file tree

Showing 6 changed files with 104 additions and 21 deletions.
diff --git a/packages/next/src/server/base-server.ts b/packages/next/src/server/base-server.ts
@@ -70,7 +70,7 @@ import { removeTrailingSlash } from '../shared/lib/router/utils/remove-trailing-
 import { denormalizePagePath } from '../shared/lib/page-path/denormalize-page-path'
 import * as Log from '../build/output/log'
 import escapePathDelimiters from '../shared/lib/router/utils/escape-path-delimiters'
-import { getUtils, normalizeNextQueryParam } from './server-utils'
+import { getUtils } from './server-utils'
 import isError, { getProperError } from '../lib/is-error'
 import {
   addRequestMeta,
@@ -109,6 +109,7 @@ import { sendResponse } from './send-response'
 import { handleInternalServerErrorResponse } from './future/route-modules/helpers/response-handlers'
 import {
   fromNodeOutgoingHttpHeaders,
+  normalizeNextQueryParam,
   toNodeOutgoingHttpHeaders,
 } from './web/utils'
 import { CACHE_ONE_YEAR, NEXT_CACHE_TAGS_HEADER } from '../lib/constants'

diff --git a/packages/next/src/server/server-utils.ts b/packages/next/src/server/server-utils.ts
@@ -55,24 +55,6 @@ export function normalizeVercelUrl(
   }
 }
 
-/**
- * Normalizes `nxtP` and `nxtI` query param values to remove the prefix.
- * This function does not mutate the input key; it calls the provided function
- * with the normalized key.
- */
-export function normalizeNextQueryParam(
-  key: string,
-  onKeyNormalized: (normalizedKey: string) => void
-) {
-  const prefixes = [NEXT_QUERY_PARAM_PREFIX, NEXT_INTERCEPTION_MARKER_PREFIX]
-  for (const prefix of prefixes) {
-    if (key !== prefix && key.startsWith(prefix)) {
-      const normalizedKey = key.substring(prefix.length)
-      onKeyNormalized(normalizedKey)
-    }
-  }
-}
-
 export function interpolateDynamicPath(
   pathname: string,
   params: ParsedUrlQuery,

diff --git a/packages/next/src/server/web/adapter.ts b/packages/next/src/server/web/adapter.ts
@@ -2,7 +2,7 @@ import type { RequestData, FetchEventResult } from './types'
 import type { RequestInit } from './spec-extension/request'
 import type { PrerenderManifest } from '../../build'
 import { PageSignatureError } from './error'
-import { fromNodeOutgoingHttpHeaders } from './utils'
+import { fromNodeOutgoingHttpHeaders, normalizeNextQueryParam } from './utils'
 import { NextFetchEvent } from './spec-extension/fetch-event'
 import { NextRequest } from './spec-extension/request'
 import { NextResponse } from './spec-extension/response'
@@ -18,7 +18,6 @@ import { requestAsyncStorage } from '../../client/components/request-async-stora
 import { getTracer } from '../lib/trace/tracer'
 import type { TextMapGetter } from 'next/dist/compiled/@opentelemetry/api'
 import { MiddlewareSpan } from '../lib/trace/constants'
-import { normalizeNextQueryParam } from '../server-utils'
 
 export class NextRequestHint extends NextRequest {
   sourcePage: string

diff --git a/packages/next/src/server/web/utils.ts b/packages/next/src/server/web/utils.ts
@@ -1,4 +1,8 @@
 import type { OutgoingHttpHeaders } from 'http'
+import {
+  NEXT_INTERCEPTION_MARKER_PREFIX,
+  NEXT_QUERY_PARAM_PREFIX,
+} from '../../lib/constants'
 
 /**
  * Converts a Node.js IncomingHttpHeaders object to a Headers object. Any
@@ -146,3 +150,21 @@ export function validateURL(url: string | URL): string {
     )
   }
 }
+
+/**
+ * Normalizes `nxtP` and `nxtI` query param values to remove the prefix.
+ * This function does not mutate the input key; it calls the provided function
+ * with the normalized key.
+ */
+export function normalizeNextQueryParam(
+  key: string,
+  onKeyNormalized: (normalizedKey: string) => void
+) {
+  const prefixes = [NEXT_QUERY_PARAM_PREFIX, NEXT_INTERCEPTION_MARKER_PREFIX]
+  for (const prefix of prefixes) {
+    if (key !== prefix && key.startsWith(prefix)) {
+      const normalizedKey = key.substring(prefix.length)
+      onKeyNormalized(normalizedKey)
+    }
+  }
+}
diff --git a/test/production/app-dir/app-edge-middleware/app-edge-middleware.test.ts b/test/production/app-dir/app-edge-middleware/app-edge-middleware.test.ts
@@ -0,0 +1,68 @@
+import { nextTestSetup } from 'e2e-utils'
+
+describe('app edge middleware', () => {
+  describe('without node.js modules', () => {
+    const { next } = nextTestSetup({
+      files: {
+        'app/page.js': `
+          export default function Page() {
+            return <p>hello world</p>
+          }
+      `,
+        'app/layout.js': `
+          export default function Root({ children }) { 
+            return (
+              <html>
+                <body>
+                  {children}
+                </body>
+              </html>
+            ) 
+          }
+        `,
+        'middleware.js': `
+          import { NextResponse } from 'next/server';
+          export async function middleware() { 
+            return NextResponse.next() 
+          }
+      `,
+      },
+    })
+    it('should not have any errors about using Node.js modules if not present in middleware', async () => {
+      expect(next.cliOutput).not.toContain('node-module-in-edge-runtime')
+    })
+  })
+
+  describe('with node.js modules', () => {
+    const { next } = nextTestSetup({
+      files: {
+        'app/page.js': `
+          export default function Page() {
+            return <p>hello world</p>
+          }
+      `,
+        'app/layout.js': `
+          export default function Root({ children }) { 
+            return (
+              <html>
+                <body>
+                  {children}
+                </body>
+              </html>
+            ) 
+          }
+        `,
+        'middleware.js': `
+          import { NextResponse } from 'next/server';
+          import { parse } from 'url';
+          export async function middleware() { 
+            return NextResponse.next() 
+          }
+      `,
+      },
+    })
+    it('should have errors about using Node.js modules when present in middleware', async () => {
+      expect(next.cliOutput).toContain('node-module-in-edge-runtime')
+    })
+  })
+})
diff --git a/test/turbopack-build-tests-manifest.json b/test/turbopack-build-tests-manifest.json
@@ -14615,6 +14615,17 @@
       "flakey": [],
       "runtimeError": false
     },
+    "test/production/app-dir/app-edge-middleware/app-edge-middleware.test.ts": {
+      "passed": [
+        "app edge middleware without node.js modules should not have any errors about using Node.js modules if not present in middleware"
+      ],
+      "failed": [
+        "app edge middleware with node.js modules should have errors about using Node.js modules when present in middleware"
+      ],
+      "pending": [],
+      "flakey": [],
+      "runtimeError": false
+    },
     "test/production/app-dir/app-fetch-build-cache/app-fetch-build-cache.test.ts": {
       "passed": [
         "app fetch build cache should have done initial build",