Skip to content

Commit 82fd1ba

Browse files
ShaneKShaneK
committed
fix(react-router): fixing views not being cleaned up properly, causing cross navigation issues
1 parent 00798d4 commit 82fd1ba

File tree

3 files changed

+210
-0
lines changed

3 files changed

+210
-0
lines changed

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

Lines changed: 36 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -616,6 +616,42 @@ export class ReactRouterViewStack extends ViewStacks {
616616
return false;
617617
}
618618

619+
// Filter out views that are unmounted, have no ionPageElement, and don't match the current route.
620+
// These are "stale" views from previous routes that should not be rendered.
621+
// Views WITH ionPageElement are handled by the normal lifecycle events.
622+
// Views that MATCH the current route should be kept (they might be transitioning).
623+
if (!viewItem.mount && !viewItem.ionPageElement) {
624+
// Check if this view's route path matches the current pathname
625+
const viewRoutePath = viewItem.reactElement?.props?.path as string | undefined;
626+
if (viewRoutePath) {
627+
// First try exact match using matchComponent
628+
const routeMatch = matchComponent(viewItem.reactElement, routeInfo.pathname);
629+
if (routeMatch) {
630+
// View matches current route, keep it
631+
return true;
632+
}
633+
634+
// For parent routes (like /multiple-tabs or /routing), check if current pathname
635+
// starts with this route's path. This handles views with IonSplitPane/IonTabs
636+
// that don't have IonPage but should remain mounted while navigating within their children.
637+
const normalizedViewPath = normalizePathnameForComparison(viewRoutePath.replace(/\/?\*$/, '')); // Remove trailing wildcard
638+
const normalizedCurrentPath = normalizePathnameForComparison(routeInfo.pathname);
639+
640+
// Check if current pathname is within this view's route hierarchy
641+
const isWithinRouteHierarchy =
642+
normalizedCurrentPath === normalizedViewPath ||
643+
normalizedCurrentPath.startsWith(normalizedViewPath + '/');
644+
645+
if (!isWithinRouteHierarchy) {
646+
// View is outside current route hierarchy, remove it
647+
setTimeout(() => {
648+
this.remove(viewItem);
649+
}, 0);
650+
return false;
651+
}
652+
}
653+
}
654+
619655
return true;
620656
});
621657

‎packages/react-router/test/base/tests/e2e/specs/cross-route-navigation.cy.js‎

