Skip to content

Commit 6e97fc8

Browse files
committed
fix(react-router): use current route as leaving location for all navigation actions
1 parent 3c90a7d commit 6e97fc8

File tree

3 files changed

+54
-24
lines changed

3 files changed

+54
-24
lines changed

packages/react-router/src/ReactRouter/IonRouter.tsx

Lines changed: 5 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -187,29 +187,12 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr
187187
const handleHistoryChange = (location: HistoryLocation, action: HistoryAction) => {
188188
let leavingLocationInfo: RouteInfo;
189189
/**
190-
* A programmatic navigation was triggered.
191-
* e.g., `<Navigate />`, `navigate()`, or `handleNavigate()`
190+
* The leaving location is always the current route, for both programmatic
191+
* and external navigations. Using `previous()` for replace actions was
192+
* incorrect: it caused the equality check below to skip navigation when
193+
* the replace destination matched the entry two slots back in history.
192194
*/
193-
if (incomingRouteParams.current) {
194-
/**
195-
* The current history entry is overwritten, so the previous entry
196-
* is the one we are leaving.
197-
*/
198-
if (incomingRouteParams.current?.routeAction === 'replace') {
199-
leavingLocationInfo = locationHistory.current.previous();
200-
} else {
201-
// If the action is 'push' or 'pop', we want to use the current route.
202-
leavingLocationInfo = locationHistory.current.current();
203-
}
204-
} else {
205-
/**
206-
* An external navigation was triggered
207-
* e.g., browser back/forward button or direct link
208-
*
209-
* The leaving location is the current route.
210-
*/
211-
leavingLocationInfo = locationHistory.current.current();
212-
}
195+
leavingLocationInfo = locationHistory.current.current();
213196

214197
const leavingUrl = leavingLocationInfo.pathname + leavingLocationInfo.search;
215198
if (leavingUrl !== location.pathname + location.search) {

packages/react-router/test/base/src/pages/replace-action/Replace.tsx

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,7 @@ import {
77
IonButton,
88
IonButtons,
99
IonBackButton,
10+
useIonRouter,
1011
} from '@ionic/react';
1112
import React from 'react';
1213
import { useNavigate } from 'react-router-dom';
@@ -24,7 +25,7 @@ const Page1: React.FC = () => (
2425
</IonToolbar>
2526
</IonHeader>
2627
<IonContent>
27-
<IonButton routerLink={'/replace-action/page2'}>Goto Page2</IonButton>
28+
<IonButton id="go-to-page2" routerLink={'/replace-action/page2'}>Goto Page2</IonButton>
2829
</IonContent>
2930
</IonPage>
3031
);
@@ -47,13 +48,15 @@ const Page2: React.FC = () => {
4748
</IonToolbar>
4849
</IonHeader>
4950
<IonContent>
50-
<IonButton onClick={() => clickButton()}>Goto Page3</IonButton>
51+
<IonButton id="go-to-page3" onClick={() => clickButton()}>Goto Page3</IonButton>
5152
</IonContent>
5253
</IonPage>
5354
);
5455
};
5556

5657
const Page3: React.FC = () => {
58+
const ionRouter = useIonRouter();
59+
5760
return (
5861
<IonPage data-pageid="page3">
5962
<IonHeader>
@@ -67,6 +70,11 @@ const Page3: React.FC = () => {
6770
<IonContent>
6871
<p>Page 3</p>
6972
<p>Note: The back button navigates to Page 1 (not Page 2) because Page 2 used replace navigation.</p>
73+
{/* Exercises the replace action path through Ionic's handleNavigate() to
74+
reproduce the scenario fixed in issue #30086. */}
75+
<IonButton id="replace-to-page1" onClick={() => ionRouter.push('/replace-action/page1', 'none', 'replace')}>
76+
Replace to Page1
77+
</IonButton>
7078
</IonContent>
7179
</IonPage>
7280
);
Lines changed: 39 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,39 @@
1+
import { test, expect } from '@playwright/test';
2+
import { ionPageVisible, ionPageDoesNotExist } from './utils/test-utils';
3+
4+
test.describe('Replace to Existing Page', () => {
5+
/**
6+
* Regression test for #30086:
7+
* Replace navigate to a page already in the Ionic location history silently
8+
* fails when using Ionic's handleNavigate() (useIonRouter.push with routeAction='replace').
9+
*
10+
* Route sequence: /page1 --(push)--> /page2 --(replace)--> /page3
11+
* From /page3, replace back to /page1 should succeed.
12+
*
13+
* Note: This test intentionally does NOT use withTestingMode() because
14+
* `?ionic:_testing=true` in the URL would cause a search param mismatch that
15+
* prevents the bug scenario from triggering.
16+
*/
17+
test('replace navigate to a page already in location history should succeed', async ({ page }) => {
18+
// Navigate WITHOUT ionic:_testing=true — search params must be empty to match
19+
// the replace destination exactly and reproduce the bug scenario.
20+
await page.goto('/replace-action/page1');
21+
await ionPageVisible(page, 'page1');
22+
23+
// 1. Push to page2
24+
await page.locator('#go-to-page2').click();
25+
await ionPageVisible(page, 'page2');
26+
27+
// 2. Replace page2 with page3
28+
await page.locator('#go-to-page3').click();
29+
await ionPageVisible(page, 'page3');
30+
await ionPageDoesNotExist(page, 'page2');
31+
32+
// 3. Replace page3 back to page1 — destination matches the previous() entry,
33+
// which was the bug trigger (#30086).
34+
await page.locator('#replace-to-page1').click();
35+
await ionPageVisible(page, 'page1');
36+
await expect(page).toHaveURL(/\/replace-action\/page1/);
37+
await ionPageDoesNotExist(page, 'page3');
38+
});
39+
});

0 commit comments

Comments
 (0)