Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor(turbopack): Use ResolvedVc<T> for struct fields in next-api, final part #73367

Merged
merged 12 commits into from
Dec 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 4 additions & 4 deletions crates/napi/src/next_api/project.rs
Original file line number Diff line number Diff line change
Expand Up @@ -529,14 +529,14 @@
} => NapiRoute {
pathname,
r#type: "page",
html_endpoint: convert_endpoint(html_endpoint),
data_endpoint: convert_endpoint(data_endpoint),
html_endpoint: convert_endpoint(*html_endpoint),
data_endpoint: convert_endpoint(*data_endpoint),
..Default::default()
},
Route::PageApi { endpoint } => NapiRoute {
pathname,
r#type: "page-api",
endpoint: convert_endpoint(endpoint),
endpoint: convert_endpoint(*endpoint),
..Default::default()
},
Route::AppPage(pages) => NapiRoute {
Expand All @@ -561,7 +561,7 @@
pathname,
original_name: Some(original_name),
r#type: "app-route",
endpoint: convert_endpoint(endpoint),
endpoint: convert_endpoint(*endpoint),
..Default::default()
},
Route::Conflict => NapiRoute {
Expand Down Expand Up @@ -949,7 +949,7 @@
}
}

/// Subscribes to lifecycle events of the compilation.

Check warning on line 952 in crates/napi/src/next_api/project.rs

View workflow job for this annotation

GitHub Actions / rustdoc check / build

