Run with --enable-source-maps by default in next dev (#71820)

We already create server source maps in dev by default, so we should make use of them. To opt-out, run `next dev --disable-source-maps` instead. Our internal `next-no-sourcemaps` script is now defunct.
vercel · Oct 28, 2024 · 655b808 · 655b808
1 parent bfdf527
commit 655b808
Show file tree

Hide file tree

Showing 13 changed files with 332 additions and 54 deletions.
diff --git a/package.json b/package.json
@@ -51,8 +51,8 @@
     "prepublishOnly": "turbo run build",
     "lint-staged": "lint-staged",
     "next-with-deps": "./scripts/next-with-deps.sh",
-    "next": "cross-env NEXT_TELEMETRY_DISABLED=1 node --trace-deprecation --enable-source-maps packages/next/dist/bin/next",
-    "next-no-sourcemaps": "cross-env NEXT_TELEMETRY_DISABLED=1 node --trace-deprecation packages/next/dist/bin/next",
+    "next": "cross-env NEXT_TELEMETRY_DISABLED=1 NODE_OPTIONS=\"--trace-deprecation --enable-source-maps\" next",
+    "next-no-sourcemaps": "echo 'No longer supported. Use `pnpm next --disable-source-maps` instead'; exit 1;",
     "clean-trace-jaeger": "node scripts/rm.mjs test/integration/basic/.next && TRACE_TARGET=JAEGER pnpm next build test/integration/basic",
     "debug": "cross-env NEXT_TELEMETRY_DISABLED=1 node --inspect --trace-deprecation --enable-source-maps packages/next/dist/bin/next",
     "postinstall": "node scripts/git-configure.mjs && node scripts/install-native.mjs",

diff --git a/packages/next/src/bin/next.ts b/packages/next/src/bin/next.ts
@@ -172,6 +172,11 @@ program
     '-H, --hostname <hostname>',
     'Specify a hostname on which to start the application (default: 0.0.0.0).'
   )
