Skip to content

Commit

Permalink
fix(turbo-tasks): Implement TaskInput for ResolvedVc` (#70814)
Browse files Browse the repository at this point in the history 
`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.
  • Loading branch information
@bgw @kdy1
bgw authored and kdy1 committed Oct 10, 2024
1 parent 1d9afcd commit c565cf7
Show file tree
Hide file tree
Showing 4 changed files with 67 additions and 40 deletions.
9 changes: 6 additions & 3 deletions crates/next-api/src/app.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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(),
Expand All @@ -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,
Expand All @@ -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,
}
Expand Down
67 changes: 35 additions & 32 deletions crates/next-core/src/app_structure.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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::{
Expand Down Expand Up @@ -296,23 +296,22 @@ 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;
}
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),
_ => {}
}
}
Expand All @@ -334,17 +333,17 @@ 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;
}
_ => continue,
};

if dynamic {
entry.push((number, MetadataWithAltItem::Dynamic { path: file }));
entry.push((number, MetadataWithAltItem::Dynamic { path: *file }));
continue;
}

Expand All @@ -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);
}
}
Expand Down Expand Up @@ -484,12 +482,12 @@ impl AppPageLoaderTree {
pub enum Entrypoint {
AppPage {
pages: Vec<AppPage>,
loader_tree: Vc<AppPageLoaderTree>,
loader_tree: ResolvedVc<AppPageLoaderTree>,
},
AppRoute {
page: AppPage,
path: Vc<FileSystemPath>,
root_layouts: Vc<Vec<Vc<FileSystemPath>>>,
path: ResolvedVc<FileSystemPath>,
root_layouts: ResolvedVc<Vec<Vc<FileSystemPath>>>,
},
AppMetadata {
page: AppPage,
Expand Down Expand Up @@ -557,7 +555,7 @@ fn add_app_page(
app_dir: Vc<FileSystemPath>,
result: &mut IndexMap<AppPath, Entrypoint>,
page: AppPage,
loader_tree: Vc<AppPageLoaderTree>,
loader_tree: ResolvedVc<AppPageLoaderTree>,
) {
let mut e = match result.entry(page.clone().into()) {
Entry::Occupied(e) => e,
Expand Down Expand Up @@ -616,8 +614,8 @@ fn add_app_route(
app_dir: Vc<FileSystemPath>,
result: &mut IndexMap<AppPath, Entrypoint>,
page: AppPage,
path: Vc<FileSystemPath>,
root_layouts: Vc<Vec<Vc<FileSystemPath>>>,
path: ResolvedVc<FileSystemPath>,
root_layouts: ResolvedVc<Vec<Vc<FileSystemPath>>>,
) {
let e = match result.entry(page.clone().into()) {
Entry::Occupied(e) => e,
Expand Down Expand Up @@ -1046,7 +1044,7 @@ async fn directory_tree_to_entrypoints_internal(
directory_name: RcStr,
directory_tree: Vc<DirectoryTree>,
app_page: AppPage,
root_layouts: Vc<Vec<Vc<FileSystemPath>>>,
root_layouts: ResolvedVc<Vec<Vc<FileSystemPath>>>,
) -> Result<Vc<Entrypoints>> {
let span = tracing::info_span!("build layout trees", name = display(&app_page));
directory_tree_to_entrypoints_internal_untraced(
Expand All @@ -1067,7 +1065,7 @@ async fn directory_tree_to_entrypoints_internal_untraced(
directory_name: RcStr,
directory_tree: Vc<DirectoryTree>,
app_page: AppPage,
root_layouts: Vc<Vec<Vc<FileSystemPath>>>,
root_layouts: ResolvedVc<Vec<Vc<FileSystemPath>>>,
) -> Result<Vc<Entrypoints>> {
let mut result = IndexMap::new();

Expand All @@ -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
};
Expand All @@ -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?,
);
}

Expand All @@ -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,
);
}
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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?;

Expand Down Expand Up @@ -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?,
);
}
}
Expand Down
23 changes: 22 additions & 1 deletion turbopack/crates/turbo-tasks/src/task/task_input.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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<T> TaskInput for ResolvedVc<T>
where
T: Send,
{
fn is_resolved(&self) -> bool {
true
}

fn is_transient(&self) -> bool {
self.node.is_transient()
}

async fn resolve(&self) -> Result<Self> {
Ok(*self)
}
}

impl<T> TaskInput for Value<T>
where
T: Any
Expand Down
8 changes: 4 additions & 4 deletions turbopack/crates/turbopack-core/src/resolve/mod.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
})
Expand All @@ -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!(
Expand All @@ -1037,7 +1037,7 @@ async fn any_exists(
) {
None
} else {
Some((ty, path))
Some((ty, *path))
},
)
}
Expand Down

0 comments on commit c565cf7

Please sign in to comment.