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.

Sign up for GitHub

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

Jump to bottom

chore: add structured app page path type #55070

Merged
merged 2 commits into from Sep 7, 2023
Merged

chore: add structured app page path type #55070

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3250d96
chore: add structured app page path type
ForsakenHarmony Sep 6, 2023
53ed729
Merge branch 'canary' into hrmny/structured-app-path
kodiakhq[bot] Sep 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
34 changes: 10 additions & 24 deletions packages/next-swc/crates/next-api/src/app.rs
View file
Open in desktop
Expand Up @@ -8,7 +8,7 @@ use next_core::{
mode::NextMode,
next_app::{
get_app_client_references_chunks, get_app_client_shared_chunks, get_app_page_entry,
get_app_route_entry, AppEntry,
get_app_route_entry, AppEntry, AppPage,
},
next_client::{
get_client_module_options_context, get_client_resolve_options_context,
Expand Down Expand Up @@ -344,8 +344,7 @@ impl AppProject {
.map(|(pathname, app_entrypoint)| async {
Ok((
pathname.clone(),
*app_entry_point_to_route(self, app_entrypoint.clone(), pathname.clone())
.await?,
*app_entry_point_to_route(self, app_entrypoint.clone()).await?,
))
})
.try_join()
Expand All @@ -360,22 +359,17 @@ impl AppProject {
pub async fn app_entry_point_to_route(
app_project: Vc<AppProject>,
entrypoint: AppEntrypoint,
pathname: String,
) -> Vc<Route> {
match entrypoint {
AppEntrypoint::AppPage {
original_name,
loader_tree,
} => Route::AppPage {
AppEntrypoint::AppPage { page, loader_tree } => Route::AppPage {
html_endpoint: Vc::upcast(
AppEndpoint {
ty: AppEndpointType::Page {
ty: AppPageEndpointType::Html,
loader_tree,
},
app_project,
pathname: pathname.clone(),
original_name: original_name.clone(),
page: page.clone(),
}
.cell(),
),
Expand All @@ -386,22 +380,17 @@ pub async fn app_entry_point_to_route(
loader_tree,
},
app_project,
pathname,
original_name,
page,
}
.cell(),
),
},
AppEntrypoint::AppRoute {
original_name,
path,
} => Route::AppRoute {
AppEntrypoint::AppRoute { page, path } => Route::AppRoute {
endpoint: Vc::upcast(
AppEndpoint {
ty: AppEndpointType::Route { path },
app_project,
pathname,
original_name,
page,
}
.cell(),
),
Expand Down Expand Up @@ -431,8 +420,7 @@ enum AppEndpointType {
struct AppEndpoint {
ty: AppEndpointType,
app_project: Vc<AppProject>,
pathname: String,
original_name: String,
page: AppPage,
}

#[turbo_tasks::value_impl]
Expand All @@ -444,8 +432,7 @@ impl AppEndpoint {
self.app_project.edge_rsc_module_context(),
loader_tree,
self.app_project.app_dir(),
self.pathname.clone(),
self.original_name.clone(),
self.page.clone(),
self.app_project.project().project_path(),
)
}
Expand All @@ -456,8 +443,7 @@ impl AppEndpoint {
self.app_project.rsc_module_context(),
self.app_project.edge_rsc_module_context(),
Vc::upcast(FileSource::new(path)),
self.pathname.clone(),
self.original_name.clone(),
self.page.clone(),
self.app_project.project().project_path(),
)
}
Expand Down
18 changes: 5 additions & 13 deletions packages/next-swc/crates/next-build/src/next_app/app_entries.rs
View file
Open in desktop
Expand Up @@ -187,31 +187,23 @@ pub async fn get_app_entries(
let mut entries = entrypoints
.await?
.iter()
.map(|(pathname, entrypoint)| async move {
.map(|(_, entrypoint)| async move {
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we just store a Set instead of a Map?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yeah, we might want to return a set, it's being constructed as a map though to check for duplicate paths

Ok(match entrypoint {
Entrypoint::AppPage {
original_name,
loader_tree,
} => get_app_page_entry(
Entrypoint::AppPage { page, loader_tree } => get_app_page_entry(
rsc_context,
// TODO add edge support
rsc_context,
*loader_tree,
app_dir,
pathname.clone(),
original_name.clone(),
page.clone(),
project_root,
),
Entrypoint::AppRoute {
original_name,
path,
} => get_app_route_entry(
Entrypoint::AppRoute { page, path } => get_app_route_entry(
rsc_context,
// TODO add edge support
rsc_context,
Vc::upcast(FileSource::new(*path)),
pathname.clone(),
original_name.clone(),
page.clone(),
project_root,
),
})
Expand Down
1 change: 1 addition & 0 deletions packages/next-swc/crates/next-core/Cargo.toml
View file
Open in desktop
Expand Up @@ -39,6 +39,7 @@ turbopack-binding = { workspace = true, features = [
"__turbo_tasks_hash",
"__turbopack",
"__turbopack_build",
"__turbopack_cli_utils",
"__turbopack_core",
"__turbopack_dev",
"__turbopack_dev_server",
Expand Down
78 changes: 37 additions & 41 deletions packages/next-swc/crates/next-core/src/app_source.rs
View file
Open in desktop
Expand Up @@ -65,7 +65,7 @@ use crate::{
fallback::get_fallback_page,
loader_tree::{LoaderTreeModule, ServerComponentTransition},
mode::NextMode,
next_app::UnsupportedDynamicMetadataIssue,
next_app::{AppPage, AppPath, PathSegment, UnsupportedDynamicMetadataIssue},
next_client::{
context::{
get_client_assets_path, get_client_module_options_context,
Expand Down Expand Up @@ -95,31 +95,28 @@ use crate::{
util::{render_data, NextRuntime},
};

fn pathname_to_segments(pathname: &str) -> Result<(Vec<BaseSegment>, RouteType)> {
fn app_path_to_segments(path: &AppPath) -> Result<(Vec<BaseSegment>, RouteType)> {
let mut segments = Vec::new();
let mut split = pathname.split('/');
while let Some(segment) = split.next() {
if segment.is_empty()
|| (segment.starts_with('(') && segment.ends_with(')') || segment.starts_with('@'))
{
// ignore
} else if segment.starts_with("[[...") && segment.ends_with("]]")
|| segment.starts_with("[...") && segment.ends_with(']')
{
// (optional) catch all segment
if split.remainder().is_some() {
bail!(
"Invalid route {}, catch all segment must be the last segment",
pathname
)
let mut iter = path.iter().peekable();

while let Some(segment) = iter.next() {
match segment {
PathSegment::Static(s) => {
segments.push(BaseSegment::Static(s.to_string()));
}
PathSegment::Dynamic(_) => {
segments.push(BaseSegment::Dynamic);
}
PathSegment::CatchAll(_) | PathSegment::OptionalCatchAll(_) => {
if iter.peek().is_some() {
bail!(
"Invalid route {}, catch all segment must be the last segment",
path
)
}

return Ok((segments, RouteType::CatchAll));
}
return Ok((segments, RouteType::CatchAll));
} else if segment.starts_with('[') || segment.ends_with(']') {
// dynamic segment
segments.push(BaseSegment::Dynamic);
} else {
// normal segment
segments.push(BaseSegment::Static(segment.to_string()));
}
}
Ok((segments, RouteType::Exact))
Expand Down Expand Up @@ -654,12 +651,12 @@ pub async fn create_app_source(
let entrypoints = entrypoints.await?;
let mut sources: Vec<_> = entrypoints
.iter()
.map(|(pathname, entrypoint)| match *entrypoint {
.map(|(_, entrypoint)| match *entrypoint {
Entrypoint::AppPage {
original_name: _,
ref page,
loader_tree,
} => create_app_page_source_for_route(
pathname.clone(),
page.clone(),
loader_tree,
context_ssr,
context,
Expand All @@ -672,11 +669,8 @@ pub async fn create_app_source(
output_path,
render_data,
),
Entrypoint::AppRoute {
original_name: _,
path,
} => create_app_route_source_for_route(
pathname.clone(),
Entrypoint::AppRoute { ref page, path } => create_app_route_source_for_route(
page.clone(),
path,
context_ssr,
project_path,
Expand All @@ -696,7 +690,7 @@ pub async fn create_app_source(
.collect();

if let Some(&Entrypoint::AppPage {
original_name: _,
page: _,
loader_tree,
}) = entrypoints.get("/_not-found")
{
Expand Down Expand Up @@ -769,7 +763,7 @@ async fn create_global_metadata_source(

#[turbo_tasks::function]
async fn create_app_page_source_for_route(
pathname: String,
page: AppPage,
loader_tree: Vc<LoaderTree>,
context_ssr: Vc<ModuleAssetContext>,
context: Vc<ModuleAssetContext>,
Expand All @@ -782,11 +776,12 @@ async fn create_app_page_source_for_route(
intermediate_output_path_root: Vc<FileSystemPath>,
render_data: Vc<JsonValue>,
) -> Result<Vc<Box<dyn ContentSource>>> {
let pathname_vc = Vc::cell(pathname.clone());
let app_path = AppPath::from(page.clone());
let pathname_vc = Vc::cell(app_path.to_string());

let params_matcher = NextParamsMatcher::new(pathname_vc);

let (base_segments, route_type) = pathname_to_segments(&pathname)?;
let (base_segments, route_type) = app_path_to_segments(&app_path)?;

let source = create_node_rendered_source(
project_path,
Expand Down Expand Up @@ -814,7 +809,7 @@ async fn create_app_page_source_for_route(
should_debug("app_source"),
);

Ok(source.issue_file_path(app_dir, format!("Next.js App Page Route {pathname}")))
Ok(source.issue_file_path(app_dir, format!("Next.js App Page Route {app_path}")))
}

#[turbo_tasks::function]
Expand Down Expand Up @@ -864,7 +859,7 @@ async fn create_app_not_found_page_source(

#[turbo_tasks::function]
async fn create_app_route_source_for_route(
pathname: String,
page: AppPage,
entry_path: Vc<FileSystemPath>,
context_ssr: Vc<ModuleAssetContext>,
project_path: Vc<FileSystemPath>,
Expand All @@ -875,11 +870,12 @@ async fn create_app_route_source_for_route(
intermediate_output_path_root: Vc<FileSystemPath>,
render_data: Vc<JsonValue>,
) -> Result<Vc<Box<dyn ContentSource>>> {
let pathname_vc = Vc::cell(pathname.to_string());
let app_path = AppPath::from(page.clone());
let pathname_vc = Vc::cell(app_path.to_string());

let params_matcher = NextParamsMatcher::new(pathname_vc);

let (base_segments, route_type) = pathname_to_segments(&pathname)?;
let (base_segments, route_type) = app_path_to_segments(&app_path)?;

let source = create_node_api_source(
project_path,
Expand All @@ -906,7 +902,7 @@ async fn create_app_route_source_for_route(
should_debug("app_source"),
);

Ok(source.issue_file_path(app_dir, format!("Next.js App Route {pathname}")))
Ok(source.issue_file_path(app_dir, format!("Next.js App Route {app_path}")))
}

/// The renderer for pages in app directory
Expand Down