feat(turbopack): add support for esm externals in app dir

vercel · Jun 19, 2024 · 83e1650 · 83e1650
1 parent 2a88a09
commit 83e1650
Show file tree

Hide file tree

Showing 5 changed files with 71 additions and 73 deletions.
diff --git a/packages/next-swc/crates/next-core/src/next_manifests/client_reference_manifest.rs b/packages/next-swc/crates/next-core/src/next_manifests/client_reference_manifest.rs
@@ -5,7 +5,10 @@ use turbo_tasks_fs::{File, FileSystemPath};
 use turbopack_binding::turbopack::{
     core::{
         asset::AssetContent,
-        chunk::{ChunkItemExt, ChunkableModule, ChunkingContext, ModuleId as TurbopackModuleId},
+        chunk::{
+            ChunkItem, ChunkItemExt, ChunkableModule, ChunkingContext,
+            ModuleId as TurbopackModuleId,
+        },
         output::OutputAsset,
         virtual_output::VirtualOutputAsset,
     },
@@ -66,11 +69,11 @@ impl ClientReferenceManifest {
                     .to_string()
                     .await?;
 
-                let client_module_id = ecmascript_client_reference
+                let client_chunk_item = ecmascript_client_reference
                     .client_module
-                    .as_chunk_item(Vc::upcast(client_chunking_context))
-                    .id()
-                    .await?;
+                    .as_chunk_item(Vc::upcast(client_chunking_context));
+
+                let client_module_id = client_chunk_item.id().await?;
 
                 let client_chunks_paths = if let Some(client_chunks) = client_references_chunks
                     .client_component_client_chunks
@@ -101,17 +104,16 @@ impl ClientReferenceManifest {
                         name: "*".into(),
                         id: (&*client_module_id).into(),
                         chunks: client_chunks_paths,
-                        // TODO(WEB-434)
-                        r#async: false,
+                        r#async: *client_chunk_item.is_self_async().await?,
                     },
                 );
 
                 if let Some(ssr_chunking_context) = ssr_chunking_context {
-                    let ssr_module_id = ecmascript_client_reference
+                    let ssr_chunk_item = ecmascript_client_reference
                         .ssr_module
-                        .as_chunk_item(Vc::upcast(ssr_chunking_context))
-                        .id()
-                        .await?;
+                        .as_chunk_item(Vc::upcast(ssr_chunking_context));
+
+                    let ssr_module_id = ssr_chunk_item.id().await?;
 
                     let ssr_chunks_paths = if runtime == NextRuntime::Edge {
                         // the chunks get added to the middleware-manifest.json instead
@@ -140,15 +142,15 @@ impl ClientReferenceManifest {
                     } else {
                         Vec::new()
                     };
+
                     let mut ssr_manifest_node = ManifestNode::default();
                     ssr_manifest_node.module_exports.insert(
                         "*".into(),
                         ManifestNodeEntry {
                             name: "*".into(),
                             id: (&*ssr_module_id).into(),
                             chunks: ssr_chunks_paths,
-                            // TODO(WEB-434)
-                            r#async: false,
+                            r#async: *ssr_chunk_item.is_self_async().await?,
                         },
                     );
 

diff --git a/packages/next-swc/crates/next-core/src/next_server/context.rs b/packages/next-swc/crates/next-core/src/next_server/context.rs
@@ -178,9 +178,9 @@ pub async fn get_server_resolve_options_context(
         project_path,
         project_path.root(),
         ExternalPredicate::Only(Vc::cell(external_packages)).cell(),
-        // TODO(sokra) esmExternals support
-        false,
+        *next_config.import_externals().await?,
     );
+
     let ty = ty.into_value();
 
     let mut custom_conditions = vec![mode.await?.condition().to_string().into()];

diff --git a/test/e2e/app-dir/app-external/app-external.test.ts b/test/e2e/app-dir/app-external/app-external.test.ts
@@ -16,7 +16,7 @@ async function resolveStreamResponse(response: any, onData?: any) {
 }
 
 describe('app dir - external dependency', () => {
-  const { next, skipped, isTurbopack } = nextTestSetup({
+  const { next, skipped } = nextTestSetup({
     files: __dirname,
     dependencies: {
       swr: 'latest',
@@ -286,10 +286,7 @@ describe('app dir - external dependency', () => {
       browser.elementByCss('#dual-pkg-outout button').click()
       await check(async () => {
         const text = await browser.elementByCss('#dual-pkg-outout p').text()
-        // TODO: enable esm externals for app router in turbopack for actions
-        expect(text).toBe(
-          isTurbopack ? 'dual-pkg-optout:cjs' : 'dual-pkg-optout:mjs'
-        )
+        expect(text).toBe('dual-pkg-optout:mjs')
         return 'success'
       }, /success/)
     })

diff --git a/test/e2e/esm-externals/esm-externals.test.ts b/test/e2e/esm-externals/esm-externals.test.ts
@@ -8,65 +8,62 @@ describe('esm-externals', () => {
   const { next, isTurbopack } = nextTestSetup({
     files: __dirname,
   })
+
   // Pages
-  {
-    const urls = ['/static', '/ssr', '/ssg']
+  describe.each(['/static', '/ssr', '/ssg'])('pages url %s', (url) => {
+    // For invalid esm packages that have "import" pointing to a non-esm-flagged module
+    // webpack is using the CJS version instead, but Turbopack is opting out of
+    // externalizing and bundling the non-esm-flagged module.
+    const expectedHtml = isTurbopack
+      ? 'Hello World+World+World+World+World+World'
+      : url === '/static'
+        ? 'Hello World+World+Alternative+World+World+World'
+        : 'Hello World+World+Alternative+World+World+Alternative'
 
-    for (const url of urls) {
-      // For invalid esm packages that have "import" pointing to a non-esm-flagged module
-      // webpack is using the CJS version instead but Turbopack is opting out of
-      // externalizing and bundling the non-esm-flagged module.
-      const expectedHtml = isTurbopack
-        ? /Hello World\+World\+World\+World\+World\+World/
-        : url === '/static'
-          ? /Hello World\+World\+Alternative\+World\+World\+World/
-          : /Hello World\+World\+Alternative\+World\+World\+Alternative/
-      // On client side, webpack always bundlings so it uses the non-esm-flagged module too.
-      const expectedText =
-        isTurbopack || url === '/static'
-          ? /Hello World\+World\+World\+World\+World\+World/
-          : /Hello World\+World\+World\+World\+World\+Alternative/
+    // On the client side, webpack always bundles, so it uses the non-esm-flagged module too.
+    const expectedText =
+      isTurbopack || url === '/static'
+        ? 'Hello World+World+World+World+World+World'
+        : 'Hello World+World+World+World+World+Alternative'
 
-      it(`should return the correct SSR HTML for ${url}`, async () => {
-        const res = await next.fetch(url)
-        const html = await res.text()
-        expect(normalize(html)).toMatch(expectedHtml)
-      })
+    it('should return the correct SSR HTML', async () => {
+      const $ = await next.render$(url)
+      const body = $('body > div > div').html()
+      expect(normalize(body)).toEqual(expectedHtml)
+    })
 
-      it(`should render the correct page for ${url}`, async () => {
-        const browser = await next.browser(url)
-        expect(await browser.elementByCss('body').text()).toMatch(expectedText)
-      })
-    }
-  }
+    it('should render the correct page', async () => {
+      const browser = await next.browser(url)
+      expect(await browser.elementByCss('body > div').text()).toEqual(
+        expectedText
+      )
+    })
+  })
 
   // App dir
-  {
-    // TODO App Dir doesn't use esmExternals: true correctly for Turbopack
-    // so we only verify that the page doesn't crash, but ignore the actual content
-    // const expectedHtml = isTurbopack ? /Hello World\+World\+World/ : /Hello World\+World\+Alternative/
+  describe.each(['/server', '/client'])('app dir url %s', (url) => {
     const expectedHtml = isTurbopack
-      ? /Hello Wrong\+Wrong\+Alternative/
-      : /Hello World\+World\+Alternative/
-    const urls = ['/server', '/client']
+      ? 'Hello World+World+World'
+      : 'Hello World+World+Alternative'
 
-    for (const url of urls) {
-      const expectedText =
-        url !== '/server'
-          ? /Hello World\+World\+World/
-          : isTurbopack
-            ? /Hello Wrong\+Wrong\+Alternative/
-            : /Hello World\+World\+Alternative/
-      it(`should return the correct SSR HTML for ${url}`, async () => {
-        const res = await next.fetch(url)
-        const html = await res.text()
-        expect(normalize(html)).toMatch(expectedHtml)
-      })
+    const expectedText =
+      url !== '/server'
+        ? 'Hello World+World+World'
+        : isTurbopack
+          ? 'Hello World+World+World'
+          : 'Hello World+World+Alternative'
 
-      it(`should render the correct page for ${url}`, async () => {
-        const browser = await next.browser(url)
-        expect(await browser.elementByCss('body').text()).toMatch(expectedText)
-      })
-    }
-  }
+    it('should return the correct SSR HTML', async () => {
+      const $ = await next.render$(url)
+      const body = $('body > div').html()
+      expect(normalize(body)).toEqual(expectedHtml)
+    })
+
+    it('should render the correct page', async () => {
+      const browser = await next.browser(url)
+      expect(await browser.elementByCss('body > div').text()).toEqual(
+        expectedText
+      )
+    })
+  })
 })
diff --git a/test/turbopack-dev-tests-manifest.json b/test/turbopack-dev-tests-manifest.json
@@ -6118,7 +6118,9 @@
       "esm-externals should return the correct SSR HTML for /ssr",
       "esm-externals should return the correct SSR HTML for /static"
     ],
-    "failed": [],
+    "failed": [
+      "esm-externals app url /client should return the correct SSR HTML"
+    ],
     "pending": [],
     "flakey": [],
     "runtimeError": false