From 6c4187c88490e2e335f889f629d5a318edd3606f Mon Sep 17 00:00:00 2001 From: Tobias Koppers Date: Wed, 25 Sep 2024 16:39:35 +0200 Subject: [PATCH] [Turbopack] fix rsc chunking optimization (#70461) ### What? * While looking for server component entries and client components, look for server utils too * Create a separate chunk group for server utils * Mark all user imports in the rsc entrypoint has server component entries * Fix order of imports to allow proper caching --------- Co-authored-by: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> --- crates/next-api/src/app.rs | 67 ++++++++++++---- crates/next-core/src/app_page_loader_tree.rs | 80 ++++++++----------- crates/next-core/src/base_loader_tree.rs | 12 ++- .../next-core/src/next_app/app_page_entry.rs | 2 +- .../visit_client_reference.rs | 69 ++++++++++++++-- .../app-edge-root-layout/index.test.ts | 2 +- .../crates/turbopack-core/src/server_fs.rs | 4 +- 7 files changed, 158 insertions(+), 78 deletions(-) diff --git a/crates/next-api/src/app.rs b/crates/next-api/src/app.rs index fb2e275b834f8e..d86f1d59bf8411 100644 --- a/crates/next-api/src/app.rs +++ b/crates/next-api/src/app.rs @@ -11,7 +11,8 @@ use next_core::{ next_app::{ app_client_references_chunks::get_app_server_reference_modules, get_app_client_references_chunks, get_app_client_shared_chunk_group, get_app_page_entry, - get_app_route_entry, metadata::route::get_app_metadata_route_entry, AppEntry, AppPage, + get_app_route_entry, include_modules_module::IncludeModulesModule, + metadata::route::get_app_metadata_route_entry, AppEntry, AppPage, }, next_client::{ get_client_module_options_context, get_client_resolve_options_context, @@ -36,7 +37,9 @@ use next_core::{ }; use serde::{Deserialize, Serialize}; use tracing::Instrument; -use turbo_tasks::{trace::TraceRawVcs, Completion, RcStr, TryJoinIterExt, Value, Vc}; +use turbo_tasks::{ + trace::TraceRawVcs, Completion, RcStr, TryJoinIterExt, Value, ValueToString, Vc, +}; use turbo_tasks_env::{CustomProcessEnv, ProcessEnv}; use turbo_tasks_fs::{File, FileContent, FileSystemPath}; use turbopack::{ @@ -686,6 +689,11 @@ fn client_shared_chunks() -> Vc { Vc::cell("client_shared_chunks".into()) } +#[turbo_tasks::function] +fn server_utils_module() -> Vc { + Vc::cell("server-utils".into()) +} + #[derive(Copy, Clone, Serialize, Deserialize, PartialEq, Eq, Debug, TraceRawVcs)] enum AppPageEndpointType { Html, @@ -1256,31 +1264,62 @@ impl AppEndpoint { let mut current_chunks = OutputAssets::empty(); let mut current_availability_info = AvailabilityInfo::Root; if let Some(client_references) = client_references { - for server_component in client_references - .await? - .server_component_entries - .iter() - .copied() - { - let server_path = server_component.server_path(); - let is_layout = - server_path.file_stem().await?.as_deref() == Some("layout"); + let client_references = client_references.await?; + let span = tracing::trace_span!("server utils",); + async { + let utils_module = IncludeModulesModule::new( + AssetIdent::from_path(this.app_project.project().project_path()) + .with_modifier(server_utils_module()), + client_references.server_utils.clone(), + ); let chunk_group = chunking_context .chunk_group( - server_component.ident(), - Vc::upcast(server_component), + utils_module.ident(), + Vc::upcast(utils_module), Value::new(current_availability_info), ) .await?; - if is_layout { + current_chunks = current_chunks + .concatenate(chunk_group.assets) + .resolve() + .await?; + current_availability_info = chunk_group.availability_info; + + anyhow::Ok(()) + } + .instrument(span) + .await?; + for server_component in client_references + .server_component_entries + .iter() + .copied() + .take(client_references.server_component_entries.len() - 1) + { + let span = tracing::trace_span!( + "layout segment", + name = server_component.ident().to_string().await?.as_str() + ); + async { + let chunk_group = chunking_context + .chunk_group( + server_component.ident(), + Vc::upcast(server_component), + Value::new(current_availability_info), + ) + .await?; + current_chunks = current_chunks .concatenate(chunk_group.assets) .resolve() .await?; current_availability_info = chunk_group.availability_info; + + anyhow::Ok(()) } + .instrument(span) + .await?; } } chunking_context diff --git a/crates/next-core/src/app_page_loader_tree.rs b/crates/next-core/src/app_page_loader_tree.rs index 0eef5e8fba7063..b78bbe5113f82d 100644 --- a/crates/next-core/src/app_page_loader_tree.rs +++ b/crates/next-core/src/app_page_loader_tree.rs @@ -5,15 +5,10 @@ use std::{ use anyhow::Result; use indexmap::IndexMap; -use turbo_tasks::{RcStr, Value, Vc}; +use turbo_tasks::{RcStr, Vc}; use turbo_tasks_fs::FileSystemPath; use turbopack::{transition::Transition, ModuleAssetContext}; -use turbopack_core::{ - context::AssetContext, - file_source::FileSource, - module::Module, - reference_type::{EcmaScriptModulesReferenceSubType, InnerAssets, ReferenceType}, -}; +use turbopack_core::{file_source::FileSource, module::Module}; use turbopack_ecmascript::{magic_identifier, text::TextContentFileSource, utils::StringifyJs}; use crate::{ @@ -193,16 +188,7 @@ impl AppPageLoaderTreeBuilder { app_page.clone(), ); - let module = self - .base - .module_asset_context - .process( - source, - Value::new(ReferenceType::EcmaScriptModules( - EcmaScriptModulesReferenceSubType::Undefined, - )), - ) - .module(); + let module = self.base.process_source(source); self.base .inner_assets .insert(inner_module_id.into(), module); @@ -237,14 +223,15 @@ impl AppPageLoaderTreeBuilder { self.base .imports .push(format!("import {identifier} from \"{inner_module_id}\";").into()); - self.base.inner_assets.insert( - inner_module_id.into(), - Vc::upcast(StructuredImageModuleType::create_module( - Vc::upcast(FileSource::new(path)), - BlurPlaceholderMode::None, - self.base.module_asset_context, - )), - ); + let module = Vc::upcast(StructuredImageModuleType::create_module( + Vc::upcast(FileSource::new(path)), + BlurPlaceholderMode::None, + self.base.module_asset_context, + )); + let module = self.base.process_module(module); + self.base + .inner_assets + .insert(inner_module_id.into(), module); let s = " "; writeln!(self.loader_tree_code, "{s}(async (props) => [{{")?; @@ -286,14 +273,9 @@ impl AppPageLoaderTreeBuilder { let module = self .base - .module_asset_context - .process( - Vc::upcast(TextContentFileSource::new(Vc::upcast(FileSource::new( - alt_path, - )))), - Value::new(ReferenceType::Internal(InnerAssets::empty())), - ) - .module(); + .process_source(Vc::upcast(TextContentFileSource::new(Vc::upcast( + FileSource::new(alt_path), + )))); self.base .inner_assets @@ -339,12 +321,18 @@ impl AppPageLoaderTreeBuilder { route: _, } = &modules; + // Ensure global metadata being written only once at the root level + // Otherwise child pages will have redundant metadata + let global_metadata = &*global_metadata.await?; + self.write_metadata( + app_page, + metadata, + if root { Some(global_metadata) } else { None }, + ) + .await?; + self.write_modules_entry(AppDirModuleType::Layout, *layout) .await?; - self.write_modules_entry(AppDirModuleType::Page, *page) - .await?; - self.write_modules_entry(AppDirModuleType::DefaultPage, *default) - .await?; self.write_modules_entry(AppDirModuleType::Error, *error) .await?; self.write_modules_entry(AppDirModuleType::Loading, *loading) @@ -353,6 +341,10 @@ impl AppPageLoaderTreeBuilder { .await?; self.write_modules_entry(AppDirModuleType::NotFound, *not_found) .await?; + self.write_modules_entry(AppDirModuleType::Page, *page) + .await?; + self.write_modules_entry(AppDirModuleType::DefaultPage, *default) + .await?; let modules_code = replace(&mut self.loader_tree_code, temp_loader_tree_code); @@ -366,16 +358,6 @@ impl AppPageLoaderTreeBuilder { self.loader_tree_code += &modules_code; - // Ensure global metadata being written only once at the root level - // Otherwise child pages will have redundant metadata - let global_metadata = &*global_metadata.await?; - self.write_metadata( - app_page, - metadata, - if root { Some(global_metadata) } else { None }, - ) - .await?; - write!(self.loader_tree_code, "}}]")?; Ok(()) } @@ -388,7 +370,9 @@ impl AppPageLoaderTreeBuilder { let modules = &loader_tree.modules; if let Some(global_error) = modules.global_error { - let module = self.base.process_module(global_error); + let module = self + .base + .process_source(Vc::upcast(FileSource::new(global_error))); self.base.inner_assets.insert(GLOBAL_ERROR.into(), module); }; diff --git a/crates/next-core/src/base_loader_tree.rs b/crates/next-core/src/base_loader_tree.rs index 2292c8ef7b3b38..152ddd07bde1db 100644 --- a/crates/next-core/src/base_loader_tree.rs +++ b/crates/next-core/src/base_loader_tree.rs @@ -8,6 +8,7 @@ use turbopack_core::{ file_source::FileSource, module::Module, reference_type::{EcmaScriptModulesReferenceSubType, ReferenceType}, + source::Source, }; use turbopack_ecmascript::{magic_identifier, utils::StringifyJs}; @@ -64,9 +65,7 @@ impl BaseLoaderTreeBuilder { i } - pub fn process_module(&self, path: Vc) -> Vc> { - let source = Vc::upcast(FileSource::new(path)); - + pub fn process_source(&self, source: Vc>) -> Vc> { let reference_type = Value::new(ReferenceType::EcmaScriptModules( EcmaScriptModulesReferenceSubType::Undefined, )); @@ -76,6 +75,11 @@ impl BaseLoaderTreeBuilder { .module() } + pub fn process_module(&self, module: Vc>) -> Vc> { + self.server_component_transition + .process_module(module, self.module_asset_context) + } + pub async fn create_module_tuple_code( &mut self, module_type: AppDirModuleType, @@ -96,7 +100,7 @@ impl BaseLoaderTreeBuilder { .into(), ); - let module = self.process_module(path); + let module = self.process_source(Vc::upcast(FileSource::new(path))); self.inner_assets .insert(format!("MODULE_{i}").into(), module); diff --git a/crates/next-core/src/next_app/app_page_entry.rs b/crates/next-core/src/next_app/app_page_entry.rs index 6ecac241094fea..c1f1bda123a256 100644 --- a/crates/next-core/src/next_app/app_page_entry.rs +++ b/crates/next-core/src/next_app/app_page_entry.rs @@ -107,7 +107,7 @@ pub async fn get_app_page_entry( let source = VirtualSource::new_with_ident( source .ident() - .with_query(Vc::cell(query.to_string().into())), + .with_query(Vc::cell(format!("?{}", query).into())), AssetContent::file(file.into()), ); diff --git a/crates/next-core/src/next_client_reference/visit_client_reference.rs b/crates/next-core/src/next_client_reference/visit_client_reference.rs index 10207b384b1c7a..d74d07db67441e 100644 --- a/crates/next-core/src/next_client_reference/visit_client_reference.rs +++ b/crates/next-core/src/next_client_reference/visit_client_reference.rs @@ -10,6 +10,7 @@ use turbo_tasks::{ trace::TraceRawVcs, RcStr, ReadRef, TryJoinIterExt, ValueToString, Vc, }; +use turbo_tasks_fs::FileSystemPath; use turbopack::css::CssModuleAsset; use turbopack_core::{ module::{Module, Modules}, @@ -50,6 +51,7 @@ pub enum ClientReferenceType { pub struct ClientReferenceGraphResult { pub client_references: Vec, pub server_component_entries: Vec>, + pub server_utils: Vec>>, } #[turbo_tasks::value(transparent)] @@ -77,6 +79,7 @@ pub async fn client_reference_graph( let mut client_references = vec![]; let mut server_component_entries = vec![]; + let mut server_utils = vec![]; let graph = AdjacencyMap::new() .skip_duplicates() @@ -86,7 +89,9 @@ pub async fn client_reference_graph( .copied() .map(|module| async move { Ok(VisitClientReferenceNode { - server_component: None, + state: VisitClientReferenceNodeState::Entry { + entry_path: module.ident().path().resolve().await?, + }, ty: VisitClientReferenceNodeType::Internal( module, module.ident().to_string().await?, @@ -111,6 +116,9 @@ pub async fn client_reference_graph( VisitClientReferenceNodeType::ClientReference(client_reference, _) => { client_references.push(*client_reference); } + VisitClientReferenceNodeType::ServerUtilEntry(server_util, _) => { + server_utils.push(*server_util); + } VisitClientReferenceNodeType::ServerComponentEntry(server_component, _) => { server_component_entries.push(*server_component); } @@ -120,6 +128,7 @@ pub async fn client_reference_graph( Ok(ClientReferenceGraphResult { client_references, server_component_entries, + server_utils, } .cell()) } @@ -133,16 +142,41 @@ struct VisitClientReference; Clone, Eq, PartialEq, Hash, Serialize, Deserialize, Debug, ValueDebugFormat, TraceRawVcs, )] struct VisitClientReferenceNode { - server_component: Option>, + state: VisitClientReferenceNodeState, ty: VisitClientReferenceNodeType, } +#[derive( + Clone, Copy, Eq, PartialEq, Hash, Serialize, Deserialize, Debug, ValueDebugFormat, TraceRawVcs, +)] +enum VisitClientReferenceNodeState { + Entry { + entry_path: Vc, + }, + InServerComponent { + server_component: Vc, + }, + InServerUtil, +} +impl VisitClientReferenceNodeState { + fn server_component(&self) -> Option> { + match self { + VisitClientReferenceNodeState::Entry { .. } => None, + VisitClientReferenceNodeState::InServerComponent { server_component } => { + Some(*server_component) + } + VisitClientReferenceNodeState::InServerUtil => None, + } + } +} + #[derive( Clone, Eq, PartialEq, Hash, Serialize, Deserialize, Debug, ValueDebugFormat, TraceRawVcs, )] enum VisitClientReferenceNodeType { ClientReference(ClientReference, ReadRef), ServerComponentEntry(Vc, ReadRef), + ServerUtilEntry(Vc>, ReadRef), Internal(Vc>, ReadRef), } @@ -155,6 +189,7 @@ impl Visit for VisitClientReference { match edge.ty { VisitClientReferenceNodeType::ClientReference(..) => VisitControlFlow::Skip(edge), VisitClientReferenceNodeType::Internal(..) => VisitControlFlow::Continue(edge), + VisitClientReferenceNodeType::ServerUtilEntry(..) => VisitControlFlow::Continue(edge), VisitClientReferenceNodeType::ServerComponentEntry(..) => { VisitControlFlow::Continue(edge) } @@ -169,6 +204,7 @@ impl Visit for VisitClientReference { // nodes' edges. VisitClientReferenceNodeType::ClientReference(..) => return Ok(vec![]), VisitClientReferenceNodeType::Internal(module, _) => module, + VisitClientReferenceNodeType::ServerUtilEntry(module, _) => module, VisitClientReferenceNodeType::ServerComponentEntry(module, _) => Vc::upcast(module), }; @@ -180,10 +216,10 @@ impl Visit for VisitClientReference { Vc::try_resolve_downcast_type::(module).await? { return Ok(VisitClientReferenceNode { - server_component: node.server_component, + state: node.state, ty: VisitClientReferenceNodeType::ClientReference( ClientReference { - server_component: node.server_component, + server_component: node.state.server_component(), ty: ClientReferenceType::EcmascriptClientReference( client_reference_module, ), @@ -197,10 +233,10 @@ impl Visit for VisitClientReference { Vc::try_resolve_downcast_type::(module).await? { return Ok(VisitClientReferenceNode { - server_component: node.server_component, + state: node.state, ty: VisitClientReferenceNodeType::ClientReference( ClientReference { - server_component: node.server_component, + server_component: node.state.server_component(), ty: ClientReferenceType::CssClientReference( css_client_reference_asset, ), @@ -214,7 +250,9 @@ impl Visit for VisitClientReference { Vc::try_resolve_downcast_type::(module).await? { return Ok(VisitClientReferenceNode { - server_component: Some(server_component_asset), + state: VisitClientReferenceNodeState::InServerComponent { + server_component: server_component_asset, + }, ty: VisitClientReferenceNodeType::ServerComponentEntry( server_component_asset, server_component_asset.ident().to_string().await?, @@ -222,8 +260,20 @@ impl Visit for VisitClientReference { }); } + if let VisitClientReferenceNodeState::Entry { entry_path } = &node.state { + if module.ident().path().resolve().await? != *entry_path { + return Ok(VisitClientReferenceNode { + state: VisitClientReferenceNodeState::InServerUtil, + ty: VisitClientReferenceNodeType::ServerUtilEntry( + module, + module.ident().to_string().await?, + ), + }); + } + } + Ok(VisitClientReferenceNode { - server_component: node.server_component, + state: node.state, ty: VisitClientReferenceNodeType::Internal( module, module.ident().to_string().await?, @@ -245,6 +295,9 @@ impl Visit for VisitClientReference { VisitClientReferenceNodeType::Internal(_, name) => { tracing::info_span!("module", name = name.to_string()) } + VisitClientReferenceNodeType::ServerUtilEntry(_, name) => { + tracing::info_span!("server util", name = name.to_string()) + } VisitClientReferenceNodeType::ServerComponentEntry(_, name) => { tracing::info_span!("layout segment", name = name.to_string()) } diff --git a/test/e2e/app-dir/app-edge-root-layout/index.test.ts b/test/e2e/app-dir/app-edge-root-layout/index.test.ts index f4eaf7187ee5e2..ce0fec9db5766c 100644 --- a/test/e2e/app-dir/app-edge-root-layout/index.test.ts +++ b/test/e2e/app-dir/app-edge-root-layout/index.test.ts @@ -25,7 +25,7 @@ describe('app-dir edge runtime root layout', () => { const middlewareManifest = await next.readFile( '.next/server/middleware-manifest.json' ) - expect(middlewareManifest).not.toContain('favicon') + expect(middlewareManifest).not.toContain('/favicon') }) } }) diff --git a/turbopack/crates/turbopack-core/src/server_fs.rs b/turbopack/crates/turbopack-core/src/server_fs.rs index 0c8ac7a988cc1c..0cae12d1e2fdef 100644 --- a/turbopack/crates/turbopack-core/src/server_fs.rs +++ b/turbopack/crates/turbopack-core/src/server_fs.rs @@ -52,12 +52,12 @@ impl FileSystem for ServerFileSystem { _fs_path: Vc, _target: Vc, ) -> Result> { - bail!("Writing is not possible to the marker filesystem for the server") + bail!("Writing is not possible to the marker filesystem for the server") } #[turbo_tasks::function] fn metadata(&self, _fs_path: Vc) -> Result> { - bail!("Reading is not possible from the marker filesystem for the server") + bail!("Reading is not possible from the marker filesystem for the server") } }