diff --git a/packages/react-router/src/ReactRouter/ReactRouterViewStack.tsx b/packages/react-router/src/ReactRouter/ReactRouterViewStack.tsx index 35e80fa5f6..b0eaf5e270 100644 --- a/packages/react-router/src/ReactRouter/ReactRouterViewStack.tsx +++ b/packages/react-router/src/ReactRouter/ReactRouterViewStack.tsx @@ -452,12 +452,35 @@ export class ReactRouterViewStack extends ViewStacks { if (routePath === '*' || routePath === '') { // Check if any other view in this outlet has a match for the current route const outletViews = this.getViewItemsForOutlet(viewItem.outletId); + + // When parent path context is available, compute the relative pathname once + // outside the loop since both routeInfo.pathname and parentPath are invariant. + const relativePathname = parentPath + ? computeRelativeToParent(routeInfo.pathname, parentPath) + : null; + let hasSpecificMatch = outletViews.some((v) => { if (v.id === viewItem.id) return false; // Skip self const vRoutePath = v.reactElement?.props?.path || ''; if (vRoutePath === '*' || vRoutePath === '') return false; // Skip other wildcard/empty routes - // Check if this view item would match the current route + // When parent path context is available and the route is relative, use + // parent-path-aware matching. This avoids false positives from + // derivePathnameToMatch's tail-slice heuristic, which can incorrectly + // match route literals that appear at the wrong position in the pathname. + // Example: pathname /parent/extra/details/99 with route details/:id — + // the tail-slice extracts ["details","99"] producing a false match. + if (parentPath && vRoutePath && !vRoutePath.startsWith('/')) { + if (relativePathname === null) { + return false; // Pathname is outside this outlet's parent scope + } + return !!matchPath({ + pathname: relativePathname, + componentProps: v.reactElement.props, + }); + } + + // Fallback to matchComponent when no parent path context is available const vMatch = v.reactElement ? matchComponent(v.reactElement, routeInfo.pathname) : null; return !!vMatch; }); diff --git a/packages/react-router/src/ReactRouter/utils/computeParentPath.ts b/packages/react-router/src/ReactRouter/utils/computeParentPath.ts index 5af01a3e64..8e6a73507f 100644 --- a/packages/react-router/src/ReactRouter/utils/computeParentPath.ts +++ b/packages/react-router/src/ReactRouter/utils/computeParentPath.ts @@ -68,17 +68,6 @@ const matchesEmbeddedWildcardRoute = (route: React.ReactElement, pathname: strin return !!matchPath({ pathname, componentProps: route.props }); }; -/** - * Checks if a route path consists entirely of parameterized segments (e.g., ":slug", ":category/:id"). - * These routes match any single segment and should not drive the parent path deeper - * than the outlet's established mount point. - */ -const isPurelyParameterized = (routePath: string | undefined): boolean => { - if (!routePath) return false; - const segments = routePath.split('/').filter(Boolean); - return segments.length > 0 && segments.every((segment) => segment.startsWith(':')); -}; - /** * Checks if a route is a specific match (not wildcard-only or index). */ @@ -180,12 +169,21 @@ const couldSpecificRouteMatch = ( // When mount path is established, skip this lookahead: the parent depth is known, // and purely parameterized routes (e.g., :slug) matching the last segment should // not prevent the wildcard from claiming the full remaining path. + // + // Only allow purely literal routes (no :params) in the lookahead. Routes with + // parameters are positionally ambiguous — their params match any segment, so a + // coincidental match at a deeper level (e.g., details/:id matching "details/99" + // inside "extra/details/99") should not prevent the wildcard from capturing the + // full remaining path. if (!outletMountPath) { for (let j = 1; j < segments.length; j++) { const futureRemaining = segments.slice(j).join('/'); const futureMatch = findFirstSpecificMatchingRoute(routeChildren, futureRemaining); - if (futureMatch && !isPurelyParameterized(futureMatch.props.path as string)) { - return true; + if (futureMatch) { + const futurePath = futureMatch.props.path as string | undefined; + if (futurePath && !futurePath.includes(':')) { + return true; + } } } } @@ -314,13 +312,12 @@ export const computeParentPath = (options: ComputeParentPathOptions): ParentPath // Check for specific route match (highest priority) if (!firstSpecificMatch && findSpecificMatch(routeChildren, remainingPath)) { - // Don't let purely parameterized routes (e.g., :slug, :id) drive the - // parent deeper than where a wildcard already matched. A :slug route - // matching the last segment of "deep/nested/path" shouldn't pull the - // parent to /parent/deep/nested — the wildcard at the correct depth - // should catch the full remaining path instead. - // Literal routes (e.g., "settings", "redirect") can still match beyond - // the wildcard depth to support redirect scenarios. + // Don't let routes containing parameter segments (e.g., :slug, details/:id) + // drive the parent deeper than where a wildcard already matched. Parameter + // segments match any value, making tail-slice matches positionally ambiguous: + // e.g., "details/:id" matching "details/99" inside "extra/details/99" is a + // coincidental match at the wrong depth. Only purely literal routes (e.g., + // "settings", "redirect") can override the wildcard at deeper levels. // // Also don't let empty/default path routes (path="" or undefined) drive // the parent deeper than a wildcard match. An empty path route matching @@ -334,7 +331,13 @@ export const computeParentPath = (options: ComputeParentPathOptions): ParentPath if (matchingRoute) { const matchingPath = matchingRoute.props.path as string | undefined; const isEmptyPath = !matchingPath || matchingPath === ''; - if (shouldSkipParameterized && (isPurelyParameterized(matchingPath as string) || isEmptyPath)) { + // When the parent path is deeper than expected (shouldSkipParameterized), + // skip routes containing ANY parameterized segments. Parameters make tail- + // slice matches positionally ambiguous: e.g., "details/:id" matching + // "details/99" inside "extra/details/99" is a coincidental match at the + // wrong depth. Only purely literal routes (e.g., "settings") can override + // the wildcard at deeper levels. + if (shouldSkipParameterized && (matchingPath?.includes(':') || isEmptyPath)) { continue; } if (firstWildcardMatch && isEmptyPath) { diff --git a/packages/react-router/test/base/src/App.tsx b/packages/react-router/test/base/src/App.tsx index 426b6df34d..95db08ac35 100644 --- a/packages/react-router/test/base/src/App.tsx +++ b/packages/react-router/test/base/src/App.tsx @@ -52,6 +52,7 @@ import PrefixMatchWildcard from './pages/prefix-match-wildcard/PrefixMatchWildca import StaleViewCleanup from './pages/stale-view-cleanup/StaleViewCleanup'; import IndexParamPriority from './pages/index-param-priority/IndexParamPriority'; import IndexRouteReuse from './pages/index-route-reuse/IndexRouteReuse'; +import TailSliceAmbiguity from './pages/tail-slice-ambiguity/TailSliceAmbiguity'; setupIonicReact(); @@ -93,6 +94,7 @@ const App: React.FC = () => { } /> } /> } /> + } /> diff --git a/packages/react-router/test/base/src/pages/Main.tsx b/packages/react-router/test/base/src/pages/Main.tsx index d8bbbcfeab..aa6f5ec9f2 100644 --- a/packages/react-router/test/base/src/pages/Main.tsx +++ b/packages/react-router/test/base/src/pages/Main.tsx @@ -101,6 +101,9 @@ const Main: React.FC = () => { Index Route Reuse + + Tail Slice Ambiguity + diff --git a/packages/react-router/test/base/src/pages/tail-slice-ambiguity/TailSliceAmbiguity.tsx b/packages/react-router/test/base/src/pages/tail-slice-ambiguity/TailSliceAmbiguity.tsx new file mode 100644 index 0000000000..25d80307bc --- /dev/null +++ b/packages/react-router/test/base/src/pages/tail-slice-ambiguity/TailSliceAmbiguity.tsx @@ -0,0 +1,97 @@ +import { + IonButton, + IonContent, + IonHeader, + IonLabel, + IonPage, + IonRouterOutlet, + IonTitle, + IonToolbar, +} from '@ionic/react'; +import React from 'react'; +import { Route, useLocation, useParams } from 'react-router-dom'; + +/** + * Test page for tail-slice ambiguity in derivePathnameToMatch. + * + * Route structure: + * /tail-slice-ambiguity/* + * └── + * ├── index → ListPage + * ├── details/:id → DetailsPage + * └── * → CatchAllPage + * + * Bug scenario: + * 1. Navigate to /tail-slice-ambiguity/details/42 → creates details/:id view + * 2. Navigate to /tail-slice-ambiguity/extra/details/99 + * 3. derivePathnameToMatch tail-slices the last 2 segments ["details", "99"] + * and matchComponent falsely matches details/:id + * 4. The catch-all * view is incorrectly deactivated because hasSpecificMatch + * finds the false positive from details/:id + * 5. User sees nothing instead of the catch-all page + */ +const TailSliceAmbiguity: React.FC = () => ( + + } /> + } /> + } /> + +); + +const ListPage: React.FC = () => ( + + + + List + + + + + Details 42 + + + Ambiguous Path + + + +); + +const DetailsPage: React.FC = () => { + const { id } = useParams<{ id: string }>(); + return ( + + + + Details + + + + Details ID: {id} + + Back to List + + + + ); +}; + +const CatchAllPage: React.FC = () => { + const location = useLocation(); + return ( + + + + Catch All + + + + Catch-all: {location.pathname} + + Back to List + + + + ); +}; + +export default TailSliceAmbiguity; diff --git a/packages/react-router/test/base/tests/e2e/specs/tail-slice-ambiguity.cy.js b/packages/react-router/test/base/tests/e2e/specs/tail-slice-ambiguity.cy.js new file mode 100644 index 0000000000..8fbb428921 --- /dev/null +++ b/packages/react-router/test/base/tests/e2e/specs/tail-slice-ambiguity.cy.js @@ -0,0 +1,74 @@ +const port = 3000; + +/** + * Tests that derivePathnameToMatch's tail-slice heuristic does not produce + * false positive matches that incorrectly deactivate catch-all routes. + * + * Route structure: + * /tail-slice-ambiguity/* + * ├── index → ListPage + * ├── details/:id → DetailsPage + * └── * → CatchAllPage + * + * Bug: navigating to /tail-slice-ambiguity/extra/details/99 after visiting + * /tail-slice-ambiguity/details/42 causes the tail-slice to extract + * ["details", "99"] which falsely matches details/:id, deactivating the + * catch-all page. + */ +describe('Tail-Slice Ambiguity', () => { + it('should show details page for /details/:id', () => { + cy.visit(`http://localhost:${port}/tail-slice-ambiguity`); + cy.ionPageVisible('tail-slice-list'); + + cy.get('#go-to-details').click(); + cy.ionPageVisible('tail-slice-details'); + cy.get('[data-testid="details-id"]').should('contain', 'Details ID: 42'); + }); + + it('should show catch-all when path has extra segments before details', () => { + cy.visit(`http://localhost:${port}/tail-slice-ambiguity`); + cy.ionPageVisible('tail-slice-list'); + + // First create the details/:id view + cy.get('#go-to-details').click(); + cy.ionPageVisible('tail-slice-details'); + cy.get('[data-testid="details-id"]').should('contain', 'Details ID: 42'); + + // Go back to list + cy.get('#back-to-list').click(); + cy.ionPageVisible('tail-slice-list'); + + // Navigate to ambiguous path - should show catch-all, NOT details + cy.get('#go-to-ambiguous').click(); + cy.ionPageVisible('tail-slice-catchall'); + cy.get('[data-testid="catchall-path"]').should('contain', '/tail-slice-ambiguity/extra/details/99'); + }); + + it('should show catch-all for ambiguous path on direct navigation', () => { + cy.visit(`http://localhost:${port}/tail-slice-ambiguity/extra/details/99`); + cy.ionPageVisible('tail-slice-catchall'); + cy.get('[data-testid="catchall-path"]').should('contain', '/tail-slice-ambiguity/extra/details/99'); + }); + + it('should correctly navigate: details → list → ambiguous → back to list', () => { + cy.visit(`http://localhost:${port}/tail-slice-ambiguity`); + cy.ionPageVisible('tail-slice-list'); + + // Visit details + cy.get('#go-to-details').click(); + cy.ionPageVisible('tail-slice-details'); + cy.get('[data-testid="details-id"]').should('contain', 'Details ID: 42'); + + // Back to list + cy.get('#back-to-list').click(); + cy.ionPageVisible('tail-slice-list'); + + // Visit ambiguous path (catch-all) + cy.get('#go-to-ambiguous').click(); + cy.ionPageVisible('tail-slice-catchall'); + + // Back to list via browser back + cy.go('back'); + cy.ionPageVisible('tail-slice-list'); + }); +});