From 2c80812eff493eff2867baf46870b4fcb8df2ff4 Mon Sep 17 00:00:00 2001 From: Will Binns-Smith Date: Fri, 26 Apr 2024 15:26:33 -0700 Subject: [PATCH] Turbopack: Allow client components from foreign code in app routes (#64751) This extends #64520 to cover cases where client components originate from node_modules (foreign code). Test Plan: Extended the integration test to cover this Closes #64412 Fixes PACK-3014 --------- Co-authored-by: OJ Kwon <1210596+kwonoj@users.noreply.github.com> --- .../crates/next-core/src/next_edge/context.rs | 11 +---- .../crates/next-core/src/next_import_map.rs | 2 +- .../next-core/src/next_server/context.rs | 41 +++++++++++-------- .../app-routes-client-component.test.ts | 1 + .../app/runtime/route.ts | 2 + .../my-module/MyModuleClientComponent.tsx | 5 +++ 6 files changed, 35 insertions(+), 27 deletions(-) create mode 100644 test/e2e/app-dir/app-routes-client-component/node_modules/my-module/MyModuleClientComponent.tsx diff --git a/packages/next-swc/crates/next-core/src/next_edge/context.rs b/packages/next-swc/crates/next-core/src/next_edge/context.rs index 3022e54aba549..a3b11a95add37 100644 --- a/packages/next-swc/crates/next-core/src/next_edge/context.rs +++ b/packages/next-swc/crates/next-core/src/next_edge/context.rs @@ -108,15 +108,8 @@ pub async fn get_edge_resolve_options_context( .map(ToString::to_string), ); - match ty { - ServerContextType::AppRSC { .. } => custom_conditions.push("react-server".to_string()), - ServerContextType::AppRoute { .. } - | ServerContextType::Pages { .. } - | ServerContextType::PagesData { .. } - | ServerContextType::PagesApi { .. } - | ServerContextType::AppSSR { .. } - | ServerContextType::Middleware { .. } - | ServerContextType::Instrumentation { .. } => {} + if ty.supports_react_server() { + custom_conditions.push("react-server".to_string()); }; let resolve_options_context = ResolveOptionsContext { diff --git a/packages/next-swc/crates/next-core/src/next_import_map.rs b/packages/next-swc/crates/next-core/src/next_import_map.rs index 1be71bf8e7cc8..8116929c721b9 100644 --- a/packages/next-swc/crates/next-core/src/next_import_map.rs +++ b/packages/next-swc/crates/next-core/src/next_import_map.rs @@ -728,7 +728,7 @@ async fn rsc_aliases( } if runtime == NextRuntime::Edge { - if matches!(ty, ServerContextType::AppRSC { .. }) { + if ty.supports_react_server() { alias["react"] = format!("next/dist/compiled/react{react_channel}/react.react-server"); alias["react-dom"] = format!("next/dist/compiled/react-dom{react_channel}/react-dom.react-server"); 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 d4bd4703e92cf..04bf7b7af4d8a 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 @@ -99,6 +99,17 @@ pub enum ServerContextType { Instrumentation, } +impl ServerContextType { + pub fn supports_react_server(&self) -> bool { + matches!( + self, + ServerContextType::AppRSC { .. } + | ServerContextType::AppRoute { .. } + | ServerContextType::PagesApi { .. } + ) + } +} + #[turbo_tasks::function] pub async fn get_server_resolve_options_context( project_path: Vc, @@ -152,18 +163,10 @@ pub async fn get_server_resolve_options_context( .map(ToString::to_string), ); - match ty { - ServerContextType::AppRSC { .. } - | ServerContextType::AppRoute { .. } - | ServerContextType::PagesApi { .. } - | ServerContextType::Middleware { .. } => { - custom_conditions.push("react-server".to_string()) - } - ServerContextType::Pages { .. } - | ServerContextType::PagesData { .. } - | ServerContextType::AppSSR { .. } - | ServerContextType::Instrumentation { .. } => {} + if ty.supports_react_server() { + custom_conditions.push("react-server".to_string()); }; + let external_cjs_modules_plugin = ExternalCjsModulesResolvePlugin::new( project_path, project_path.root(), @@ -325,7 +328,7 @@ pub async fn get_server_module_options_context( let mut foreign_next_server_rules = get_next_server_transforms_rules(next_config, ty.into_value(), mode, true, next_runtime) .await?; - let internal_custom_rules = + let mut internal_custom_rules = get_next_server_internal_transforms_rules(ty.into_value(), *next_config.mdx_rs().await?) .await?; @@ -622,10 +625,16 @@ pub async fn get_server_module_options_context( ecmascript_client_reference_transition_name, } => { next_server_rules.extend(source_transform_rules); + + let mut common_next_server_rules = vec![ + get_next_react_server_components_transform_rule(next_config, true, Some(app_dir)) + .await?, + ]; + if let Some(ecmascript_client_reference_transition_name) = ecmascript_client_reference_transition_name { - next_server_rules.push(get_ecma_transform_rule( + common_next_server_rules.push(get_ecma_transform_rule( Box::new(ClientDirectiveTransformer::new( ecmascript_client_reference_transition_name, )), @@ -634,10 +643,8 @@ pub async fn get_server_module_options_context( )); } - next_server_rules.push( - get_next_react_server_components_transform_rule(next_config, true, Some(app_dir)) - .await?, - ); + next_server_rules.extend(common_next_server_rules.iter().cloned()); + internal_custom_rules.extend(common_next_server_rules); let module_options_context = ModuleOptionsContext { esm_url_rewrite_behavior: Some(UrlRewriteBehavior::Full), diff --git a/test/e2e/app-dir/app-routes-client-component/app-routes-client-component.test.ts b/test/e2e/app-dir/app-routes-client-component/app-routes-client-component.test.ts index 96c9d2e740826..b0a5079ac0a64 100644 --- a/test/e2e/app-dir/app-routes-client-component/app-routes-client-component.test.ts +++ b/test/e2e/app-dir/app-routes-client-component/app-routes-client-component.test.ts @@ -8,6 +8,7 @@ describe('referencing a client component in an app route', () => { it('responds without error', async () => { expect(JSON.parse(await next.render('/runtime'))).toEqual({ clientComponent: 'function', + myModuleClientComponent: 'function', }) }) }) diff --git a/test/e2e/app-dir/app-routes-client-component/app/runtime/route.ts b/test/e2e/app-dir/app-routes-client-component/app/runtime/route.ts index e4ce5094e904d..fd7505a005999 100644 --- a/test/e2e/app-dir/app-routes-client-component/app/runtime/route.ts +++ b/test/e2e/app-dir/app-routes-client-component/app/runtime/route.ts @@ -1,8 +1,10 @@ import { NextResponse } from 'next/server' import { ClientComponent } from '../../ClientComponent' +import { MyModuleClientComponent } from 'my-module/MyModuleClientComponent' export function GET() { return NextResponse.json({ clientComponent: typeof ClientComponent, + myModuleClientComponent: typeof MyModuleClientComponent, }) } diff --git a/test/e2e/app-dir/app-routes-client-component/node_modules/my-module/MyModuleClientComponent.tsx b/test/e2e/app-dir/app-routes-client-component/node_modules/my-module/MyModuleClientComponent.tsx new file mode 100644 index 0000000000000..c6cd8470693c8 --- /dev/null +++ b/test/e2e/app-dir/app-routes-client-component/node_modules/my-module/MyModuleClientComponent.tsx @@ -0,0 +1,5 @@ +'use client' + +export function MyModuleClientComponent() { + return
MyModuleClientComponent
+}