public documentation for `project_update_info_subscribe` links to private item `UpdateMessage::Start`
///
/// Emits an [UpdateMessage::Start] event when any computation starts.
/// Emits an [UpdateMessage::End] event when there was no computation for the
Expand Down
8 changes: 4 additions & 4 deletions crates/next-api/src/app.rs
Original file line number Diff line number Diff line change
Expand Up @@ -711,24 +711,24 @@ pub fn app_entry_point_to_route(
root_layouts,
} => Route::AppRoute {
original_name: page.to_string(),
endpoint: Vc::upcast(
endpoint: ResolvedVc::upcast(
AppEndpoint {
ty: AppEndpointType::Route { path, root_layouts },
app_project,
page,
}
.cell(),
.resolved_cell(),
),
},
AppEntrypoint::AppMetadata { page, metadata } => Route::AppRoute {
original_name: page.to_string(),
endpoint: Vc::upcast(
endpoint: ResolvedVc::upcast(
AppEndpoint {
ty: AppEndpointType::Metadata { metadata },
app_project,
page,
}
.cell(),
.resolved_cell(),
),
},
}
Expand Down
8 changes: 4 additions & 4 deletions crates/next-api/src/global_module_id_strategy.rs
Original file line number Diff line number Diff line change
Expand Up @@ -47,11 +47,11 @@ impl GlobalModuleIdStrategyBuilder {
html_endpoint,
data_endpoint,
} => {
preprocessed_module_ids.push(preprocess_module_ids(*html_endpoint));
preprocessed_module_ids.push(preprocess_module_ids(*data_endpoint));
preprocessed_module_ids.push(preprocess_module_ids(**html_endpoint));
preprocessed_module_ids.push(preprocess_module_ids(**data_endpoint));
}
Route::PageApi { endpoint } => {
preprocessed_module_ids.push(preprocess_module_ids(*endpoint));
preprocessed_module_ids.push(preprocess_module_ids(**endpoint));
}
Route::AppPage(page_routes) => {
for page_route in page_routes {
Expand All @@ -65,7 +65,7 @@ impl GlobalModuleIdStrategyBuilder {
original_name: _,
endpoint,
} => {
preprocessed_module_ids.push(preprocess_module_ids(*endpoint));
preprocessed_module_ids.push(preprocess_module_ids(**endpoint));
}
Route::Conflict => {
tracing::info!("WARN: conflict");
Expand Down
87 changes: 57 additions & 30 deletions crates/next-api/src/pages.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
use std::future::IntoFuture;

use anyhow::{bail, Context, Result};
use futures::future::BoxFuture;
use next_core::{
all_assets_from_entries, create_page_loader_entry_module, get_asset_path_from_pathname,
get_edge_resolve_options_context,
Expand Down Expand Up @@ -105,7 +106,11 @@ impl PagesProject {
async fn add_page_to_routes(
routes: &mut FxIndexMap<RcStr, Route>,
page: Vc<PagesStructureItem>,
make_route: impl Fn(Vc<RcStr>, Vc<RcStr>, Vc<PagesStructureItem>) -> Route,
make_route: impl Fn(
Vc<RcStr>,
Vc<RcStr>,
Vc<PagesStructureItem>,
) -> BoxFuture<'static, Result<Route>>,
) -> Result<()> {
let PagesStructureItem {
next_router_path,
Expand All @@ -115,15 +120,19 @@ impl PagesProject {
let pathname: RcStr = format!("/{}", next_router_path.await?.path).into();
let pathname_vc = Vc::cell(pathname.clone());
let original_name = Vc::cell(format!("/{}", original_path.await?.path).into());
let route = make_route(pathname_vc, original_name, page);
let route = make_route(pathname_vc, original_name, page).await?;
routes.insert(pathname, route);
Ok(())
}

async fn add_dir_to_routes(
routes: &mut FxIndexMap<RcStr, Route>,
dir: Vc<PagesDirectoryStructure>,
make_route: impl Fn(Vc<RcStr>, Vc<RcStr>, Vc<PagesStructureItem>) -> Route,
make_route: impl Fn(
Vc<RcStr>,
Vc<RcStr>,
Vc<PagesStructureItem>,
) -> BoxFuture<'static, Result<Route>>,
) -> Result<()> {
let mut queue = vec![dir];
while let Some(dir) = queue.pop() {
Expand All @@ -145,37 +154,55 @@ impl PagesProject {

if let Some(api) = *api {
add_dir_to_routes(&mut routes, *api, |pathname, original_name, page| {
Route::PageApi {
endpoint: Vc::upcast(PageEndpoint::new(
PageEndpointType::Api,
self,
pathname,
original_name,
page,
pages_structure,
)),
}
Box::pin(async move {
Ok(Route::PageApi {
endpoint: ResolvedVc::upcast(
PageEndpoint::new(
PageEndpointType::Api,
self,
pathname,
original_name,
page,
pages_structure,
)
.to_resolved()
.await?,
),
})
})
})
.await?;
}

let make_page_route = |pathname, original_name, page| Route::Page {
html_endpoint: Vc::upcast(PageEndpoint::new(
PageEndpointType::Html,
self,
pathname,
original_name,
page,
pages_structure,
)),
data_endpoint: Vc::upcast(PageEndpoint::new(
PageEndpointType::Data,
self,
pathname,
original_name,
page,
pages_structure,
)),
let make_page_route = |pathname, original_name, page| -> BoxFuture<_> {
Box::pin(async move {
Ok(Route::Page {
html_endpoint: ResolvedVc::upcast(
PageEndpoint::new(
PageEndpointType::Html,
self,
pathname,
original_name,
page,
pages_structure,
)
.to_resolved()
.await?,
),
data_endpoint: ResolvedVc::upcast(
PageEndpoint::new(
PageEndpointType::Data,
self,
pathname,
original_name,
page,
pages_structure,
)
.to_resolved()
.await?,
),
})
})
};

if let Some(pages) = *pages {
Expand Down
35 changes: 11 additions & 24 deletions crates/next-api/src/route.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,9 @@
use anyhow::Result;
use serde::{Deserialize, Serialize};
use turbo_rcstr::RcStr;
use turbo_tasks::{debug::ValueDebugFormat, trace::TraceRawVcs, Completion, FxIndexMap, Vc};
use turbo_tasks::{
debug::ValueDebugFormat, trace::TraceRawVcs, Completion, FxIndexMap, ResolvedVc, Vc,
};
use turbopack_core::module::Modules;

use crate::paths::ServerPath;
Expand Down Expand Up @@ -30,43 +32,28 @@ impl AppPageRoute {
#[derive(Clone, Debug)]
pub enum Route {
Page {
html_endpoint: Vc<Box<dyn Endpoint>>,
data_endpoint: Vc<Box<dyn Endpoint>>,
html_endpoint: ResolvedVc<Box<dyn Endpoint>>,
data_endpoint: ResolvedVc<Box<dyn Endpoint>>,
},
PageApi {
endpoint: Vc<Box<dyn Endpoint>>,
endpoint: ResolvedVc<Box<dyn Endpoint>>,
},
AppPage(Vec<AppPageRoute>),
AppRoute {
original_name: String,
endpoint: Vc<Box<dyn Endpoint>>,
endpoint: ResolvedVc<Box<dyn Endpoint>>,
},
Conflict,
}

impl Route {
pub async fn resolve(&mut self) -> Result<()> {
match self {
Route::Page {
html_endpoint,
data_endpoint,
} => {
*html_endpoint = html_endpoint.resolve().await?;
*data_endpoint = data_endpoint.resolve().await?;
if let Route::AppPage(routes) = self {
for route in routes {
route.resolve().await?;
Copy link
Contributor

@mischnic mischnic Dec 5, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We need to change AppPageRoute to contain ResolvedVc as well

Then this whole function can be removed

}
Route::PageApi { endpoint } => {
*endpoint = endpoint.resolve().await?;
}
Route::AppPage(routes) => {
for route in routes {
route.resolve().await?;
}
}
Route::AppRoute { endpoint, .. } => {
*endpoint = endpoint.resolve().await?;
}
Route::Conflict => {}
}

Ok(())
}
}
Expand Down
Loading