From 4064c64024a157b48adcabbf1db38011fe8873d6 Mon Sep 17 00:00:00 2001 From: OJ Kwon <1210596+kwonoj@users.noreply.github.com> Date: Mon, 18 Mar 2024 13:26:04 -0700 Subject: [PATCH] fix(next-core): carry over original segment config to metadata route (#63419) ### What Fixes to honor metadata route's segment config - turbopack replaces it into route handler, then parsed segment config so original segment config was ignored always. Closes PACK-2762 --- packages/next-swc/crates/next-api/src/app.rs | 1 + .../next-core/src/next_app/app_favicon_entry.rs | 1 + .../next-core/src/next_app/app_route_entry.rs | 11 ++++++++++- .../next-core/src/next_app/metadata/route.rs | 16 +++++++++++++++- test/turbopack-tests-manifest.json | 5 ++--- 5 files changed, 29 insertions(+), 5 deletions(-) diff --git a/packages/next-swc/crates/next-api/src/app.rs b/packages/next-swc/crates/next-api/src/app.rs index 8bafad0c6d6e..096879ef26f3 100644 --- a/packages/next-swc/crates/next-api/src/app.rs +++ b/packages/next-swc/crates/next-api/src/app.rs @@ -495,6 +495,7 @@ impl AppEndpoint { Vc::upcast(FileSource::new(path)), self.page.clone(), self.app_project.project().project_path(), + None, ) } diff --git a/packages/next-swc/crates/next-core/src/next_app/app_favicon_entry.rs b/packages/next-swc/crates/next-core/src/next_app/app_favicon_entry.rs index b72fc9af399a..4d7d54dabb2e 100644 --- a/packages/next-swc/crates/next-core/src/next_app/app_favicon_entry.rs +++ b/packages/next-swc/crates/next-core/src/next_app/app_favicon_entry.rs @@ -89,5 +89,6 @@ pub async fn get_app_route_favicon_entry( // TODO(alexkirsz) Get this from the metadata? AppPage(vec![PageSegment::Static("/favicon.ico".to_string())]), project_root, + None, )) } diff --git a/packages/next-swc/crates/next-core/src/next_app/app_route_entry.rs b/packages/next-swc/crates/next-core/src/next_app/app_route_entry.rs index 5cf997000e6d..47eaf58d9bed 100644 --- a/packages/next-swc/crates/next-core/src/next_app/app_route_entry.rs +++ b/packages/next-swc/crates/next-core/src/next_app/app_route_entry.rs @@ -16,6 +16,7 @@ use turbopack_binding::{ }; use crate::{ + app_segment_config::NextSegmentConfig, next_app::{AppEntry, AppPage, AppPath}, next_edge::entry::wrap_edge_entry, parse_segment_config_from_source, @@ -23,6 +24,12 @@ use crate::{ }; /// Computes the entry for a Next.js app route. +/// # Arguments +/// +/// * `original_segment_config` - A next segment config to be specified +/// explicitly for the given source. +/// For some cases `source` may not be the original but the handler (dynamic +/// metadata) which will lose segment config. #[turbo_tasks::function] pub async fn get_app_route_entry( nodejs_context: Vc, @@ -30,8 +37,10 @@ pub async fn get_app_route_entry( source: Vc>, page: AppPage, project_root: Vc, + original_segment_config: Option>, ) -> Result> { - let config = parse_segment_config_from_source(source); + let config = + original_segment_config.unwrap_or_else(|| parse_segment_config_from_source(source)); let is_edge = matches!(config.await?.runtime, Some(NextRuntime::Edge)); let context = if is_edge { edge_context diff --git a/packages/next-swc/crates/next-core/src/next_app/metadata/route.rs b/packages/next-swc/crates/next-core/src/next_app/metadata/route.rs index e7db3bc5a4a4..363a9449a953 100644 --- a/packages/next-swc/crates/next-core/src/next_app/metadata/route.rs +++ b/packages/next-swc/crates/next-core/src/next_app/metadata/route.rs @@ -9,7 +9,10 @@ use turbo_tasks::{ValueToString, Vc}; use turbopack_binding::{ turbo::tasks_fs::{File, FileContent, FileSystemPath}, turbopack::{ - core::{asset::AssetContent, source::Source, virtual_source::VirtualSource}, + core::{ + asset::AssetContent, file_source::FileSource, source::Source, + virtual_source::VirtualSource, + }, ecmascript::utils::StringifyJs, turbopack::ModuleAssetContext, }, @@ -20,6 +23,7 @@ use crate::{ app_structure::MetadataItem, mode::NextMode, next_app::{app_entry::AppEntry, app_route_entry::get_app_route_entry, AppPage, PageSegment}, + parse_segment_config_from_source, }; /// Computes the route source for a Next.js metadata file. @@ -55,12 +59,22 @@ pub fn get_app_metadata_route_entry( mode: NextMode, metadata: MetadataItem, ) -> Vc { + // Read original source's segment config before replacing source into + // dynamic|static metadata route handler. + let original_path = match metadata { + MetadataItem::Static { path } | MetadataItem::Dynamic { path } => path, + }; + + let source = Vc::upcast(FileSource::new(original_path)); + let config = parse_segment_config_from_source(source); + get_app_route_entry( nodejs_context, edge_context, get_app_metadata_route_source(page.clone(), mode, metadata), page, project_root, + Some(config), ) } diff --git a/test/turbopack-tests-manifest.json b/test/turbopack-tests-manifest.json index d319ba79fe10..b3bbb9ce3e9d 100644 --- a/test/turbopack-tests-manifest.json +++ b/test/turbopack-tests-manifest.json @@ -3774,11 +3774,10 @@ "app dir - metadata dynamic routes social image routes should support params as argument in dynamic routes", "app dir - metadata dynamic routes text routes should handle robots.[ext] dynamic routes", "app dir - metadata dynamic routes text routes should handle sitemap.[ext] dynamic routes", - "app dir - metadata dynamic routes text routes should not throw if client components are imported but not used" - ], - "failed": [ + "app dir - metadata dynamic routes text routes should not throw if client components are imported but not used", "app dir - metadata dynamic routes social image routes should handle custom fonts in both edge and nodejs runtime" ], + "failed": [], "pending": [], "flakey": [], "runtimeError": false