From 161b5882d88b33526cdbdab2964abe3004f7d0cb Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Fri, 4 Oct 2024 13:13:45 +0200 Subject: [PATCH 1/4] fix --- crates/next-api/src/app.rs | 53 +++++++- .../src/next_client_reference/mod.rs | 2 +- .../visit_client_reference.rs | 119 ++++++++++++------ 3 files changed, 133 insertions(+), 41 deletions(-) diff --git a/crates/next-api/src/app.rs b/crates/next-api/src/app.rs index 5772561ae4be6..ecebae257a8fa 100644 --- a/crates/next-api/src/app.rs +++ b/crates/next-api/src/app.rs @@ -18,7 +18,9 @@ use next_core::{ get_client_module_options_context, get_client_resolve_options_context, get_client_runtime_entries, ClientContextType, RuntimeEntries, }, - next_client_reference::{client_reference_graph, NextEcmascriptClientReferenceTransition}, + next_client_reference::{ + client_reference_graph, ClientReferenceGraphResult, NextEcmascriptClientReferenceTransition, + }, next_config::NextConfig, next_dynamic::NextDynamicTransition, next_edge::route_regex::get_named_middleware_regex, @@ -62,7 +64,7 @@ use turbopack_core::{ source::Source, virtual_output::VirtualOutputAsset, }; -use turbopack_ecmascript::resolve::cjs_resolve; +use turbopack_ecmascript::{resolve::cjs_resolve, tree_shake::asset::EcmascriptModulePartAsset}; use crate::{ dynamic_imports::{ @@ -834,8 +836,6 @@ impl AppEndpoint { let rsc_entry = app_entry.rsc_entry; - let rsc_entry_asset = Vc::upcast(rsc_entry); - let client_chunking_context = this.app_project.project().client_chunking_context(); let (app_server_reference_modules, client_dynamic_imports, client_references) = @@ -862,7 +862,50 @@ impl AppEndpoint { } let client_shared_availability_info = client_shared_chunk_group.availability_info; - let client_references = client_reference_graph(Vc::cell(vec![rsc_entry_asset])); + let client_references = { + let modules = if let Some(module_part_asset) = + Vc::try_resolve_downcast_type::(rsc_entry) + .await? + { + let mut modules: Vec<_> = module_part_asset + .await? + .full_module + .await? + .inner_assets + .iter() + .try_join() // TODO prevent double collect + .await? + .iter() + .flat_map(|a| a.values()) + .copied() + .collect(); // TODO prevent double collect + modules.push(Vc::upcast(module_part_asset)); + modules + } else { + vec![rsc_entry] + }; + + println!( + "before {:?}", + modules + .iter() + .map(|module| module.ident().to_string()) + .try_join() + .await? + ); + + let mut client_references: ClientReferenceGraphResult = + ClientReferenceGraphResult::default(); + + for module in modules { + let current_client_references = + client_reference_graph(module, client_references.visited_nodes).await?; + + client_references.extend(¤t_client_references); + } + // TODO remove cell + client_references.cell() + }; let ssr_chunking_context = if process_ssr { Some(match runtime { diff --git a/crates/next-core/src/next_client_reference/mod.rs b/crates/next-core/src/next_client_reference/mod.rs index b349d8c16ebcb..69f19c4815cd3 100644 --- a/crates/next-core/src/next_client_reference/mod.rs +++ b/crates/next-core/src/next_client_reference/mod.rs @@ -7,5 +7,5 @@ pub use ecmascript_client_reference::{ }; pub use visit_client_reference::{ client_reference_graph, ClientReference, ClientReferenceGraphResult, ClientReferenceType, - ClientReferenceTypes, + ClientReferenceTypes, VisitedClientReferenceGraphNodes, }; 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 ef463f415f743..ea3890eecb1b0 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 @@ -1,4 +1,4 @@ -use std::future::Future; +use std::{collections::HashSet, future::Future}; use anyhow::Result; use indexmap::{IndexMap, IndexSet}; @@ -6,16 +6,13 @@ use serde::{Deserialize, Serialize}; use tracing::Instrument; use turbo_tasks::{ debug::ValueDebugFormat, - graph::{AdjacencyMap, GraphTraversal, Visit, VisitControlFlow}, + graph::{AdjacencyMap, GraphTraversal, Visit, VisitControlFlow, VisitedNodes}, trace::TraceRawVcs, RcStr, ReadRef, TryJoinIterExt, ValueToString, Vc, }; use turbo_tasks_fs::FileSystemPath; use turbopack::css::CssModuleAsset; -use turbopack_core::{ - module::{Module, Modules}, - reference::primary_referenced_modules, -}; +use turbopack_core::{module::Module, reference::primary_referenced_modules}; use super::ecmascript_client_reference::ecmascript_client_reference_module::EcmascriptClientReferenceModule; use crate::next_server_component::server_component_module::NextServerComponentModule; @@ -46,8 +43,8 @@ pub enum ClientReferenceType { CssClientReference(Vc), } -#[turbo_tasks::value] -#[derive(Debug)] +#[turbo_tasks::value(shared)] +#[derive(Clone, Debug)] pub struct ClientReferenceGraphResult { pub client_references: Vec, /// Only the [`ClientReferenceType::EcmascriptClientReference`]s are listed in this map. @@ -56,6 +53,30 @@ pub struct ClientReferenceGraphResult { IndexMap>, Vec>>>, pub server_component_entries: Vec>, pub server_utils: Vec>>, + pub visited_nodes: Vc, +} + +impl Default for ClientReferenceGraphResult { + fn default() -> Self { + ClientReferenceGraphResult { + client_references: Default::default(), + client_references_by_server_component: Default::default(), + server_component_entries: Default::default(), + server_utils: Default::default(), + visited_nodes: VisitedClientReferenceGraphNodes::empty(), + } + } +} + +#[turbo_tasks::value(shared)] +pub struct VisitedClientReferenceGraphNodes(HashSet); + +#[turbo_tasks::value_impl] +impl VisitedClientReferenceGraphNodes { + #[turbo_tasks::function] + pub fn empty() -> Vc { + VisitedClientReferenceGraphNodes(Default::default()).cell() + } } #[turbo_tasks::value(transparent)] @@ -74,13 +95,39 @@ impl ClientReferenceGraphResult { } } +impl ClientReferenceGraphResult { + pub fn extend(&mut self, other: &Self) { + self.client_references + .extend(other.client_references.iter().copied()); + for (k, v) in other.client_references_by_server_component.iter() { + self.client_references_by_server_component + .entry(*k) + .or_insert_with(Vec::new) + .extend(v); + } + self.server_component_entries + .extend(other.server_component_entries.iter().copied()); + self.server_utils.extend(other.server_utils.iter().copied()); + // This is merged already by `client_reference_graph` itself + self.visited_nodes = other.visited_nodes; + } +} + +// impl ClientReferenceGraphResult { +// pub fn take_visited(&mut self) -> Vc { +// std::mem::replace( +// &mut self.visited_nodes, +// VisitedClientReferenceGraphNodes::empty(), +// ) +// } +// } + #[turbo_tasks::function] pub async fn client_reference_graph( - entries: Vc, + entry: Vc>, + visited_nodes: Vc, ) -> Result> { async move { - let entries = entries.await?; - let mut client_references = vec![]; let mut server_component_entries = vec![]; let mut server_utils = vec![]; @@ -90,33 +137,34 @@ pub async fn client_reference_graph( // first client_references_by_server_component.insert(None, Vec::new()); - let graph = AdjacencyMap::new() - .skip_duplicates() + let (graph, visited_nodes) = AdjacencyMap::new() + .skip_duplicates_with_visited_nodes(VisitedNodes(visited_nodes.await?.0.clone())) .visit( - entries - .iter() - .copied() - .map(|module| async move { - Ok(VisitClientReferenceNode { - state: VisitClientReferenceNodeState::Entry { - entry_path: module.ident().path().resolve().await?, - }, - ty: VisitClientReferenceNodeType::Internal( - module, - module.ident().to_string().await?, - ), - }) - }) - .try_join() - .await?, + vec![VisitClientReferenceNode { + state: { + if let Some(server_component) = + Vc::try_resolve_downcast_type::(entry) + .await? + { + VisitClientReferenceNodeState::InServerComponent { server_component } + } else { + VisitClientReferenceNodeState::Entry { + entry_path: entry.ident().path().resolve().await?, + } + } + }, + ty: VisitClientReferenceNodeType::Internal( + entry, + entry.ident().to_string().await?, + ), + }], VisitClientReference, ) .await .completed()? - .into_inner() - .into_reverse_topological(); + .into_inner_with_visited(); - for node in graph { + for node in graph.into_reverse_topological() { match &node.ty { VisitClientReferenceNodeType::Internal(_asset, _) => { // No-op. These nodes are only useful during graph @@ -148,6 +196,7 @@ pub async fn client_reference_graph( client_references_by_server_component, server_component_entries, server_utils, + visited_nodes: VisitedClientReferenceGraphNodes(visited_nodes.0).cell(), } .cell()) } @@ -207,9 +256,9 @@ impl Visit for VisitClientReference { fn visit(&mut self, edge: Self::Edge) -> VisitControlFlow { match edge.ty { VisitClientReferenceNodeType::ClientReference(..) => VisitControlFlow::Skip(edge), - VisitClientReferenceNodeType::Internal(..) => VisitControlFlow::Continue(edge), - VisitClientReferenceNodeType::ServerUtilEntry(..) => VisitControlFlow::Continue(edge), - VisitClientReferenceNodeType::ServerComponentEntry(..) => { + VisitClientReferenceNodeType::Internal(..) + | VisitClientReferenceNodeType::ServerUtilEntry(..) + | VisitClientReferenceNodeType::ServerComponentEntry(..) => { VisitControlFlow::Continue(edge) } } From 00e6cca0cc670b19837ca6d3bbdd1cd26dc6e79e Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Fri, 4 Oct 2024 16:11:13 +0200 Subject: [PATCH 2/4] fixups --- crates/next-api/src/app.rs | 33 ++++++++++++--------------------- 1 file changed, 12 insertions(+), 21 deletions(-) diff --git a/crates/next-api/src/app.rs b/crates/next-api/src/app.rs index ecebae257a8fa..ef14b45b84d68 100644 --- a/crates/next-api/src/app.rs +++ b/crates/next-api/src/app.rs @@ -867,33 +867,24 @@ impl AppEndpoint { Vc::try_resolve_downcast_type::(rsc_entry) .await? { - let mut modules: Vec<_> = module_part_asset + let modules: Vec<_> = module_part_asset .await? .full_module .await? .inner_assets .iter() - .try_join() // TODO prevent double collect + .try_join() .await? .iter() .flat_map(|a| a.values()) .copied() - .collect(); // TODO prevent double collect - modules.push(Vc::upcast(module_part_asset)); + .chain(std::iter::once(Vc::upcast(module_part_asset))) + .collect(); modules } else { vec![rsc_entry] }; - println!( - "before {:?}", - modules - .iter() - .map(|module| module.ident().to_string()) - .try_join() - .await? - ); - let mut client_references: ClientReferenceGraphResult = ClientReferenceGraphResult::default(); @@ -903,9 +894,9 @@ impl AppEndpoint { client_references.extend(¤t_client_references); } - // TODO remove cell - client_references.cell() + client_references }; + let client_references_cell = client_references.clone().cell(); let ssr_chunking_context = if process_ssr { Some(match runtime { @@ -926,7 +917,6 @@ impl AppEndpoint { let mut visited_modules = VisitedDynamicImportModules::empty(); for refs in client_references - .await? .client_references_by_server_component .values() { @@ -949,7 +939,7 @@ impl AppEndpoint { }; let client_references_chunks = get_app_client_references_chunks( - client_references, + client_references_cell, client_chunking_context, Value::new(client_shared_availability_info), ssr_chunking_context, @@ -1049,7 +1039,7 @@ impl AppEndpoint { node_root, client_relative_path, app_entry.original_name.clone(), - client_references, + client_references_cell, client_references_chunks, client_chunking_context, ssr_chunking_context, @@ -1077,7 +1067,9 @@ impl AppEndpoint { } ( - Some(get_app_server_reference_modules(client_references.types())), + Some(get_app_server_reference_modules( + client_references_cell.types(), + )), Some(client_dynamic_imports), Some(client_references), ) @@ -1314,8 +1306,7 @@ impl AppEndpoint { let mut current_chunks = OutputAssets::empty(); let mut current_availability_info = AvailabilityInfo::Root; if let Some(client_references) = client_references { - let client_references = client_references.await?; - let span = tracing::trace_span!("server utils",); + let span = tracing::trace_span!("server utils"); async { let utils_module = IncludeModulesModule::new( AssetIdent::from_path(this.app_project.project().project_path()) From 3e04e55ac175d0a586cc6f7519c3ce8d86488f61 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Fri, 4 Oct 2024 16:20:09 +0200 Subject: [PATCH 3/4] Cleanup --- .../next_client_reference/visit_client_reference.rs | 10 +--------- 1 file changed, 1 insertion(+), 9 deletions(-) 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 ea3890eecb1b0..2e76428dfa705 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 @@ -96,6 +96,7 @@ impl ClientReferenceGraphResult { } impl ClientReferenceGraphResult { + /// Merges multiple return values of client_reference_graph together. pub fn extend(&mut self, other: &Self) { self.client_references .extend(other.client_references.iter().copied()); @@ -113,15 +114,6 @@ impl ClientReferenceGraphResult { } } -// impl ClientReferenceGraphResult { -// pub fn take_visited(&mut self) -> Vc { -// std::mem::replace( -// &mut self.visited_nodes, -// VisitedClientReferenceGraphNodes::empty(), -// ) -// } -// } - #[turbo_tasks::function] pub async fn client_reference_graph( entry: Vc>, From 7ac962c68da592df8008f985713e0a6254e85a11 Mon Sep 17 00:00:00 2001 From: Niklas Mischkulnig <4586894+mischnic@users.noreply.github.com> Date: Fri, 4 Oct 2024 20:31:21 +0200 Subject: [PATCH 4/4] Remove typecast hack --- crates/next-api/src/app.rs | 49 ++++---- .../src/next_client_reference/mod.rs | 4 +- .../visit_client_reference.rs | 114 ++++++++++++++---- 3 files changed, 116 insertions(+), 51 deletions(-) diff --git a/crates/next-api/src/app.rs b/crates/next-api/src/app.rs index ef14b45b84d68..d03c2040cb576 100644 --- a/crates/next-api/src/app.rs +++ b/crates/next-api/src/app.rs @@ -19,7 +19,8 @@ use next_core::{ get_client_runtime_entries, ClientContextType, RuntimeEntries, }, next_client_reference::{ - client_reference_graph, ClientReferenceGraphResult, NextEcmascriptClientReferenceTransition, + client_reference_graph, find_server_entries, NextEcmascriptClientReferenceTransition, + ServerEntries, VisitedClientReferenceGraphNodes, }, next_config::NextConfig, next_dynamic::NextDynamicTransition, @@ -64,7 +65,7 @@ use turbopack_core::{ source::Source, virtual_output::VirtualOutputAsset, }; -use turbopack_ecmascript::{resolve::cjs_resolve, tree_shake::asset::EcmascriptModulePartAsset}; +use turbopack_ecmascript::resolve::cjs_resolve; use crate::{ dynamic_imports::{ @@ -863,34 +864,26 @@ impl AppEndpoint { let client_shared_availability_info = client_shared_chunk_group.availability_info; let client_references = { - let modules = if let Some(module_part_asset) = - Vc::try_resolve_downcast_type::(rsc_entry) - .await? - { - let modules: Vec<_> = module_part_asset - .await? - .full_module - .await? - .inner_assets - .iter() - .try_join() - .await? - .iter() - .flat_map(|a| a.values()) - .copied() - .chain(std::iter::once(Vc::upcast(module_part_asset))) - .collect(); - modules - } else { - vec![rsc_entry] - }; - - let mut client_references: ClientReferenceGraphResult = - ClientReferenceGraphResult::default(); + let ServerEntries { + server_component_entries, + server_utils, + } = &*find_server_entries(rsc_entry).await?; + + let mut client_references = client_reference_graph( + server_utils.clone(), + VisitedClientReferenceGraphNodes::empty(), + ) + .await? + .clone_value(); - for module in modules { + for module in server_component_entries + .iter() + .map(|m| Vc::upcast::>(*m)) + .chain(std::iter::once(rsc_entry)) + { let current_client_references = - client_reference_graph(module, client_references.visited_nodes).await?; + client_reference_graph(vec![module], client_references.visited_nodes) + .await?; client_references.extend(¤t_client_references); } diff --git a/crates/next-core/src/next_client_reference/mod.rs b/crates/next-core/src/next_client_reference/mod.rs index 69f19c4815cd3..3fff07518a6de 100644 --- a/crates/next-core/src/next_client_reference/mod.rs +++ b/crates/next-core/src/next_client_reference/mod.rs @@ -6,6 +6,6 @@ pub use ecmascript_client_reference::{ ecmascript_client_reference_transition::NextEcmascriptClientReferenceTransition, }; pub use visit_client_reference::{ - client_reference_graph, ClientReference, ClientReferenceGraphResult, ClientReferenceType, - ClientReferenceTypes, VisitedClientReferenceGraphNodes, + client_reference_graph, find_server_entries, ClientReference, ClientReferenceGraphResult, + ClientReferenceType, ClientReferenceTypes, ServerEntries, VisitedClientReferenceGraphNodes, }; 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 2e76428dfa705..48d39969b6717 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 @@ -116,7 +116,7 @@ impl ClientReferenceGraphResult { #[turbo_tasks::function] pub async fn client_reference_graph( - entry: Vc>, + entries: Vec>>, visited_nodes: Vc, ) -> Result> { async move { @@ -132,25 +132,34 @@ pub async fn client_reference_graph( let (graph, visited_nodes) = AdjacencyMap::new() .skip_duplicates_with_visited_nodes(VisitedNodes(visited_nodes.await?.0.clone())) .visit( - vec![VisitClientReferenceNode { - state: { - if let Some(server_component) = - Vc::try_resolve_downcast_type::(entry) - .await? - { - VisitClientReferenceNodeState::InServerComponent { server_component } - } else { - VisitClientReferenceNodeState::Entry { - entry_path: entry.ident().path().resolve().await?, - } - } - }, - ty: VisitClientReferenceNodeType::Internal( - entry, - entry.ident().to_string().await?, - ), - }], - VisitClientReference, + entries + .iter() + .copied() + .map(|module| async move { + Ok(VisitClientReferenceNode { + state: if let Some(server_component) = + Vc::try_resolve_downcast_type::(module) + .await? + { + VisitClientReferenceNodeState::InServerComponent { + server_component, + } + } else { + VisitClientReferenceNodeState::Entry { + entry_path: module.ident().path().resolve().await?, + } + }, + ty: VisitClientReferenceNodeType::Internal( + module, + module.ident().to_string().await?, + ), + }) + }) + .try_join() + .await?, + VisitClientReference { + stop_at_server_entries: false, + }, ) .await .completed()? @@ -196,7 +205,60 @@ pub async fn client_reference_graph( .await } -struct VisitClientReference; +#[turbo_tasks::value(shared)] +#[derive(Clone, Debug)] +pub struct ServerEntries { + pub server_component_entries: Vec>, + pub server_utils: Vec>>, +} + +#[turbo_tasks::function] +pub async fn find_server_entries(entry: Vc>) -> Result> { + let graph = AdjacencyMap::new() + .skip_duplicates() + .visit( + vec![VisitClientReferenceNode { + state: { + VisitClientReferenceNodeState::Entry { + entry_path: entry.ident().path().resolve().await?, + } + }, + ty: VisitClientReferenceNodeType::Internal(entry, entry.ident().to_string().await?), + }], + VisitClientReference { + stop_at_server_entries: true, + }, + ) + .await + .completed()? + .into_inner(); + + let mut server_component_entries = vec![]; + let mut server_utils = vec![]; + for node in graph.reverse_topological() { + match &node.ty { + VisitClientReferenceNodeType::ServerUtilEntry(server_util, _) => { + server_utils.push(*server_util); + } + VisitClientReferenceNodeType::ServerComponentEntry(server_component, _) => { + server_component_entries.push(*server_component); + } + VisitClientReferenceNodeType::Internal(_, _) + | VisitClientReferenceNodeType::ClientReference(_, _) => {} + } + } + + Ok(ServerEntries { + server_component_entries, + server_utils, + } + .cell()) +} + +struct VisitClientReference { + /// Used to discover ServerComponents and ServerUtils + stop_at_server_entries: bool, +} #[derive( Clone, Eq, PartialEq, Hash, Serialize, Deserialize, Debug, ValueDebugFormat, TraceRawVcs, @@ -246,6 +308,16 @@ impl Visit for VisitClientReference { type EdgesFuture = impl Future>; fn visit(&mut self, edge: Self::Edge) -> VisitControlFlow { + if self.stop_at_server_entries + && matches!( + edge.ty, + VisitClientReferenceNodeType::ServerUtilEntry(..) + | VisitClientReferenceNodeType::ServerComponentEntry(..) + ) + { + return VisitControlFlow::Skip(edge); + } + match edge.ty { VisitClientReferenceNodeType::ClientReference(..) => VisitControlFlow::Skip(edge), VisitClientReferenceNodeType::Internal(..)