Lines changed: 168 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,168 @@
1+
const port = 3000;
2+
3+
describe('Cross-Route Navigation', () => {
4+
/**
5+
* This test verifies that navigation between different top-level routes works correctly.
6+
*
7+
* Routing uses <IonRouterOutlet id="routing-main"> and MultipleTabs uses
8+
* <IonRouterOutlet id="multiple-tabs-main">, ensuring view isolation between outlets.
9+
*/
10+
it('should navigate from home to routing and back correctly', () => {
11+
// Start at home
12+
cy.visit(`http://localhost:${port}/`);
13+
cy.ionPageVisible('home');
14+
15+
// Navigate to routing by clicking the link
16+
cy.contains('ion-item', 'Routing').click();
17+
18+
// Routing should redirect to /routing/tabs/home and show the home-page
19+
cy.ionPageVisible('home-page');
20+
21+
// Go back to the main home page using browser back
22+
cy.go('back');
23+
24+
// Home page should be visible again
25+
cy.ionPageVisible('home');
26+
});
27+
28+
it('should navigate from home to multiple-tabs correctly', () => {
29+
// Start at home
30+
cy.visit(`http://localhost:${port}/`);
31+
cy.ionPageVisible('home');
32+
33+
// Navigate to multiple-tabs
34+
cy.contains('ion-item', 'Multiple Tabs').click();
35+
36+
// Multiple tabs should redirect to /multiple-tabs/tab1/pagea and show PageA
37+
cy.ionPageVisible('PageA');
38+
});
39+
40+
it('should navigate home -> routing -> back -> routing again', () => {
41+
// Start at home
42+
cy.visit(`http://localhost:${port}/`);
43+
cy.ionPageVisible('home');
44+
45+
// Navigate to routing
46+
cy.contains('ion-item', 'Routing').click();
47+
cy.ionPageVisible('home-page');
48+
49+
// Go back to home
50+
cy.go('back');
51+
cy.ionPageVisible('home');
52+
53+
// Navigate to routing again - Navigate should fire again
54+
cy.contains('ion-item', 'Routing').click();
55+
cy.ionPageVisible('home-page');
56+
});
57+
58+
it('should navigate home -> multiple-tabs -> back -> multiple-tabs again', () => {
59+
// Start at home
60+
cy.visit(`http://localhost:${port}/`);
61+
cy.ionPageVisible('home');
62+
63+
// Navigate to multiple-tabs
64+
cy.contains('ion-item', 'Multiple Tabs').click();
65+
cy.ionPageVisible('PageA');
66+
67+
// Go back to home
68+
cy.go('back');
69+
cy.ionPageVisible('home');
70+
71+
// Navigate to multiple-tabs again - Navigate should fire again
72+
cy.contains('ion-item', 'Multiple Tabs').click();
73+
cy.ionPageVisible('PageA');
74+
});
75+
76+
/**
77+
* This test verifies behavior when navigating between different top-level routes
78+
* that use separate outlet IDs. With unique outlet IDs, view items are completely
79+
* isolated between outlets, so "stale views" from one outlet don't interfere with
80+
* another outlet.
81+
*
82+
* This test uses ionPageDoesNotExist instead of ionPageHidden because views from
83+
* one route hierarchy (like /routing/*) are completely unmounted (not just hidden)
84+
* when navigating to a different route hierarchy (like /multiple-tabs/*).
85+
*/
86+
it('should navigate home -> routing -> home -> multiple-tabs without stale views', () => {
87+
// Start at home
88+
cy.visit(`http://localhost:${port}/`);
89+
cy.ionPageVisible('home');
90+
91+
// Navigate to routing
92+
cy.contains('ion-item', 'Routing').click();
93+
cy.ionPageVisible('home-page');
94+
cy.url().should('include', '/routing/tabs/home');
95+
96+
// Go back to home
97+
cy.go('back');
98+
cy.ionPageVisible('home');
99+
100+
// Navigate to multiple-tabs - this is where stale views could interfere
101+
cy.contains('ion-item', 'Multiple Tabs').click();
102+
cy.ionPageVisible('PageA');
103+
cy.url().should('include', '/multiple-tabs/tab1/pagea');
104+
105+
// The routing home-page should NOT exist in the DOM (views are unmounted when leaving route)
106+
cy.ionPageDoesNotExist('home-page');
107+
});
108+
109+
/**
110+
* Test the reverse: multiple-tabs -> home -> routing
111+
* With unique outlet IDs, views are isolated and properly unmounted.
112+
*/
113+
it('should navigate home -> multiple-tabs -> home -> routing without stale views', () => {
114+
// Start at home
115+
cy.visit(`http://localhost:${port}/`);
116+
cy.ionPageVisible('home');
117+
118+
// Navigate to multiple-tabs
119+
cy.contains('ion-item', 'Multiple Tabs').click();
120+
cy.ionPageVisible('PageA');
121+
cy.url().should('include', '/multiple-tabs/tab1/pagea');
122+
123+
// Go back to home
124+
cy.go('back');
125+
cy.ionPageVisible('home');
126+
127+
// Navigate to routing - stale views from multiple-tabs should be cleaned up
128+
cy.contains('ion-item', 'Routing').click();
129+
cy.ionPageVisible('home-page');
130+
cy.url().should('include', '/routing/tabs/home');
131+
132+
// PageA from multiple-tabs should NOT exist in the DOM (views are unmounted when leaving route)
133+
cy.ionPageDoesNotExist('PageA');
134+
});
135+
136+
/**
137+
* Test navigating to another page and back to routing
138+
* With unique outlet IDs, views are isolated and properly unmounted.
139+
*/
140+
it('should not have overlay issues when navigating between different routes', () => {
141+
// Start at home
142+
cy.visit(`http://localhost:${port}/`);
143+
cy.ionPageVisible('home');
144+
145+
// Navigate to routing
146+
cy.contains('ion-item', 'Routing').click();
147+
cy.ionPageVisible('home-page');
148+
149+
// Go back to home
150+
cy.go('back');
151+
cy.ionPageVisible('home');
152+
153+
// Navigate to multiple-tabs
154+
cy.contains('ion-item', 'Multiple Tabs').click();
155+
cy.ionPageVisible('PageA');
156+
157+
// Go back to home again
158+
cy.go('back');
159+
cy.ionPageVisible('home');
160+
161+
// Navigate back to routing - no stale views should overlay
162+
cy.contains('ion-item', 'Routing').click();
163+
cy.ionPageVisible('home-page');
164+
165+
// Verify PageA does not exist in the DOM (views are unmounted when leaving route)
166+
cy.ionPageDoesNotExist('PageA');
167+
});
168+
});

‎packages/react/src/routing/ViewLifeCycleManager.tsx‎

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,12 @@ export class ViewLifeCycleManager extends React.Component<ViewTransitionManagerP
4040
this._isMounted = true;
4141
}
4242

43+
componentDidUpdate(_prevProps: ViewTransitionManagerProps) {
44+
// View lifecycle is managed through ionViewDidLeave events.
45+
// Components with IonPage will receive these events and be destroyed accordingly.
46+
// The StackManager handles cleanup of views that no longer match routes.
47+
}
48+
4349
componentWillUnmount() {
4450
this._isMounted = false;
4551
}

0 commit comments

Comments
 (0)