+  .option(
+    '--disable-source-maps',
+    "Don't start the Dev server with `--enable-source-maps`.",
+    false
+  )
   .option(
     '--experimental-https',
     'Starts the server with HTTPS and generates a self-signed certificate.'

diff --git a/packages/next/src/cli/next-dev.ts b/packages/next/src/cli/next-dev.ts
@@ -40,6 +40,7 @@ import { flushAllTraces, trace } from '../trace'
 import { traceId } from '../trace/shared'
 
 export type NextDevOptions = {
+  disableSourceMaps: boolean
   turbo?: boolean
   turbopack?: boolean
   port: number
@@ -267,6 +268,12 @@ const nextDev = async (
         delete nodeOptions['max_old_space_size']
       }
 
+      if (options.disableSourceMaps) {
+        delete nodeOptions['enable-source-maps']
+      } else {
+        nodeOptions['enable-source-maps'] = true
+      }
+
       if (nodeDebugType) {
         const address = getParsedDebugAddress()
         address.port = address.port + 1

diff --git a/run-tests.js b/run-tests.js
@@ -480,6 +480,10 @@ ${ENDGROUP}`)
         // unset CI env so CI behavior is only explicitly
         // tested when enabled
         CI: '',
+        // But some tests need to fork based on machine? CI? behavior differences
+        // Only use read this in tests.
+        // For implementation forks, use `process.env.CI` instead
+        NEXT_TEST_CI: process.env.CI,
 
         ...(options.local
           ? {}

diff --git a/test/development/middleware-errors/index.test.ts b/test/development/middleware-errors/index.test.ts
@@ -3,6 +3,7 @@ import {
   assertNoRedbox,
   check,
   getRedboxSource,
+  retry,
 } from 'next-test-utils'
 import stripAnsi from 'strip-ansi'
 import { nextTestSetup } from 'e2e-utils'
@@ -31,23 +32,22 @@ describe('middleware - development errors', () => {
 
     it('logs the error correctly', async () => {
       await next.fetch('/')
-      const output = stripAnsi(next.cliOutput)
-      await check(() => {
-        if (isTurbopack) {
-          expect(stripAnsi(next.cliOutput)).toMatch(
-            /middleware.js \(\d+:\d+\) @ __TURBOPACK__default__export__/
-          )
-        } else {
-          expect(stripAnsi(next.cliOutput)).toMatch(
-            /middleware.js \(\d+:\d+\) @ default/
-          )
-        }
-
-        expect(stripAnsi(next.cliOutput)).toMatch(/boom/)
-        return 'success'
-      }, 'success')
-      expect(output).not.toContain(
-        'webpack-internal:///(middleware)/./middleware.js'
+
+      await retry(() => {
+        expect(stripAnsi(next.cliOutput)).toContain('boom')
+      })
+      // TODO: assert on full, ignore-listed stack
+      expect(stripAnsi(next.cliOutput)).toContain(
+        isTurbopack
+          ? '\n ⨯ middleware.js (3:15) @ __TURBOPACK__default__export__' +
+              '\n ⨯ Error: boom' +
+              '\n    at __TURBOPACK__default__export__ (./middleware.js:3:15)'
+          : '\n ⨯ middleware.js (3:15) @ default' +
+              '\n ⨯ boom' +
+              '\n  1 |' +
+              '\n  2 |       export default function () {' +
+              "\n> 3 |         throw new Error('boom')" +
+              '\n    |               ^'
       )
     })
 
@@ -79,13 +79,20 @@ describe('middleware - development errors', () => {
 
     it('logs the error correctly', async () => {
       await next.fetch('/')
-      await check(
-        () => stripAnsi(next.cliOutput),
-        new RegExp(`unhandledRejection: Error: async boom!`, 'm')
+
+      await retry(() => {
+        expect(stripAnsi(next.cliOutput)).toContain(
+          'unhandledRejection: Error: async boom!'
+        )
+      })
+      // TODO: assert on full, ignore-listed stack
+      expect(stripAnsi(next.cliOutput)).toContain(
+        isTurbopack
+          ? 'unhandledRejection: Error: async boom!\n    at throwError ('
+          : 'unhandledRejection: Error: async boom!' +
+              '\n    at throwError (webpack-internal:///(middleware)/./middleware.js:8:11)' +
+              '\n    at __WEBPACK_DEFAULT_EXPORT__ (webpack-internal:///(middleware)/./middleware.js:11:5)'
       )
-      // expect(output).not.toContain(
-      //   'webpack-internal:///(middleware)/./middleware.js'
-      // )
     })
 
     it('does not render the error', async () => {
@@ -112,17 +119,37 @@ describe('middleware - development errors', () => {
 
     it('logs the error correctly', async () => {
       await next.fetch('/')
-      // const output = stripAnsi(next.cliOutput)
-      await check(() => {
-        expect(stripAnsi(next.cliOutput)).toMatch(
-          /middleware.js \(\d+:\d+\) @ eval/
+
+      await retry(() => {
+        expect(stripAnsi(next.cliOutput)).toContain('Dynamic Code Evaluation')
+      })
+      // TODO: assert on full, ignore-listed stack
+      if (isTurbopack) {
+        // Locally, prefixes the "test is not defined".
+        // In CI, it prefixes "Dynamic Code Evaluation".
+        expect(stripAnsi(next.cliOutput)).toContain(
+          '\n ⚠ middleware.js (3:22) @ __TURBOPACK__default__export__' +
+            '\n ⨯ middleware.js (4:9) @ eval'
         )
-        expect(stripAnsi(next.cliOutput)).toMatch(/test is not defined/)
-        return 'success'
-      }, 'success')
-      // expect(output).not.toContain(
-      //   'webpack-internal:///(middleware)/./middleware.js'
-      // )
+      }
+      expect(stripAnsi(next.cliOutput)).toContain(
+        isTurbopack
+          ? '\n ⨯ Error: test is not defined' +
+              '\n    at eval (./middleware.js:4:9)' +
+              '\n    at <unknown> (./middleware.js:4:9'
+          : '\n ⨯ Error [ReferenceError]: test is not defined' +
+              '\n    at eval (file://webpack-internal:///(middleware)/./middleware.js)' +
+              '\n    at eval (webpack://_N_E/middleware.js?3bcb:4:8)'
+      )
+      expect(stripAnsi(next.cliOutput)).toContain(
+        isTurbopack
+          ? "\n ⚠ Error: Dynamic Code Evaluation (e. g. 'eval', 'new Function') not allowed in Edge Runtime" +
+              '\nLearn More: https://nextjs.org/docs/messages/edge-dynamic-code-evaluation' +
+              '\n    at __TURBOPACK__default__export__ (./middleware.js:3:22)'
+          : '\n ⚠ middleware.js (4:9) @ eval' +
+              "\n ⚠ Dynamic Code Evaluation (e. g. 'eval', 'new Function') not allowed in Edge Runtime" +
+              '\nLearn More: https://nextjs.org/docs/messages/edge-dynamic-code-evaluation'
+      )
     })
 
     it('renders the error correctly and recovers', async () => {
@@ -150,22 +177,18 @@ describe('middleware - development errors', () => {
 
     it('logs the error correctly', async () => {
       await next.fetch('/')
-      const output = stripAnsi(next.cliOutput)
-      await check(() => {
-        if (isTurbopack) {
-          expect(stripAnsi(next.cliOutput)).toMatch(
-            /middleware.js \(\d+:\d+\) @ \[project\]\/middleware\.js \[middleware\] \(ecmascript\)/
-          )
-        } else {
-          expect(stripAnsi(next.cliOutput)).toMatch(
-            /middleware.js \(\d+:\d+\) @ <unknown>/
-          )
-        }
-        expect(stripAnsi(next.cliOutput)).toMatch(/booooom!/)
-        return 'success'
-      }, 'success')
-      expect(output).not.toContain(
-        'webpack-internal:///(middleware)/./middleware.js'
+
+      await retry(() => {
+        expect(stripAnsi(next.cliOutput)).toContain(`Error: booooom!`)
+      })
+      // TODO: assert on full, ignore-listed stack
+      expect(stripAnsi(next.cliOutput)).toContain(
+        isTurbopack
+          ? '\n ⨯ middleware.js (3:13) @ [project]/middleware.js [middleware] (ecmascript)' +
+              '\n ⨯ Error: booooom!' +
+              '\n    at <unknown> ([project]/middleware.js [middleware] (ecmascript) (./middleware.js:3:13)'
+          : '\n ⨯ Error: booooom!' +
+              '\n    at <unknown> (webpack://_N_E/middleware.js'
       )
     })
 

diff --git a/test/e2e/app-dir/server-source-maps/fixtures/default/.gitignore b/test/e2e/app-dir/server-source-maps/fixtures/default/.gitignore
@@ -0,0 +1,2 @@
+node_modules/
+pnpm-lock.yaml
diff --git a/test/e2e/app-dir/server-source-maps/fixtures/default/app/rsc-error-log-ignore-listed/page.js b/test/e2e/app-dir/server-source-maps/fixtures/default/app/rsc-error-log-ignore-listed/page.js
@@ -0,0 +1,11 @@
+import { run } from 'internal-pkg'
+
+function logError() {
+  const error = new Error('Boom')
+  console.error(error)
+}
+
+export default function Page() {
+  run(() => logError())
+  return null
+}
diff --git a/test/e2e/app-dir/server-source-maps/fixtures/default/app/ssr-error-log-ignore-listed/page.js b/test/e2e/app-dir/server-source-maps/fixtures/default/app/ssr-error-log-ignore-listed/page.js
@@ -0,0 +1,12 @@
+'use client'
+import { run } from 'internal-pkg'
+
+function logError() {
+  const error = new Error('Boom')
+  console.error(error)
+}
+
+export default function Page() {
+  run(() => logError())
+  return null
+}
diff --git a/test/e2e/app-dir/server-source-maps/fixtures/default/app/ssr-error-log/page.js b/test/e2e/app-dir/server-source-maps/fixtures/default/app/ssr-error-log/page.js
@@ -0,0 +1,11 @@
+'use client'
+
+function logError() {
+  const error = new Error('Boom')
+  console.error(error)
+}
+
+export default function Page() {
+  logError()
+  return null
+}
diff --git a/test/e2e/app-dir/server-source-maps/fixtures/default/internal-pkg/index.js b/test/e2e/app-dir/server-source-maps/fixtures/default/internal-pkg/index.js
@@ -0,0 +1,3 @@
+export function run(fn) {
+  return fn()
+}
diff --git a/test/e2e/app-dir/server-source-maps/fixtures/default/internal-pkg/package.json b/test/e2e/app-dir/server-source-maps/fixtures/default/internal-pkg/package.json
@@ -0,0 +1,10 @@
+{
+  "name": "internal-pkg",
+  "private": true,
+  "type": "module",
+  "exports": {
+    ".": {
+      "default": "./index.js"
+    }
+  }
+}
diff --git a/test/e2e/app-dir/server-source-maps/fixtures/default/package.json b/test/e2e/app-dir/server-source-maps/fixtures/default/package.json
@@ -0,0 +1,5 @@
+{
+  "dependencies": {
+    "internal-pkg": "file:./internal-pkg"
+  }
+}