From cbf32150a5b146c14585717fca3d882fa455ba5f Mon Sep 17 00:00:00 2001 From: Benjamin Woodruff Date: Mon, 7 Oct 2024 13:53:35 -0700 Subject: [PATCH] fix(turbo-tasks): Implement `TaskInput for `ResolvedVc` (#70814) `TaskInput` isn't needed/used for a bare `ResolvedVc`, as we'll expose `ResolvedVc` arguments as `Vc`, but it is useful for structs that contain `ResolvedVc` and want to derive `TaskInput`. This PR also ports `next_core::app_structure::Entrypoint` to use `ResolvedVc` as a way of testing this. --- crates/next-api/src/app.rs | 9 ++- crates/next-core/src/app_structure.rs | 67 ++++++++++--------- .../crates/turbo-tasks/src/task/task_input.rs | 23 ++++++- .../crates/turbopack-core/src/resolve/mod.rs | 8 +-- 4 files changed, 67 insertions(+), 40 deletions(-) diff --git a/crates/next-api/src/app.rs b/crates/next-api/src/app.rs index 5772561ae4be62..6469ec953672b9 100644 --- a/crates/next-api/src/app.rs +++ b/crates/next-api/src/app.rs @@ -627,7 +627,7 @@ pub fn app_entry_point_to_route( AppEndpoint { ty: AppEndpointType::Page { ty: AppPageEndpointType::Html, - loader_tree, + loader_tree: *loader_tree, }, app_project, page: page.clone(), @@ -638,7 +638,7 @@ pub fn app_entry_point_to_route( AppEndpoint { ty: AppEndpointType::Page { ty: AppPageEndpointType::Rsc, - loader_tree, + loader_tree: *loader_tree, }, app_project, page, @@ -656,7 +656,10 @@ pub fn app_entry_point_to_route( original_name: page.to_string(), endpoint: Vc::upcast( AppEndpoint { - ty: AppEndpointType::Route { path, root_layouts }, + ty: AppEndpointType::Route { + path: *path, + root_layouts: *root_layouts, + }, app_project, page, } diff --git a/crates/next-core/src/app_structure.rs b/crates/next-core/src/app_structure.rs index 33aa5a7dc4c39d..7c3cf03bc4fdef 100644 --- a/crates/next-core/src/app_structure.rs +++ b/crates/next-core/src/app_structure.rs @@ -10,8 +10,8 @@ use rustc_hash::FxHashMap; use serde::{Deserialize, Serialize}; use tracing::Instrument; use turbo_tasks::{ - debug::ValueDebugFormat, trace::TraceRawVcs, RcStr, TaskInput, TryJoinIterExt, ValueToString, - Vc, + debug::ValueDebugFormat, trace::TraceRawVcs, RcStr, ResolvedVc, TaskInput, TryJoinIterExt, + ValueToString, Vc, }; use turbo_tasks_fs::{DirectoryContent, DirectoryEntry, FileSystemEntryType, FileSystemPath}; use turbopack_core::issue::{ @@ -296,7 +296,6 @@ async fn get_directory_tree_internal( let entry = entry.resolve_symlink().await?; match entry { DirectoryEntry::File(file) => { - let file = file.resolve().await?; // Do not process .d.ts files as routes if basename.ends_with(".d.ts") { continue; @@ -304,15 +303,15 @@ async fn get_directory_tree_internal( if let Some((stem, ext)) = basename.split_once('.') { if page_extensions_value.iter().any(|e| e == ext) { match stem { - "page" => modules.page = Some(file), - "layout" => modules.layout = Some(file), - "error" => modules.error = Some(file), - "global-error" => modules.global_error = Some(file), - "loading" => modules.loading = Some(file), - "template" => modules.template = Some(file), - "not-found" => modules.not_found = Some(file), - "default" => modules.default = Some(file), - "route" => modules.route = Some(file), + "page" => modules.page = Some(*file), + "layout" => modules.layout = Some(*file), + "error" => modules.error = Some(*file), + "global-error" => modules.global_error = Some(*file), + "loading" => modules.loading = Some(*file), + "template" => modules.template = Some(*file), + "not-found" => modules.not_found = Some(*file), + "default" => modules.default = Some(*file), + "route" => modules.route = Some(*file), _ => {} } } @@ -334,9 +333,9 @@ async fn get_directory_tree_internal( "opengraph-image" => &mut metadata_open_graph, "sitemap" => { if dynamic { - modules.metadata.sitemap = Some(MetadataItem::Dynamic { path: file }); + modules.metadata.sitemap = Some(MetadataItem::Dynamic { path: *file }); } else { - modules.metadata.sitemap = Some(MetadataItem::Static { path: file }); + modules.metadata.sitemap = Some(MetadataItem::Static { path: *file }); } continue; } @@ -344,7 +343,7 @@ async fn get_directory_tree_internal( }; if dynamic { - entry.push((number, MetadataWithAltItem::Dynamic { path: file })); + entry.push((number, MetadataWithAltItem::Dynamic { path: *file })); continue; } @@ -360,16 +359,15 @@ async fn get_directory_tree_internal( entry.push(( number, MetadataWithAltItem::Static { - path: file, + path: *file, alt_path, }, )); } DirectoryEntry::Directory(dir) => { - let dir = dir.resolve().await?; // appDir ignores paths starting with an underscore if !basename.starts_with('_') { - let result = get_directory_tree(dir, page_extensions); + let result = get_directory_tree(*dir, page_extensions); subdirectories.insert(basename.clone(), result); } } @@ -484,12 +482,12 @@ impl AppPageLoaderTree { pub enum Entrypoint { AppPage { pages: Vec, - loader_tree: Vc, + loader_tree: ResolvedVc, }, AppRoute { page: AppPage, - path: Vc, - root_layouts: Vc>>, + path: ResolvedVc, + root_layouts: ResolvedVc>>, }, AppMetadata { page: AppPage, @@ -557,7 +555,7 @@ fn add_app_page( app_dir: Vc, result: &mut IndexMap, page: AppPage, - loader_tree: Vc, + loader_tree: ResolvedVc, ) { let mut e = match result.entry(page.clone().into()) { Entry::Occupied(e) => e, @@ -616,8 +614,8 @@ fn add_app_route( app_dir: Vc, result: &mut IndexMap, page: AppPage, - path: Vc, - root_layouts: Vc>>, + path: ResolvedVc, + root_layouts: ResolvedVc>>, ) { let e = match result.entry(page.clone().into()) { Entry::Occupied(e) => e, @@ -1046,7 +1044,7 @@ async fn directory_tree_to_entrypoints_internal( directory_name: RcStr, directory_tree: Vc, app_page: AppPage, - root_layouts: Vc>>, + root_layouts: ResolvedVc>>, ) -> Result> { let span = tracing::info_span!("build layout trees", name = display(&app_page)); directory_tree_to_entrypoints_internal_untraced( @@ -1067,7 +1065,7 @@ async fn directory_tree_to_entrypoints_internal_untraced( directory_name: RcStr, directory_tree: Vc, app_page: AppPage, - root_layouts: Vc>>, + root_layouts: ResolvedVc>>, ) -> Result> { let mut result = IndexMap::new(); @@ -1082,7 +1080,7 @@ async fn directory_tree_to_entrypoints_internal_untraced( let root_layouts = if let Some(layout) = modules.layout { let mut layouts = (*root_layouts.await?).clone(); layouts.push(layout); - Vc::cell(layouts) + ResolvedVc::cell(layouts) } else { root_layouts }; @@ -1104,7 +1102,10 @@ async fn directory_tree_to_entrypoints_internal_untraced( app_dir, &mut result, app_page.complete(PageType::Page)?, - loader_tree.context("loader tree should be created for a page/default")?, + loader_tree + .context("loader tree should be created for a page/default")? + .to_resolved() + .await?, ); } @@ -1113,7 +1114,7 @@ async fn directory_tree_to_entrypoints_internal_untraced( app_dir, &mut result, app_page.complete(PageType::Route)?, - route, + route.to_resolved().await?, root_layouts, ); } @@ -1191,7 +1192,7 @@ async fn directory_tree_to_entrypoints_internal_untraced( }, modules: modules.without_leafs(), global_metadata, - }.cell(); + }.resolved_cell(); { let app_page = app_page @@ -1223,7 +1224,7 @@ async fn directory_tree_to_entrypoints_internal_untraced( subdir_name.clone(), subdirectory, child_app_page.clone(), - root_layouts, + *root_layouts, ) .await?; @@ -1278,7 +1279,9 @@ async fn directory_tree_to_entrypoints_internal_untraced( &mut result, page.clone(), loader_tree - .context("loader tree should be created for a page/default")?, + .context("loader tree should be created for a page/default")? + .to_resolved() + .await?, ); } } diff --git a/turbopack/crates/turbo-tasks/src/task/task_input.rs b/turbopack/crates/turbo-tasks/src/task/task_input.rs index d7c70e03371d1c..5da843a58ba38c 100644 --- a/turbopack/crates/turbo-tasks/src/task/task_input.rs +++ b/turbopack/crates/turbo-tasks/src/task/task_input.rs @@ -3,7 +3,9 @@ use std::{any::Any, fmt::Debug, future::Future, hash::Hash, time::Duration}; use anyhow::Result; use serde::{Deserialize, Serialize}; -use crate::{MagicAny, RcStr, TaskId, TransientInstance, TransientValue, Value, ValueTypeId, Vc}; +use crate::{ + MagicAny, RcStr, ResolvedVc, TaskId, TransientInstance, TransientValue, Value, ValueTypeId, Vc, +}; /// Trait to implement in order for a type to be accepted as a /// [`#[turbo_tasks::function]`][crate::function] argument. @@ -108,6 +110,25 @@ where } } +// `TaskInput` isn't needed/used for a bare `ResolvedVc`, as we'll expose `ResolvedVc` arguments as +// `Vc`, but it is useful for structs that contain `ResolvedVc` and want to derive `TaskInput`. +impl TaskInput for ResolvedVc +where + T: Send, +{ + fn is_resolved(&self) -> bool { + true + } + + fn is_transient(&self) -> bool { + self.node.is_transient() + } + + async fn resolve(&self) -> Result { + Ok(*self) + } +} + impl TaskInput for Value where T: Any diff --git a/turbopack/crates/turbopack-core/src/resolve/mod.rs b/turbopack/crates/turbopack-core/src/resolve/mod.rs index 7a3aa635a8b42b..e9c504aa6e9dfb 100644 --- a/turbopack/crates/turbopack-core/src/resolve/mod.rs +++ b/turbopack/crates/turbopack-core/src/resolve/mod.rs @@ -1012,9 +1012,9 @@ async fn type_exists( for path in result.symlinks.iter() { refs.push(Vc::upcast(FileSource::new(**path))); } - let path = result.path.resolve().await?; + let path = result.path; Ok(if *path.get_type().await? == ty { - Some(path) + Some(*path) } else { None }) @@ -1028,7 +1028,7 @@ async fn any_exists( for path in result.symlinks.iter() { refs.push(Vc::upcast(FileSource::new(**path))); } - let path = result.path.resolve().await?; + let path = result.path; let ty = *path.get_type().await?; Ok( if matches!( @@ -1037,7 +1037,7 @@ async fn any_exists( ) { None } else { - Some((ty, path)) + Some((ty, *path)) }, ) }