From fedca460a071e8928deaf6b007e50db41e80966f Mon Sep 17 00:00:00 2001 From: ShaneK Date: Thu, 4 Dec 2025 10:13:56 -0800 Subject: [PATCH] fix(react-router): preserve nested outlet params when navigating between sibling routes --- .../src/ReactRouter/IonRouter.tsx | 9 ++- .../src/ReactRouter/ReactRouterViewStack.tsx | 64 +++++++++++++---- .../src/pages/nested-params/NestedParams.tsx | 5 +- .../base/tests/e2e/specs/nested-params.cy.js | 70 +++++++++++++++++++ .../react/src/routing/OutletPageManager.tsx | 6 +- 5 files changed, 138 insertions(+), 16 deletions(-) create mode 100644 packages/react-router/test/base/tests/e2e/specs/nested-params.cy.js diff --git a/packages/react-router/src/ReactRouter/IonRouter.tsx b/packages/react-router/src/ReactRouter/IonRouter.tsx index 18700006aa..6954995602 100644 --- a/packages/react-router/src/ReactRouter/IonRouter.tsx +++ b/packages/react-router/src/ReactRouter/IonRouter.tsx @@ -6,7 +6,14 @@ * and animate. */ -import type { AnimationBuilder, RouteAction, RouteInfo, RouteManagerContextState, RouterDirection, RouterOptions } from '@ionic/react'; +import type { + AnimationBuilder, + RouteAction, + RouteInfo, + RouteManagerContextState, + RouterDirection, + RouterOptions, +} from '@ionic/react'; import { LocationHistory, NavManager, RouteManagerContext, generateId, getConfig } from '@ionic/react'; import type { Action as HistoryAction, Location } from 'history'; import type { PropsWithChildren } from 'react'; diff --git a/packages/react-router/src/ReactRouter/ReactRouterViewStack.tsx b/packages/react-router/src/ReactRouter/ReactRouterViewStack.tsx index 9aecba1334..32598cb22b 100644 --- a/packages/react-router/src/ReactRouter/ReactRouterViewStack.tsx +++ b/packages/react-router/src/ReactRouter/ReactRouterViewStack.tsx @@ -142,14 +142,24 @@ export class ReactRouterViewStack extends ViewStacks { // Special case: reuse tabs/* and other specific wildcard routes // Don't reuse index routes (empty path) or generic catch-all wildcards (*) if (existingPath === routePath && existingPath !== '' && existingPath !== '*') { - // For parameterized routes (containing :param), only reuse if the ACTUAL pathname matches - // This ensures /details/1 and /details/2 get separate view items and component instances + // Parameterized routes need pathname matching to ensure /details/1 and /details/2 + // get separate view items. For wildcard routes (e.g., user/:userId/*), compare + // pathnameBase to allow child path changes while preserving the parent view. const hasParams = routePath.includes(':'); + const isWildcard = routePath.includes('*'); if (hasParams) { - // Check if the existing view item's pathname matches the new pathname - const existingPathname = v.routeData?.match?.pathname; - if (existingPathname !== routeInfo.pathname) { - return false; // Different param values, don't reuse + if (isWildcard) { + const existingPathnameBase = v.routeData?.match?.pathnameBase; + const newMatch = matchComponent(reactElement, routeInfo.pathname, false); + const newPathnameBase = newMatch?.pathnameBase; + if (existingPathnameBase !== newPathnameBase) { + return false; + } + } else { + const existingPathname = v.routeData?.match?.pathname; + if (existingPathname !== routeInfo.pathname) { + return false; + } } } return true; @@ -339,13 +349,34 @@ export class ReactRouterViewStack extends ViewStacks { {(parentContext) => { const parentMatches = parentContext?.matches ?? []; - const accumulatedParentParams = parentMatches.reduce>( + let accumulatedParentParams = parentMatches.reduce>( (acc, match) => { return { ...acc, ...match.params }; }, {} ); + // If parentMatches is empty, try to extract params from view items in other outlets. + // This handles cases where React context propagation doesn't work as expected + // for nested router outlets. + if (parentMatches.length === 0 && Object.keys(accumulatedParentParams).length === 0) { + const allViewItems = this.getAllViewItems(); + for (const otherViewItem of allViewItems) { + // Skip view items from the same outlet + if (otherViewItem.outletId === viewItem.outletId) continue; + + // Check if this view item's route could match the current pathname + const otherMatch = otherViewItem.routeData?.match; + if (otherMatch && otherMatch.params && Object.keys(otherMatch.params).length > 0) { + // Check if the current pathname starts with this view item's matched pathname + const matchedPathname = otherMatch.pathnameBase || otherMatch.pathname; + if (matchedPathname && routeInfo.pathname.startsWith(matchedPathname)) { + accumulatedParentParams = { ...accumulatedParentParams, ...otherMatch.params }; + } + } + } + } + const combinedParams = { ...accumulatedParentParams, ...(routeMatch?.params ?? {}), @@ -620,6 +651,8 @@ export class ReactRouterViewStack extends ViewStacks { if (result) { const hasParams = result.params && Object.keys(result.params).length > 0; const isSamePath = result.pathname === previousMatch?.pathname; + const isWildcardRoute = viewItemPath.includes('*'); + const isParameterRoute = viewItemPath.includes(':'); // Don't allow view items with undefined paths to match specific routes // This prevents broken index route view items from interfering with navigation @@ -627,10 +660,18 @@ export class ReactRouterViewStack extends ViewStacks { return false; } - // For parameterized routes, never reuse if the pathname is different - // This ensures /details/1 and /details/2 get separate view items - const isParameterRoute = viewItemPath.includes(':'); + // For parameterized routes, check if we should reuse the view item. + // Wildcard routes (e.g., user/:userId/*) compare pathnameBase to allow + // child path changes while preserving the parent view. if (isParameterRoute && !isSamePath) { + if (isWildcardRoute) { + const isSameBase = result.pathnameBase === previousMatch?.pathnameBase; + if (isSameBase) { + match = result; + viewItem = v; + return true; + } + } return false; } @@ -642,8 +683,7 @@ export class ReactRouterViewStack extends ViewStacks { return true; } - // For wildcard routes, only reuse if the pathname exactly matches - const isWildcardRoute = viewItemPath.includes('*'); + // For wildcard routes (without params), only reuse if the pathname exactly matches if (isWildcardRoute && isSamePath) { match = result; viewItem = v; diff --git a/packages/react-router/test/base/src/pages/nested-params/NestedParams.tsx b/packages/react-router/test/base/src/pages/nested-params/NestedParams.tsx index 53bcda03cb..a92f525306 100644 --- a/packages/react-router/test/base/src/pages/nested-params/NestedParams.tsx +++ b/packages/react-router/test/base/src/pages/nested-params/NestedParams.tsx @@ -37,9 +37,12 @@ const Landing: React.FC = () => ( A nested route will try to read the parent :userId parameter. - + Go to User 42 Details + + Go to User 99 Details + ); diff --git a/packages/react-router/test/base/tests/e2e/specs/nested-params.cy.js b/packages/react-router/test/base/tests/e2e/specs/nested-params.cy.js new file mode 100644 index 0000000000..a52483192f --- /dev/null +++ b/packages/react-router/test/base/tests/e2e/specs/nested-params.cy.js @@ -0,0 +1,70 @@ +const port = 3000; + +describe('Nested Params', () => { + /* + Tests that route params are correctly passed to nested routes + when using parameterized wildcard routes (e.g., user/:userId/*). + */ + + it('/nested-params > Landing page should be visible', () => { + cy.visit(`http://localhost:${port}/nested-params`); + cy.ionPageVisible('nested-params-landing'); + }); + + it('/nested-params > Navigate to user details > Params should be available', () => { + cy.visit(`http://localhost:${port}/nested-params`); + cy.ionPageVisible('nested-params-landing'); + + cy.get('#go-to-user-42').click(); + + cy.get('[data-testid="user-layout-param"]').should('contain', 'Layout sees user: 42'); + cy.get('[data-testid="user-details-param"]').should('contain', 'Details view user: 42'); + }); + + it('/nested-params > Navigate between sibling routes > Params should be maintained', () => { + cy.visit(`http://localhost:${port}/nested-params`); + cy.ionPageVisible('nested-params-landing'); + + // Navigate to user 42 details + cy.get('#go-to-user-42').click(); + cy.get('[data-testid="user-layout-param"]').should('contain', 'Layout sees user: 42'); + cy.get('[data-testid="user-details-param"]').should('contain', 'Details view user: 42'); + + // Navigate to settings (sibling route) + cy.get('#go-to-settings').click(); + cy.get('[data-testid="user-layout-param"]').should('contain', 'Layout sees user: 42'); + cy.get('[data-testid="user-settings-param"]').should('contain', 'Settings view user: 42'); + + // Navigate back to details + cy.contains('ion-button', 'Back to Details').click(); + cy.get('[data-testid="user-layout-param"]').should('contain', 'Layout sees user: 42'); + cy.get('[data-testid="user-details-param"]').should('contain', 'Details view user: 42'); + }); + + it('/nested-params > Direct navigation to nested route > Params should be available', () => { + // Navigate directly to a nested route with params + cy.visit(`http://localhost:${port}/nested-params/user/123/settings`); + + cy.get('[data-testid="user-layout-param"]').should('contain', 'Layout sees user: 123'); + cy.get('[data-testid="user-settings-param"]').should('contain', 'Settings view user: 123'); + }); + + it('/nested-params > Different users should have different params', () => { + cy.visit(`http://localhost:${port}/nested-params`); + cy.ionPageVisible('nested-params-landing'); + + // Navigate to user 42 + cy.get('#go-to-user-42').click(); + cy.get('[data-testid="user-layout-param"]').should('contain', 'Layout sees user: 42'); + cy.get('[data-testid="user-details-param"]').should('contain', 'Details view user: 42'); + + // Go back to landing + cy.go('back'); + cy.ionPageVisible('nested-params-landing'); + + // Navigate to user 99 + cy.get('#go-to-user-99').click(); + cy.get('[data-testid="user-layout-param"]').should('contain', 'Layout sees user: 99'); + cy.get('[data-testid="user-details-param"]').should('contain', 'Details view user: 99'); + }); +}); diff --git a/packages/react/src/routing/OutletPageManager.tsx b/packages/react/src/routing/OutletPageManager.tsx index 0e0d1f9ea0..e970ca6774 100644 --- a/packages/react/src/routing/OutletPageManager.tsx +++ b/packages/react/src/routing/OutletPageManager.tsx @@ -12,6 +12,7 @@ interface OutletPageManagerProps { forwardedRef?: React.ForwardedRef; routeInfo?: RouteInfo; StackManager: any; // TODO(FW-2959): type + id?: string; } export class OutletPageManager extends React.Component { @@ -83,15 +84,16 @@ export class OutletPageManager extends React.Component { } render() { - const { StackManager, children, routeInfo, ...props } = this.props; + const { StackManager, children, routeInfo, id, ...props } = this.props; return ( {(context) => { this.ionLifeCycleContext = context; return ( - + (this.ionRouterOutlet = val)} + id={id} {...props} > {children}