Skip to content

Commit dea15ec

Browse files
gnoffgnoff
authored
Require canonical routes for interception routes (#97428)
Interception routes only describe the branch used during a soft navigation, but every URL they can produce still needs to be handled by an ordinary App Router page matcher for a direct request or refresh. Today an interception-only URL can appear to work during client navigation and then 404 when the same URL is loaded directly. This makes strict route matching reject that configuration. For example, this setup is incomplete: ``` app/ @modal/ (.)photo/ [...id]/ page.tsx photo/ [id]/ page.tsx ``` The ordinary route handles `/photo/1`, but the interception route also handles `/photo/1/2`, which has no ordinary page matcher. Widening the ordinary route makes the setup valid: ``` app/ @modal/ (.)photo/ [...id]/ page.tsx photo/ [...id]/ page.tsx ``` The validation checks coverage rather than requiring an identical matcher. A broader catchall can cover the intercepted pattern, and multiple ordinary matchers can cover different accepted lengths. For example, `items/page.tsx` together with `items/[...rest]/page.tsx` covers an intercepted `items/[[...parts]]` matcher. Route handlers and generated metadata routes do not count as canonical page matchers. Webpack and Turbopack now apply the same coverage check and report the interception matcher and the canonical pattern that is not fully covered. Existing interception fixtures now include semantically complete ordinary routes instead of relying on interception-only URLs. <!-- NEXT_JS_LLM -->
1 parent 7b7cee3 commit dea15ec

20 files changed

Lines changed: 683 additions & 57 deletions

File tree

‎crates/next-core/src/app_structure.rs‎

Lines changed: 83 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -992,6 +992,43 @@ async fn directory_tree_to_entrypoints(
992992
}
993993
}
994994

995+
let ordinary_routes = retained_entrypoints
996+
.iter()
997+
.filter_map(|(route, entrypoint)| {
998+
(!route.contains_interception() && matches!(entrypoint, Entrypoint::AppPage { .. }))
999+
.then_some(route)
1000+
})
1001+
.collect::<Vec<_>>();
1002+
let missing_canonical_interception_routes = retained_entrypoints
1003+
.iter()
1004+
.filter_map(|(interception_route, entrypoint)| {
1005+
let Entrypoint::AppPage {
1006+
participating_page_files,
1007+
..
1008+
} = entrypoint
1009+
else {
1010+
return None;
1011+
};
1012+
let canonical_route = interception_route.intercepted_path()?;
1013+
if canonical_route.is_route_pattern_covered_by(ordinary_routes.iter().copied()) {
1014+
return None;
1015+
}
1016+
1017+
Some((
1018+
interception_route.clone(),
1019+
canonical_route,
1020+
participating_page_files.first()?.clone(),
1021+
))
1022+
})
1023+
.collect::<Vec<_>>();
1024+
if !missing_canonical_interception_routes.is_empty() {
1025+
MissingCanonicalInterceptionRoutesIssue {
1026+
routes: missing_canonical_interception_routes,
1027+
}
1028+
.resolved_cell()
1029+
.emit();
1030+
}
1031+
9951032
let mut authored_pages = FxIndexSet::default();
9961033
collect_authored_page_files(&plain_tree, &mut authored_pages);
9971034

@@ -1021,6 +1058,52 @@ async fn directory_tree_to_entrypoints(
10211058
Ok(Vc::cell(retained_entrypoints))
10221059
}
10231060

