From d0451eda090b385ba438f0c61400a16874f1d711 Mon Sep 17 00:00:00 2001 From: ShaneK Date: Wed, 17 Dec 2025 09:59:20 -0800 Subject: [PATCH] chore(react-router): lots of comments, code clean up, comment clean up, and a bit of ordering --- .../src/ReactRouter/IonRouter.tsx | 2 ++ .../src/ReactRouter/StackManager.tsx | 3 +- .../react-router/test/base/scripts/sync.sh | 3 ++ .../react-router/test/base/src/pages/Main.tsx | 8 +---- .../pages/dynamic-routes/DynamicRoutes.tsx | 1 - .../src/pages/muiltiple-tabs/MultipleTabs.tsx | 2 -- .../test/base/src/pages/refs/Refs.tsx | 1 - .../test/base/src/pages/routing/OtherPage.tsx | 4 --- .../test/base/tests/e2e/support/commands.js | 30 ++----------------- .../react/src/routing/OutletPageManager.tsx | 4 +-- packages/react/test/base/scripts/sync.sh | 3 ++ 11 files changed, 15 insertions(+), 46 deletions(-) diff --git a/packages/react-router/src/ReactRouter/IonRouter.tsx b/packages/react-router/src/ReactRouter/IonRouter.tsx index f1d475a733..83c2516ecf 100644 --- a/packages/react-router/src/ReactRouter/IonRouter.tsx +++ b/packages/react-router/src/ReactRouter/IonRouter.tsx @@ -104,6 +104,8 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr didMountRef.current = true; }, []); + // Sync route params extracted by React Router's path matching back into routeInfo. + // The view stack's match may contain params (e.g., :id) not present in the initial routeInfo. useEffect(() => { const activeView = viewStack.current.findViewItemByRouteInfo(routeInfo, undefined, true); const matchedParams = activeView?.routeData.match?.params as RouteParams | undefined; diff --git a/packages/react-router/src/ReactRouter/StackManager.tsx b/packages/react-router/src/ReactRouter/StackManager.tsx index 24fbba961f..bf819266b1 100644 --- a/packages/react-router/src/ReactRouter/StackManager.tsx +++ b/packages/react-router/src/ReactRouter/StackManager.tsx @@ -506,7 +506,8 @@ export class StackManager extends React.PureComponent { !leavingRoutePath.endsWith('/*') && !leavingViewItem.reactElement?.props?.index; - // Skip removal only for container-to-container transitions + // Skip removal for container-to-container transitions (e.g., /tabs/* → /settings/*). + // These routes manage their own nested outlets; unmounting would disrupt child views. if (isEnteringContainerRoute && !isLeavingSpecificRoute) { return; } diff --git a/packages/react-router/test/base/scripts/sync.sh b/packages/react-router/test/base/scripts/sync.sh index ad0d879c77..f44c34a44a 100644 --- a/packages/react-router/test/base/scripts/sync.sh +++ b/packages/react-router/test/base/scripts/sync.sh @@ -15,4 +15,7 @@ npm pack ../../../../react npm pack ../../../ # Install Dependencies +# TODO: Remove --legacy-peer-deps once @ionic/react peer deps align with test app versions. +# Currently needed because packed tarballs may have peer dep ranges that conflict with +# the specific React/React-Router versions in test apps. npm install *.tgz --no-save --legacy-peer-deps diff --git a/packages/react-router/test/base/src/pages/Main.tsx b/packages/react-router/test/base/src/pages/Main.tsx index 1a8ed4f107..c4032b4606 100644 --- a/packages/react-router/test/base/src/pages/Main.tsx +++ b/packages/react-router/test/base/src/pages/Main.tsx @@ -10,18 +10,12 @@ import { } from '@ionic/react'; import React from 'react'; -import packageJson from '../../package.json'; - const Main: React.FC = () => { - const majorVersion = packageJson.dependencies['react-router'].match( - /(\d+)\.(\d+)\.(\d+)/ - )?.[1]; - return ( - Test App - React Router v{majorVersion} + Main diff --git a/packages/react-router/test/base/src/pages/dynamic-routes/DynamicRoutes.tsx b/packages/react-router/test/base/src/pages/dynamic-routes/DynamicRoutes.tsx index f91da67b4f..4fb55e8417 100644 --- a/packages/react-router/test/base/src/pages/dynamic-routes/DynamicRoutes.tsx +++ b/packages/react-router/test/base/src/pages/dynamic-routes/DynamicRoutes.tsx @@ -30,7 +30,6 @@ const DynamicRoutes: React.FC = () => { return ( {routes} - {/* } /> */} } /> } /> diff --git a/packages/react-router/test/base/src/pages/muiltiple-tabs/MultipleTabs.tsx b/packages/react-router/test/base/src/pages/muiltiple-tabs/MultipleTabs.tsx index 5ccc4c371a..df7225cbbf 100644 --- a/packages/react-router/test/base/src/pages/muiltiple-tabs/MultipleTabs.tsx +++ b/packages/react-router/test/base/src/pages/muiltiple-tabs/MultipleTabs.tsx @@ -61,7 +61,6 @@ const Tab1: React.FC = () => { path="/multiple-tabs/tab1" element={} /> - {/* } /> */} } /> } /> @@ -87,7 +86,6 @@ const Tab2: React.FC = () => { path="/multiple-tabs/tab2" element={} /> - {/* } /> */} } /> } /> diff --git a/packages/react-router/test/base/src/pages/refs/Refs.tsx b/packages/react-router/test/base/src/pages/refs/Refs.tsx index 9b20848e9f..667f7be5f9 100644 --- a/packages/react-router/test/base/src/pages/refs/Refs.tsx +++ b/packages/react-router/test/base/src/pages/refs/Refs.tsx @@ -13,7 +13,6 @@ import { Route } from "react-router"; const Refs: React.FC = () => { return ( - {/* } /> */} } /> } /> diff --git a/packages/react-router/test/base/src/pages/routing/OtherPage.tsx b/packages/react-router/test/base/src/pages/routing/OtherPage.tsx index b1e452e9bb..a8c6fe67f6 100644 --- a/packages/react-router/test/base/src/pages/routing/OtherPage.tsx +++ b/packages/react-router/test/base/src/pages/routing/OtherPage.tsx @@ -22,8 +22,6 @@ const OtherPage: React.FC = () => { }, []); return ( - // - // @@ -37,8 +35,6 @@ const OtherPage: React.FC = () => { Go to tab3 - // }> - // ); }; diff --git a/packages/react-router/test/base/tests/e2e/support/commands.js b/packages/react-router/test/base/tests/e2e/support/commands.js index ec63d211c4..fa8ceb4c05 100644 --- a/packages/react-router/test/base/tests/e2e/support/commands.js +++ b/packages/react-router/test/base/tests/e2e/support/commands.js @@ -79,51 +79,25 @@ Cypress.Commands.add('ionSwipeToGoBack', (complete = false, selector = 'ion-rout }) Cypress.Commands.add('ionMenuNav', (contains) => { - // cy.get('ion-menu.show-menu').should('exist'); - // cy.wait(1000) cy.contains('ion-item', contains).click({ force: true }); cy.wait(250); - // cy.get('div.ion-page').click(); - // cy.get('ion-menu').then(menu => { - // cy.wait(1000) - // menu[0].isOpen(open => { - // if(open) { - // menu[0].toggle() - // } - // cy.get('ion-menu.show-menu').should('not.exist'); - // }) - // }) - // cy.get('ion-menu.show-menu').should('not.exist'); - - // cy.wait(1000) - // cy.wait(1000) - // cy.contains('ion-item', contains).click() - // cy.contains('ion-item', contains).parent('ion-menu-toggle').click({ force: true }); }); Cypress.Commands.add('ionTabClick', (tabText) => { // TODO FW-2800: figure out how to get rid of this wait. Switching tabs after a forward nav to a details page needs it cy.wait(500); cy.contains('ion-tab-button', tabText).click({ force: true }); - // cy.get('ion-tab-button.tab-selected').contains(tabText) }); Cypress.Commands.add('ionBackClick', (pageId) => { cy.get(`div.ion-page[data-pageid=${pageId}]`) .should('be.visible', true) - // .should('have.length', 1) .find('ion-back-button') .click(); }); -Cypress.Commands.add('ionMenuClick', () => { - // Todo: figure out how to get menu to close - // cy.get(`div.ion-page[aria-hidden!=true]`) - // .should('have.length', 1) - // .find('ion-menu-button') - // .click() - // cy.get('ion-menu.show-menu').should('exist'); -}); +// TODO: ionMenuClick is not implemented - figure out how to get menu to close +Cypress.Commands.add('ionMenuClick', () => {}); Cypress.Commands.add('ionHardwareBackEvent', () => { cy.document().trigger('backbutton'); diff --git a/packages/react/src/routing/OutletPageManager.tsx b/packages/react/src/routing/OutletPageManager.tsx index e970ca6774..2b98cf0ad9 100644 --- a/packages/react/src/routing/OutletPageManager.tsx +++ b/packages/react/src/routing/OutletPageManager.tsx @@ -90,10 +90,10 @@ export class OutletPageManager extends React.Component { {(context) => { this.ionLifeCycleContext = context; return ( - + (this.ionRouterOutlet = val)} id={id} + setRef={(val: HTMLIonRouterOutletElement) => (this.ionRouterOutlet = val)} {...props} > {children} diff --git a/packages/react/test/base/scripts/sync.sh b/packages/react/test/base/scripts/sync.sh index 0626fbd8f0..7c0955602b 100755 --- a/packages/react/test/base/scripts/sync.sh +++ b/packages/react/test/base/scripts/sync.sh @@ -15,4 +15,7 @@ npm pack ../../../ npm pack ../../../../react-router # Install Dependencies +# TODO: Remove --legacy-peer-deps once @ionic/react peer deps align with test app versions. +# Currently needed because packed tarballs may have peer dep ranges that conflict with +# the specific React/React-Router versions in test apps. npm install *.tgz --no-save --legacy-peer-deps