Skip to content

Commit 36c6b13

Browse files
ShaneKShaneK
committed
fix(react-router): reject orphaned IonPage registrations during navigation
1 parent 60cedf0 commit 36c6b13

5 files changed

Lines changed: 227 additions & 0 deletions

File tree

‎packages/react-router/src/ReactRouter/StackManager.tsx‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -657,9 +657,25 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
657657
this.ionPageWaitTimeout = undefined;
658658
}
659659
this.pendingPageTransition = false;
660+
660661
const foundView = this.context.findViewItemByRouteInfo(routeInfo, this.id);
661662
if (foundView) {
662663
const oldPageElement = foundView.ionPageElement;
664+
665+
/**
666+
* FIX for issue #28878: Reject orphaned IonPage registrations.
667+
*
668+
* When a component conditionally renders different IonPages (e.g., list vs empty state)
669+
* using React keys, and state changes simultaneously with navigation, the new IonPage
670+
* tries to register for a route we're navigating away from. This creates a stale view.
671+
*
672+
* Only reject if both pageIds exist and differ, to allow nested outlet registrations.
673+
*/
674+
if (this.shouldRejectOrphanedPage(page, oldPageElement, routeInfo)) {
675+
this.hideAndRemoveOrphanedPage(page);
676+
return;
677+
}
678+
663679
foundView.ionPageElement = page;
664680
foundView.ionRoute = true;
665681

@@ -675,6 +691,45 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
675691
this.handlePageTransition(routeInfo);
676692
}
677693

694+
/**
695+
* Determines if a new IonPage registration should be rejected as orphaned.
696+
* This happens when a component re-renders with a different IonPage while navigating away.
697+
*/
698+
private shouldRejectOrphanedPage(
699+
newPage: HTMLElement,
700+
oldPageElement: HTMLElement | undefined,
701+
routeInfo: RouteInfo
702+
): boolean {
703+
if (!oldPageElement || oldPageElement === newPage) {
704+
return false;
705+
}
706+
707+
const newPageId = newPage.getAttribute('data-pageid');
708+
const oldPageId = oldPageElement.getAttribute('data-pageid');
709+
710+
// Only reject if both pageIds exist and are different
711+
if (!newPageId || !oldPageId || newPageId === oldPageId) {
712+
return false;
713+
}
714+
715+
// Reject only if we're navigating away from this route
716+
return this.props.routeInfo.pathname !== routeInfo.pathname;
717+
}
718+
719+
/**
720+
* Hides an orphaned IonPage and schedules its removal from the DOM.
721+
*/
722+
private hideAndRemoveOrphanedPage(page: HTMLElement): void {
723+
page.classList.add('ion-page-hidden');
724+
page.setAttribute('aria-hidden', 'true');
725+
726+
setTimeout(() => {
727+
if (page.parentElement) {
728+
page.remove();
729+
}
730+
}, VIEW_UNMOUNT_DELAY_MS);
731+
}
732+
678733
/**
679734
* Configures the router outlet for the swipe-to-go-back gesture.
680735
*

‎packages/react-router/test/base/src/App.tsx‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,7 @@ import TabHistoryIsolation from './pages/tab-history-isolation/TabHistoryIsolati
4545
import Overlays from './pages/overlays/Overlays';
4646
import NestedTabsRelativeLinks from './pages/nested-tabs-relative-links/NestedTabsRelativeLinks';
4747
import RootSplatTabs from './pages/root-splat-tabs/RootSplatTabs';
48+
import ContentChangeNavigation from './pages/content-change-navigation/ContentChangeNavigation';
4849

4950
setupIonicReact();
5051

@@ -79,6 +80,7 @@ const App: React.FC = () => {
7980
<Route path="relative-paths/*" element={<RelativePaths />} />
8081
<Route path="/nested-tabs-relative-links/*" element={<NestedTabsRelativeLinks />} />
8182
<Route path="/root-splat-tabs/*" element={<RootSplatTabs />} />
83+
<Route path="/content-change-navigation/*" element={<ContentChangeNavigation />} />
8284
</IonRouterOutlet>
8385
</IonReactRouter>
8486
</IonApp>

‎packages/react-router/test/base/src/pages/Main.tsx‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,9 @@ const Main: React.FC = () => {
8686
<IonItem routerLink="/root-splat-tabs">
8787
<IonLabel>Root Splat Tabs</IonLabel>
8888
</IonItem>
89+
<IonItem routerLink="/content-change-navigation">
90+
<IonLabel>Content Change Navigation</IonLabel>
91+
</IonItem>
8992
</IonList>
9093
</IonContent>
9194
</IonPage>

‎packages/react-router/test/base/src/pages/content-change-navigation/ContentChangeNavigation.tsx‎

Lines changed: 112 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,112 @@
1+
/**
2+
* Test case for GitHub issue #28878
3+
* https://github.com/ionic-team/ionic-framework/issues/28878
4+
*
5+
* Reproduces the bug where changing view content while navigating causes
6+
* an invalid view to be rendered.
7+
*/
8+
9+
import {
10+
IonContent,
11+
IonHeader,
12+
IonPage,
13+
IonTitle,
14+
IonToolbar,
15+
IonList,
16+
IonItem,
17+
IonLabel,
18+
IonButton,
19+
IonRouterOutlet,
20+
IonButtons,
21+
IonBackButton,
22+
} from '@ionic/react';
23+
import React, { useState } from 'react';
24+
import { Route, Navigate, useNavigate } from 'react-router-dom';
25+
26+
const ListPage: React.FC = () => {
27+
const [items, setItems] = useState(['Item 1', 'Item 2', 'Item 3']);
28+
const navigate = useNavigate();
29+
30+
const clearItemsAndNavigate = () => {
31+
setItems([]);
32+
navigate('/content-change-navigation/home');
33+
};
34+
35+
// Using different keys forces React to unmount/remount IonPage
36+
if (items.length === 0) {
37+
return (
38+
<IonPage key="empty" data-pageid="list-empty-page">
39+
<IonHeader>
40+
<IonToolbar>
41+
<IonButtons slot="start">
42+
<IonBackButton defaultHref="/content-change-navigation/home" />
43+
</IonButtons>
44+
<IonTitle>Empty List</IonTitle>
45+
</IonToolbar>
46+
</IonHeader>
47+
<IonContent>
48+
<div data-testid="empty-view">There are no items</div>
49+
<IonButton routerLink="/content-change-navigation/home" data-testid="go-home-from-empty">
50+
Go Home
51+
</IonButton>
52+
</IonContent>
53+
</IonPage>
54+
);
55+
}
56+
57+
return (
58+
<IonPage key="list" data-pageid="list-page">
59+
<IonHeader>
60+
<IonToolbar>
61+
<IonButtons slot="start">
62+
<IonBackButton defaultHref="/content-change-navigation/home" />
63+
</IonButtons>
64+
<IonTitle>Item List</IonTitle>
65+
</IonToolbar>
66+
</IonHeader>
67+
<IonContent>
68+
<IonList>
69+
{items.map((item, index) => (
70+
<IonItem key={index}>
71+
<IonLabel>{item}</IonLabel>
72+
</IonItem>
73+
))}
74+
</IonList>
75+
<br />
76+
<IonButton onClick={clearItemsAndNavigate} data-testid="clear-and-navigate">
77+
Remove all items and navigate to home
78+
</IonButton>
79+
</IonContent>
80+
</IonPage>
81+
);
82+
};
83+
84+
const HomePage: React.FC = () => {
85+
return (
86+
<IonPage data-pageid="content-nav-home">
87+
<IonHeader>
88+
<IonToolbar>
89+
<IonTitle>Home</IonTitle>
90+
</IonToolbar>
91+
</IonHeader>
92+
<IonContent>
93+
<div data-testid="home-content">Home Page Content</div>
94+
<IonButton routerLink="/content-change-navigation/list" data-testid="go-to-list">
95+
Go to list page
96+
</IonButton>
97+
</IonContent>
98+
</IonPage>
99+
);
100+
};
101+
102+
const ContentChangeNavigation: React.FC = () => {
103+
return (
104+
<IonRouterOutlet>
105+
<Route index element={<Navigate to="home" replace />} />
106+
<Route path="home" element={<HomePage />} />
107+
<Route path="list" element={<ListPage />} />
108+
</IonRouterOutlet>
109+
);
110+
};
111+
112+
export default ContentChangeNavigation;