1061+
#[turbo_tasks::value]
1062+
struct MissingCanonicalInterceptionRoutesIssue {
1063+
routes: Vec<(AppPath, AppPath, FileSystemPath)>,
1064+
}
1065+
1066+
#[async_trait]
1067+
#[turbo_tasks::value_impl]
1068+
impl Issue for MissingCanonicalInterceptionRoutesIssue {
1069+
async fn file_path(&self) -> Result<FileSystemPath> {
1070+
Ok(self.routes[0].2.clone())
1071+
}
1072+
1073+
fn stage(&self) -> IssueStage {
1074+
IssueStage::AppStructure
1075+
}
1076+
1077+
fn severity(&self) -> IssueSeverity {
1078+
IssueSeverity::Error
1079+
}
1080+
1081+
async fn title(&self) -> Result<StyledString> {
1082+
Ok(StyledString::Text(rcstr!(
1083+
"Interception routes must have a canonical route"
1084+
)))
1085+
}
1086+
1087+
async fn description(&self) -> Result<Option<StyledString>> {
1088+
let routes = self
1089+
.routes
1090+
.iter()
1091+
.map(|(interception_route, canonical_route, _)| {
1092+
format!("- {interception_route} (expected {canonical_route})")
1093+
})
1094+
.collect::<Vec<_>>()
1095+
.join("\n");
1096+
Ok(Some(StyledString::Text(
1097+
format!(
1098+
"The following interception routes do not have a canonical \
1099+
route:\n{routes}\n\nEvery interception route must have a matching \
1100+
non-interception route so the URL can be loaded directly or refreshed."
1101+
)
1102+
.into(),
1103+
)))
1104+
}
1105+
}
1106+
10241107
fn collect_authored_page_files(
10251108
directory_tree: &PlainDirectoryTree,
10261109
pages: &mut FxIndexSet<FileSystemPath>,

‎crates/next-core/src/next_app/mod.rs‎

Lines changed: 215 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -429,6 +429,165 @@ impl AppPath {
429429
)
430430
})
431431
}
432+
433+
/// Returns the ordinary route that an interception route substitutes for.
434+
/// This is the route that must handle a direct request or hard refresh.
435+
pub fn intercepted_path(&self) -> Option<AppPath> {
436+
let (interception_index, segment) =
437+
self.iter().enumerate().find_map(|(index, segment)| {
438+
let PathSegment::Static(segment) = segment else {
439+
return None;
440+
};
441+
442+
let (marker, target) =
443+
["(..)(..)", "(...)", "(..)", "(.)"]
444+
.into_iter()
445+
.find_map(|marker| {
446+
segment.strip_prefix(marker).map(|target| (marker, target))
447+
})?;
448+
Some((index, (marker, target)))
449+
})?;
450+
451+
let (marker, target) = segment;
452+
if target.is_empty() {
453+
return None;
454+
}
455+
456+
let mut canonical_segments = match marker {
457+
"(...)" => Vec::new(),
458+
_ => self.0[..interception_index].to_vec(),
459+
};
460+
let levels_to_pop = match marker {
461+
"(..)(..)" => 2,
462+
"(..)" => 1,
463+
_ => 0,
464+
};
465+
for _ in 0..levels_to_pop {
466+
canonical_segments.pop()?;
467+
}
468+
let target = if let Some(target) = target
469+
.strip_prefix("[[...")
470+
.and_then(|target| target.strip_suffix("]]"))
471+
{
472+
PathSegment::OptionalCatchAll(target.into())
473+
} else if let Some(target) = target
474+
.strip_prefix("[...")
475+
.and_then(|target| target.strip_suffix(']'))
476+
{
477+
PathSegment::CatchAll(target.into())
478+
} else if let Some(target) = target
479+
.strip_prefix('[')
480+
.and_then(|target| target.strip_suffix(']'))
481+
{
482+
PathSegment::Dynamic(target.into())
483+
} else {
484+
PathSegment::Static(target.into())
485+
};
486+
canonical_segments.push(target);
487+
canonical_segments.extend_from_slice(&self.0[interception_index + 1..]);
488+
489+
Some(AppPath(canonical_segments))
490+
}
491+
492+
/// Returns whether the supplied ordinary route patterns cover every URL matched by this
493+
/// route pattern.
494+
pub fn is_route_pattern_covered_by<'a>(
495+
&self,
496+
ordinary_routes: impl IntoIterator<Item = &'a AppPath>,
497+
) -> bool {
498+
let route = RoutePattern::new(self);
499+
let ordinary_routes = ordinary_routes
500+
.into_iter()
501+
.map(RoutePattern::new)
502+
.collect::<Vec<_>>();
503+
504+
if !route.unbounded {
505+
return ordinary_routes
506+
.iter()
507+
.any(|ordinary| pattern_covers_at_length(ordinary, &route, route.min_length));
508+
}
509+
510+
let Some(unbounded_coverage_start) = ordinary_routes
511+
.iter()
512+
.filter(|ordinary| ordinary.unbounded && prefix_covers(ordinary, &route))
513+
.map(|ordinary| ordinary.min_length.max(route.min_length))
514+
.min()
515+
else {
516+
return false;
517+
};
518+
519+
// An optional or required catchall can have its shorter paths covered by fixed routes
520+
// before another catchall takes over the remaining suffix.
521+
(route.min_length..unbounded_coverage_start).all(|length| {
522+
ordinary_routes
523+
.iter()
524+
.any(|ordinary| pattern_covers_at_length(ordinary, &route, length))
525+
})
526+
}
527+
}
528+
529+
struct RoutePattern<'a> {
530+
prefix: &'a [PathSegment],
531+
min_length: usize,
532+
unbounded: bool,
533+
}
534+
535+
impl<'a> RoutePattern<'a> {
536+
fn new(path: &'a AppPath) -> Self {
537+
match path.last() {
538+
Some(PathSegment::CatchAll(_)) => Self {
539+
prefix: &path[..path.len() - 1],
540+
min_length: path.len(),
541+
unbounded: true,
542+
},
543+
Some(PathSegment::OptionalCatchAll(_)) => Self {
544+
prefix: &path[..path.len() - 1],
545+
min_length: path.len() - 1,
546+
unbounded: true,
547+
},
548+
_ => Self {
549+
prefix: path,
550+
min_length: path.len(),
551+
unbounded: false,
552+
},
553+
}
554+
}
555+
}
556+
557+
fn accepts_length(pattern: &RoutePattern<'_>, length: usize) -> bool {
558+
if pattern.unbounded {
559+
length >= pattern.min_length
560+
} else {
561+
length == pattern.min_length
562+
}
563+
}
564+
565+
fn prefix_covers(canonical: &RoutePattern<'_>, intercepted: &RoutePattern<'_>) -> bool {
566+
canonical
567+
.prefix
568+
.iter()
569+
.enumerate()
570+
.all(|(index, canonical_segment)| match canonical_segment {
571+
PathSegment::Dynamic(_) => true,
572+
PathSegment::Static(canonical_segment) => matches!(
573+
intercepted.prefix.get(index),
574+
Some(PathSegment::Static(intercepted_segment))
575+
if canonical_segment == intercepted_segment
576+
),
577+
PathSegment::CatchAll(_) | PathSegment::OptionalCatchAll(_) => {
578+
unreachable!("catchall segments are excluded from the route prefix")
579+
}
580+
})
581+
}
582+
583+
fn pattern_covers_at_length(
584+
canonical: &RoutePattern<'_>,
585+
intercepted: &RoutePattern<'_>,
586+
length: usize,
587+
) -> bool {
588+
accepts_length(canonical, length)
589+
&& accepts_length(intercepted, length)
590+
&& prefix_covers(canonical, intercepted)
432591
}
433592

