backport: force module format for virtual client-proxy (#74162) (#74608)

Fixes #74062 (`jotai` ran into this error [when they added `"type": "commonjs"` to their package.json](pmndrs/jotai#2579 (reply in thread))) > this is a bug in the transform we do when a `'use client'` directive is encountered. I think what's happening is that we're creating a virtual file that uses ESM import/export syntax, but it's called proxy.js (not proxy.mjs), so the `"type": "commonjs" `makes turbopack "correctly" upset at the ESM syntax. #74062 (comment) The (slightly kludgy) solution is to use `proxy.mjs` or `proxy.cjs` to force the module format, bypassing the issue where `proxy.js` changes meaning depending on `package.json#type`.
vercel · Jan 7, 2025 · dcb208e · dcb208e
1 parent 6f6766a
commit dcb208e
Show file tree

Hide file tree

Showing 14 changed files with 131 additions and 1 deletion.
diff --git a/..._client_reference/ecmascript_client_reference/ecmascript_client_reference_proxy_module.rs b/..._client_reference/ecmascript_client_reference/ecmascript_client_reference_proxy_module.rs
@@ -70,11 +70,13 @@ impl EcmascriptClientReferenceProxyModule {
     #[turbo_tasks::function]
     async fn proxy_module(&self) -> Result<Vc<Box<dyn EcmascriptChunkPlaceable>>> {
         let mut code = CodeBuilder::default();
+        let is_esm: bool;
 
         let server_module_path = &*self.server_module_ident.to_string().await?;
 
         // Adapted from https://github.com/facebook/react/blob/c5b9375767e2c4102d7e5559d383523736f1c902/packages/react-server-dom-webpack/src/ReactFlightWebpackNodeLoader.js#L323-L354
         if let EcmascriptExports::EsmExports(exports) = &*self.client_module.get_exports().await? {
+            is_esm = true;
             let exports = exports.expand_exports().await?;
 
             if !exports.dynamic_exports.is_empty() {
@@ -130,6 +132,7 @@ impl EcmascriptClientReferenceProxyModule {
                 }
             }
         } else {
+            is_esm = false;
             writedoc!(
                 code,
                 r#"
@@ -146,7 +149,13 @@ impl EcmascriptClientReferenceProxyModule {
             AssetContent::file(File::from(code.source_code().clone()).into());
 
         let proxy_source = VirtualSource::new(
-            self.server_module_ident.path().join("proxy.js".into()),
+            self.server_module_ident.path().join(
+                // Depending on the original format, we call the file `proxy.mjs` or `proxy.cjs`.
+                // This is because we're placing the virtual module next to the original code, so
+                // its parsing will be affected by `type` fields in package.json --
+                // a bare `proxy.js` may end up being unexpectedly parsed as the wrong format.
+                format!("proxy.{}", if is_esm { "mjs" } else { "cjs" }).into(),
+            ),
             proxy_module_content,
         );
 

diff --git a/test/e2e/app-dir/client-module-with-package-type/app/import-cjs/page.tsx b/test/e2e/app-dir/client-module-with-package-type/app/import-cjs/page.tsx
@@ -0,0 +1,11 @@
+import * as React from 'react'
+
+import EsmFromCjs from 'lib-cjs'
+
+export default function Page() {
+  return (
+    <p>
+      lib-cjs: <EsmFromCjs />
+    </p>
+  )
+}
diff --git a/test/e2e/app-dir/client-module-with-package-type/app/import-esm/page.tsx b/test/e2e/app-dir/client-module-with-package-type/app/import-esm/page.tsx
@@ -0,0 +1,11 @@
+import * as React from 'react'
+
+import EsmFromEsm from 'lib-esm'
+
+export default function Page() {
+  return (
+    <p>
+      lib-esm: <EsmFromEsm />
+    </p>
+  )
+}
diff --git a/test/e2e/app-dir/client-module-with-package-type/app/layout.tsx b/test/e2e/app-dir/client-module-with-package-type/app/layout.tsx
@@ -0,0 +1,8 @@
+import { ReactNode } from 'react'
+export default function Root({ children }: { children: ReactNode }) {
+  return (
+    <html>
+      <body>{children}</body>
+    </html>
+  )
+}
diff --git a/test/e2e/app-dir/client-module-with-package-type/app/require-cjs/page.tsx b/test/e2e/app-dir/client-module-with-package-type/app/require-cjs/page.tsx
@@ -0,0 +1,11 @@
+import * as React from 'react'
+
+const CjsFromCjs = require('lib-cjs')
+
+export default function Page() {
+  return (
+    <p>
+      lib-cjs: <CjsFromCjs />
+    </p>
+  )
+}
diff --git a/test/e2e/app-dir/client-module-with-package-type/app/require-esm/page.tsx b/test/e2e/app-dir/client-module-with-package-type/app/require-esm/page.tsx
@@ -0,0 +1,11 @@
+import * as React from 'react'
+
+const CjsFromEsm = require('lib-esm')
+
+export default function Page() {
+  return (
+    <p>
+      lib-esm: <CjsFromEsm />
+    </p>
+  )
+}
diff --git a/test/e2e/app-dir/client-module-with-package-type/index.test.ts b/test/e2e/app-dir/client-module-with-package-type/index.test.ts
@@ -0,0 +1,31 @@
+import { nextTestSetup } from 'e2e-utils'
+
+describe('esm-client-module-without-exports', () => {
+  const { next } = nextTestSetup({
+    files: __dirname,
+  })
+
+  describe('"type": "commonjs" in package.json', () => {
+    it('should render without errors: import cjs', async () => {
+      const $ = await next.render$('/import-cjs')
+      expect($('p').text()).toContain('lib-cjs: esm')
+    })
+
+    it('should render without errors: require cjs', async () => {
+      const $ = await next.render$('/require-cjs')
+      expect($('p').text()).toContain('lib-cjs: cjs')
+    })
+  })
+
+  describe('"type": "module" in package.json', () => {
+    it('should render without errors: import esm', async () => {
+      const $ = await next.render$('/import-esm')
+      expect($('p').text()).toContain('lib-esm: esm')
+    })
+
+    it('should render without errors: require esm', async () => {
+      const $ = await next.render$('/require-esm')
+      expect($('p').text()).toContain('lib-esm: cjs')
+    })
+  })
+})
diff --git a/test/e2e/app-dir/client-module-with-package-type/next.config.js b/test/e2e/app-dir/client-module-with-package-type/next.config.js
@@ -0,0 +1,6 @@
+/**
+ * @type {import('next').NextConfig}
+ */
+const nextConfig = {}
+
+module.exports = nextConfig
diff --git a/test/e2e/app-dir/client-module-with-package-type/node_modules/lib-cjs/index.js b/test/e2e/app-dir/client-module-with-package-type/node_modules/lib-cjs/index.js
diff --git a/test/e2e/app-dir/client-module-with-package-type/node_modules/lib-cjs/index.mjs b/test/e2e/app-dir/client-module-with-package-type/node_modules/lib-cjs/index.mjs
diff --git a/test/e2e/app-dir/client-module-with-package-type/node_modules/lib-cjs/package.json b/test/e2e/app-dir/client-module-with-package-type/node_modules/lib-cjs/package.json
diff --git a/test/e2e/app-dir/client-module-with-package-type/node_modules/lib-esm/index.cjs b/test/e2e/app-dir/client-module-with-package-type/node_modules/lib-esm/index.cjs
diff --git a/test/e2e/app-dir/client-module-with-package-type/node_modules/lib-esm/index.js b/test/e2e/app-dir/client-module-with-package-type/node_modules/lib-esm/index.js
diff --git a/test/e2e/app-dir/client-module-with-package-type/node_modules/lib-esm/package.json b/test/e2e/app-dir/client-module-with-package-type/node_modules/lib-esm/package.json