‎packages/react-router/test/base/tests/e2e/specs/content-change-navigation.cy.js‎

Lines changed: 55 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,55 @@
1+
/**
2+
* Tests for GitHub issue #28878
3+
* https://github.com/ionic-team/ionic-framework/issues/28878
4+
*
5+
* Verifies that when view content changes (causing IonPage to remount)
6+
* while navigation is happening, the correct view is displayed.
7+
*/
8+
9+
const port = 3000;
10+
11+
describe('Content Change Navigation Tests (Issue #28878)', () => {
12+
it('should navigate to list page correctly', () => {
13+
cy.visit(`http://localhost:${port}/content-change-navigation`);
14+
cy.ionPageVisible('content-nav-home');
15+
16+
cy.get('[data-testid="go-to-list"]').click();
17+
cy.wait(300);
18+
19+
cy.ionPageVisible('list-page');
20+
cy.url().should('include', '/content-change-navigation/list');
21+
});
22+
23+
it('when clearing items and navigating, should show home page, not empty view', () => {
24+
cy.visit(`http://localhost:${port}/content-change-navigation`);
25+
cy.ionPageVisible('content-nav-home');
26+
27+
cy.get('[data-testid="go-to-list"]').click();
28+
cy.wait(300);
29+
cy.ionPageVisible('list-page');
30+
31+
// Bug scenario: clearing items renders a different IonPage while navigating away
32+
cy.get('[data-testid="clear-and-navigate"]').click();
33+
cy.wait(500);
34+
35+
cy.url().should('include', '/content-change-navigation/home');
36+
cy.url().should('not.include', '/content-change-navigation/list');
37+
cy.ionPageVisible('content-nav-home');
38+
cy.get('[data-testid="home-content"]').should('be.visible');
39+
40+
// The empty view should NOT be visible (the fix ensures it's hidden)
41+
cy.get('[data-testid="empty-view"]').should('not.be.visible');
42+
});
43+
44+
it('direct navigation to home should work correctly', () => {
45+
cy.visit(`http://localhost:${port}/content-change-navigation/home`);
46+
cy.ionPageVisible('content-nav-home');
47+
cy.get('[data-testid="home-content"]').should('be.visible');
48+
});
49+
50+
it('direct navigation to list should work correctly', () => {
51+
cy.visit(`http://localhost:${port}/content-change-navigation/list`);
52+
cy.ionPageVisible('list-page');
53+
cy.contains('Item 1').should('be.visible');
54+
});
55+
});

0 commit comments

Comments
 (0)