From 83e16501c9d9f7b1a256b8ff8d987c2b32d79f7f Mon Sep 17 00:00:00 2001 From: hrmny <8845940+ForsakenHarmony@users.noreply.github.com> Date: Tue, 23 Apr 2024 15:17:22 +0200 Subject: [PATCH] feat(turbopack): add support for esm externals in app dir --- .../client_reference_manifest.rs | 28 ++--- .../next-core/src/next_server/context.rs | 4 +- .../app-dir/app-external/app-external.test.ts | 7 +- test/e2e/esm-externals/esm-externals.test.ts | 101 +++++++++--------- test/turbopack-dev-tests-manifest.json | 4 +- 5 files changed, 71 insertions(+), 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 index 082f35cba9808..764810c755a12 100644 --- 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,6 +142,7 @@ impl ClientReferenceManifest { } else { Vec::new() }; + let mut ssr_manifest_node = ManifestNode::default(); ssr_manifest_node.module_exports.insert( "*".into(), @@ -147,8 +150,7 @@ impl ClientReferenceManifest { 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 index 9022efc48df0a..dc2e766eb89bf 100644 --- 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 index 61a398d84bad5..d7ad6d5aec7a6 100644 --- 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 index be5b8340b137b..f90e112b0e3b1 100644 --- 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 index b315c9b1739ee..d3672de23cce8 100644 --- 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