Turbopack: respect productionBrowserSourceMaps (#78014)

Let's try this again

~~Blocked by #78058
Blocked by #82448

- [x] Add regression test
- [x] Update failing assertions

Closes PACK-4304

Author: mischnic

crates/next-core/src/next_config.rs

@@ -1701,12 +1701,12 @@ impl NextConfig {
     }
 
     #[turbo_tasks::function]
-    pub fn client_source_maps(&self, _mode: Vc<NextMode>) -> Result<Vc<bool>> {
-        // Temporarily always enable client source maps as tests regress.
-        // TODO: Respect both `self.experimental.turbopack_source_maps` and
-        //       `self.production_browser_source_maps`
+    pub async fn client_source_maps(&self, mode: Vc<NextMode>) -> Result<Vc<bool>> {
         let source_maps = self.experimental.turbopack_source_maps;
-        Ok(Vc::cell(source_maps.unwrap_or(true)))
+        Ok(Vc::cell(source_maps.unwrap_or(match &*mode.await? {
+            NextMode::Development => true,
+            NextMode::Build => self.production_browser_source_maps,
+        })))
     }
 
     #[turbo_tasks::function]

test/integration/css-fixtures/next.config.js

@@ -3,8 +3,5 @@ module.exports = {
     // Make sure entries are not getting disposed.
     maxInactiveAge: 1000 * 60 * 60,
   },
-  webpack(cfg) {
-    cfg.devtool = 'source-map'
-    return cfg
-  },
+  productionBrowserSourceMaps: true,
 }

test/integration/css/test/css-compilation.test.js

@@ -116,39 +116,8 @@ module.exports = {
               delete sourceMapContentParsed.file
               delete sourceMapContentParsed.sources
 
-              if (process.env.IS_TURBOPACK_TEST && useLightningcss) {
-                expect(sourceMapContentParsed).toMatchInlineSnapshot(`
-                 {
-                   "mappings": "AAAA,qDACE,2BAKF,0DAIA,kDAIA,uCAIA",
-                   "names": [],
-                   "sourcesContent": [
-                     "@media (480px <= width < 768px) {
-                   ::placeholder {
-                     color: green;
-                   }
-                 }
-
-                 .flex-parsing {
-                   flex: 0 0 calc(50% - var(--vertical-gutter));
-                 }
-
-                 .transform-parsing {
-                   transform: translate3d(0px, 0px);
-                 }
-
-                 .css-grid-shorthand {
-                   grid-column: span 2;
-                 }
-
-                 .g-docs-sidenav .filter::-webkit-input-placeholder {
-                   opacity: 80%;
-                 }
-                 ",
-                   ],
-                   "version": 3,
-                 }
-                `)
-              } else if (process.env.IS_TURBOPACK_TEST && !useLightningcss) {
+              if (process.env.IS_TURBOPACK_TEST) {
+                // Turbopack always uses lightningcss
                 expect(sourceMapContentParsed).toMatchInlineSnapshot(`
                  {
                    "mappings": "AAAA,qDACE,2BAKF,0DAIA,kDAIA,uCAIA",
@@ -183,6 +152,7 @@ module.exports = {
               } else if (useLightningcss) {
                 expect(sourceMapContentParsed).toMatchInlineSnapshot(`
                  {
+                   "ignoreList": [],
                    "mappings": "AAAA,qDACE,cACE,WACF,CACF,CAEA,cACE,2CACF,CAEA,mBACE,0BACF,CAEA,oBACE,kBACF,CAEA,mDACE,UACF",
                    "names": [],
                    "sourceRoot": "",
@@ -217,6 +187,7 @@ module.exports = {
               } else {
                 expect(sourceMapContentParsed).toMatchInlineSnapshot(`
                   {
+                    "ignoreList": [],
                     "mappings": "AAAA,+CACE,cACE,WACF,CACF,CAEA,cACE,2CACF,CAEA,mBACE,0BACF,CAEA,oBACE,kBACF,CAEA,mDACE,WACF",
                     "names": [],
                     "sourceRoot": "",

test/integration/production-browser-sourcemaps/next.config.js

@@ -0,0 +1,78 @@
+import fs from 'fs-extra'
+import path from 'path'
+import { getBuildManifest } from 'next-test-utils'
+import { recursiveReadDir } from 'next/dist/lib/recursive-readdir'
+import { nextTestSetup } from 'e2e-utils'
+
+function extractSourceMappingURL(jsContent): string | null {
+  // Matches both //# and //@ sourceMappingURL=...
+  const match = jsContent.match(/\/\/[#@] sourceMappingURL=([^\s]+)/)
+  return match ? match[1] : null
+}
+
+async function sourceMapExistsForFile(jsFilePath) {
+  const jsContent = await fs.readFile(jsFilePath, 'utf8')
+  const sourceMappingURL = extractSourceMappingURL(jsContent)
+  if (!sourceMappingURL) {
+    if (/^__turbopack_load_page_chunks__\(["']/.test(jsContent)) {
+      // There is no sourcemap in these loader chunks, ignore
+      return undefined
+    }
+    return false
+  }
+  const mapPath = path.join(path.dirname(jsFilePath), sourceMappingURL)
+  return await fs.pathExists(mapPath)
+}
+
+describe('Production browser sourcemaps', () => {
+  describe.each([false, true] as const)(
+    'productionBrowserSourceMaps = %s',
+    (productionBrowserSourceMaps) => {
+      const { next, skipped } = nextTestSetup({
+        files: __dirname,
+        nextConfig: {
+          productionBrowserSourceMaps,
+        },
+        skipDeployment: true,
+      })
+
+      if (skipped) {
+        return
+      }
+
+      it('check sourcemaps for all browser files', async () => {
+        const buildManifest = getBuildManifest(next.testDir)
+
+        // These currently don't have sourcemaps
+        let polyfillFiles = new Set(
+          buildManifest.polyfillFiles.map((f) => '/' + path.basename(f))
+        )
+
+        const staticDir = path.join(next.testDir, '.next', 'static', 'chunks')
+        const browserFiles = await recursiveReadDir(staticDir)
+        const jsFiles = browserFiles.filter(
+          (file) => file.endsWith('.js') && !polyfillFiles.has(file)
+        )
+        expect(jsFiles).not.toBeEmpty()
+
+        for (const file of jsFiles) {
+          const jsPath = path.join(staticDir, file)
+          expect(await sourceMapExistsForFile(jsPath)).toBeOneOf([
+            productionBrowserSourceMaps,
+            undefined,
+          ])
+        }
+
+        for (let page of ['/ssr', '/static']) {
+          const jsFiles = buildManifest.pages[page]
+          for (const file of jsFiles) {
+            const jsPath = path.join(next.testDir, '.next', file)
+            expect(await sourceMapExistsForFile(jsPath)).toBe(
+              productionBrowserSourceMaps
+            )
+          }
+        }
+      })
+    }
+  )
+})