434593
impl Deref for AppPath {
@@ -489,7 +648,62 @@ impl From<AppPage> for AppPath {
489648

490649
#[cfg(test)]
491650
mod test {
492-
use crate::next_app::{AppPage, PageSegment, PageType};
651+
use crate::next_app::{AppPage, AppPath, PageSegment, PageType};
652+
653+
#[test]
654+
fn resolves_intercepted_app_paths() {
655+
for (interception, canonical) in [
656+
("/(.)photo/[id]", "/photo/[id]"),
657+
("/feed/(.)photo/[id]", "/feed/photo/[id]"),
658+
("/feed/(..)photo/[id]", "/photo/[id]"),
659+
("/feed/nested/(..)(..)photo/[id]", "/photo/[id]"),
660+
("/feed/(...)photo/[id]", "/photo/[id]"),
661+
("/(.)[username]/[id]", "/[username]/[id]"),
662+
("/(.)[...slug]", "/[...slug]"),
663+
("/(.)[[...slug]]", "/[[...slug]]"),
664+
] {
665+
let interception =
666+
AppPath::from(AppPage::parse(interception.trim_start_matches('/')).unwrap());
667+
let canonical =
668+
AppPath::from(AppPage::parse(canonical.trim_start_matches('/')).unwrap());
669+
670+
assert_eq!(interception.intercepted_path(), Some(canonical));
671+
}
672+
}
673+
674+
#[test]
675+
fn checks_route_pattern_coverage() {
676+
for (route, ordinary_routes, expected) in [
677+
("/photo/[id]", &["/photo/[slug]"][..], true),
678+
("/showcase/[...parts]", &["/[...slug]"][..], true),
679+
(
680+
"/items/[...parts]",
681+
&["/items/[id]", "/items/[id]/[...rest]"][..],
682+
true,
683+
),
684+
(
685+
"/items/[[...parts]]",
686+
&["/items", "/items/[...rest]"][..],
687+
true,
688+
),
689+
("/photo/[id]", &[][..], false),
690+
("/items/[...parts]", &["/items/[id]"][..], false),
691+
("/items/[[...parts]]", &["/items/[...rest]"][..], false),
692+
("/items/[id]", &["/items/one", "/items/two"][..], false),
693+
] {
694+
let route = AppPath::from(AppPage::parse(route.trim_start_matches('/')).unwrap());
695+
let ordinary_routes = ordinary_routes
696+
.iter()
697+
.map(|route| AppPath::from(AppPage::parse(route.trim_start_matches('/')).unwrap()))
698+
.collect::<Vec<_>>();
699+
700+
assert_eq!(
701+
route.is_route_pattern_covered_by(ordinary_routes.iter()),
702+
expected,
703+
"coverage for {route} from {ordinary_routes:?}"
704+
);
705+
}
706+
}
493707

494708
#[test]
495709
fn test_normalize_metadata_route() {

‎packages/next/src/build/entries.ts‎

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -47,6 +47,7 @@ import {
4747
import { encodeMatchers } from './webpack/loaders/next-middleware-loader'
4848
import type { EdgeFunctionLoaderOptions } from './webpack/loaders/next-edge-function-loader'
4949
import { isAppRouteRoute } from '../lib/is-app-route-route'
50+
import { isAppPageRoute } from '../lib/is-app-page-route'
5051
import { getRouteLoaderEntry } from './webpack/loaders/next-route-loader'
5152
import {
5253
isInternalComponent,
@@ -59,6 +60,8 @@ import type { PageExtensions } from './page-extensions-type'
5960
import type { MappedPages } from './build-context'
6061
import { PAGE_TYPES } from '../lib/page-types'
6162
import { UnmatchedAppPagesError } from '../shared/lib/errors/unmatched-app-pages-error'
63+
import { MissingCanonicalInterceptionRoutesError } from '../shared/lib/errors/missing-canonical-interception-routes-error'
64+
import { findMissingCanonicalInterceptionRoutes } from '../shared/lib/router/utils/interception-routes'
6265

6366
type ObjectValue<T> = T extends { [key: string]: infer V } ? V : never
6467
import { getStaticInfoIncludingLayouts } from './get-static-info-including-layouts'
@@ -431,6 +434,24 @@ export async function createEntrypoints(
431434
strictRouteMatching: config.experimental.strictRouteMatching,
432435
defaultAppPaths: Object.keys(appDefaultPaths ?? {}),
433436
})
437+
// Only App Router pages can make an intercepted URL directly renderable.
438+
// Route handlers and metadata routes may share the pathname, but they
439+
// cannot provide the canonical page shown by an initial request.
440+
const appPagePathsPerRoute = Object.fromEntries(
441+
Object.entries(appPathsPerRoute).flatMap(([route, routeAppPaths]) => {
442+
const pageAppPaths = routeAppPaths.filter(isAppPageRoute)
443+
return pageAppPaths.length > 0 ? [[route, pageAppPaths]] : []
444+
})
445+
)
446+
const missingCanonicalInterceptionRoutes = config.experimental
447+
.strictRouteMatching
448+
? findMissingCanonicalInterceptionRoutes(appPagePathsPerRoute)
449+
: []
450+
if (missingCanonicalInterceptionRoutes.length > 0) {
451+
throw new MissingCanonicalInterceptionRoutesError(
452+
missingCanonicalInterceptionRoutes
453+
)
454+
}
434455
if (unmatchedAppPages.length > 0) {
435456
throw new UnmatchedAppPagesError(
436457
unmatchedAppPages.map((appPath) => {

0 commit comments

Comments
 (0)