Skip to content

Commit 36ba4c5

Browse files
ShaneKShaneK
committed
fix(react-router): prevent duplicate history entries when replacing to previous URL
1 parent 2f2d822 commit 36ba4c5

File tree

3 files changed

+213
-5
lines changed

3 files changed

+213
-5
lines changed

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

Lines changed: 22 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -642,6 +642,28 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr
642642
}
643643
}
644644

645+
// When a replace action targets the same URL as the immediately previous
646+
// history entry, using replaceState would create a duplicate browser history
647+
// entry (the previous and current entries would both have the same URL).
648+
// Navigate back to the previous entry instead to avoid the duplicate.
649+
// Use 'back' direction so the leaving view is unmounted (same as replace).
650+
if (routeAction === 'replace') {
651+
const prevEntry = locationHistory.current.previous();
652+
const currentEntry = locationHistory.current.current();
653+
const prevPath = prevEntry ? prevEntry.pathname + (prevEntry.search || '') : undefined;
654+
if (prevEntry && currentEntry && prevEntry !== currentEntry && prevPath === path) {
655+
incomingRouteParams.current = {
656+
...prevEntry,
657+
routeAction: 'pop',
658+
routeDirection: 'back',
659+
routeAnimation,
660+
};
661+
forwardStack.current.push(currentLocationKeyRef.current);
662+
navigate(-1);
663+
return;
664+
}
665+
}
666+
645667
const baseParams = incomingRouteParams.current ?? {};
646668
incomingRouteParams.current = {
647669
...baseParams,

‎packages/react-router/test/base/tests/e2e/playwright/nested-params.spec.ts‎

Lines changed: 7 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -23,17 +23,19 @@ test.describe('Nested Params', () => {
2323
await expect(page.getByText('Details view user: 99')).toBeVisible();
2424

2525
// Step 2: Back to landing via routerLink (push).
26-
// Use .first() because the hidden user 99 details page stays in the DOM,
27-
// leaving a duplicate #back-to-landing button.
28-
await page.locator('#back-to-landing').first().click();
26+
// Scope to the visible page -- the hidden user 99 details page may still
27+
// have a #back-to-landing in the DOM (view stack cleanup is async).
28+
await page.locator('.ion-page:not(.ion-page-hidden):not(.ion-page-invisible) #back-to-landing').click();
2929
await ionPageVisible(page, 'nested-params-landing');
3030

3131
// Step 3: Navigate to user 42 details
3232
await page.locator('#go-to-user-42').click();
3333
await expect(page.getByText('Details view user: 42')).toBeVisible();
3434

35-
// Step 4: Back to landing via routerLink (push)
36-
await page.locator('#back-to-landing').first().click();
35+
// Step 4: Back to landing via routerLink (push).
36+
// Scope to the visible page -- the hidden user 99 details page may still
37+
// have a #back-to-landing in the DOM (view stack cleanup is async).
38+
await page.locator('.ion-page:not(.ion-page-hidden):not(.ion-page-invisible) #back-to-landing').click();
3739
await ionPageVisible(page, 'nested-params-landing');
3840

3941
// Step 5: Navigate to user 42 details AGAIN -- this triggered the infinite loop

‎packages/react-router/test/base/tests/e2e/playwright/replace-history.spec.ts‎

Lines changed: 184 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,184 @@
1+
import { test, expect } from '@playwright/test';
2+
import {
3+
ionPageVisible,
4+
ionPageDoesNotExist,
5+
ionBackClick,
6+
ionNav,
7+
ionGoBack,
8+
withTestingMode,
9+
} from './utils/test-utils';
10+
11+
test.describe('Replace History Entries', () => {
12+
/**
13+
* Tests that replace navigation does not create duplicate browser history entries
14+
* when using browser back for all navigation.
15+
*
16+
* Flow: home -> page1 -> push page2 -> replace page3 -> browser back -> page1 -> browser back -> home
17+
*/
18+
test('replace should not create duplicate history entries with browser back', async ({ page }) => {
19+
await page.goto(withTestingMode('/'));
20+
await ionPageVisible(page, 'home');
21+
22+
// Navigate to the replace-action test
23+
await ionNav(page, 'ion-item', 'Replace Action');
24+
await ionPageVisible(page, 'page1');
25+
26+
// Push to page2
27+
await page.locator('#go-to-page2').click();
28+
await ionPageVisible(page, 'page2');
29+
30+
// Replace page2 with page3
31+
await page.locator('#go-to-page3').click();
32+
await ionPageVisible(page, 'page3');
33+
await ionPageDoesNotExist(page, 'page2');
34+
35+
// Browser back from page3 -> page1 (page2 was replaced)
36+
await ionGoBack(page, '/replace-action/page1');
37+
await ionPageVisible(page, 'page1');
38+
39+
// Browser back from page1 -> home, NOT page1 again
40+
await ionGoBack(page, '/');
41+
await ionPageVisible(page, 'home');
42+
});
43+
44+
/**
45+
* Tests that Ionic back button + browser back does not create duplicate entries.
46+
*
47+
* The Ionic back button calls handleNavigateBack which may use navigate(-1) or
48+
* handleNavigate (replace). If it incorrectly uses a replace, the browser history
49+
* will have page1 twice: once from the original navigation and once from the
50+
* replace that overwrote page3's entry.
51+
*
52+
* Flow: home -> page1 -> push page2 -> replace page3 -> IonBackButton -> page1 -> browser back -> home
53+
*/
54+
test('ionic back button after replace should not create duplicate history entries', async ({ page }) => {
55+
await page.goto(withTestingMode('/'));
56+
await ionPageVisible(page, 'home');
57+
58+
// Navigate to the replace-action test
59+
await ionNav(page, 'ion-item', 'Replace Action');
60+
await ionPageVisible(page, 'page1');
61+
62+
// Push to page2
63+
await page.locator('#go-to-page2').click();
64+
await ionPageVisible(page, 'page2');
65+
66+
// Replace page2 with page3
67+
await page.locator('#go-to-page3').click();
68+
await ionPageVisible(page, 'page3');
69+
await ionPageDoesNotExist(page, 'page2');
70+
71+
// Use the IonBackButton (not browser back) from page3
72+
await ionBackClick(page, 'page3');
73+
await ionPageVisible(page, 'page1');
74+
await expect(page).toHaveURL(/\/replace-action\/page1/);
75+
76+
// Browser back from page1 -> should go to home, NOT page1 again
77+
await ionGoBack(page);
78+
// Page1 should no longer be visible - we should be at home
79+
const url = page.url();
80+
expect(url).not.toContain('/replace-action/page1');
81+
await ionPageVisible(page, 'home');
82+
});
83+
84+
/**
85+
* Tests browser forward still works after going back from a replaced page.
86+
* This verifies navigate(-1) is used (not replace) for the back navigation.
87+
*/
88+
test('browser forward should work after going back from replaced page', async ({ page }) => {
89+
await page.goto(withTestingMode('/'));
90+
await ionPageVisible(page, 'home');
91+
92+
await ionNav(page, 'ion-item', 'Replace Action');
93+
await ionPageVisible(page, 'page1');
94+
95+
await page.locator('#go-to-page2').click();
96+
await ionPageVisible(page, 'page2');
97+
98+
await page.locator('#go-to-page3').click();
99+
await ionPageVisible(page, 'page3');
100+
await ionPageDoesNotExist(page, 'page2');
101+
102+
// Browser back -> page1
103+
await ionGoBack(page, '/replace-action/page1');
104+
await ionPageVisible(page, 'page1');
105+
106+
// Browser forward -> page3 (forward history should be preserved)
107+
await page.goForward();
108+
await page.waitForTimeout(500);
109+
await ionPageVisible(page, 'page3');
110+
});
111+
112+
/**
113+
* Tests the "Replace to Page1" button on Page3 which uses ionRouter.push
114+
* with routeAction='replace'. This goes through handleNavigate() rather than
115+
* React Router's navigate() directly.
116+
*
117+
* Flow: home -> page1 -> page2 -> replace page3 -> replace back to page1 -> browser back -> home
118+
*/
119+
test('ionRouter.push with replace should not create duplicate history entries', async ({ page }) => {
120+
// Note: NOT using withTestingMode() - search params interfere with replace-to-existing
121+
await page.goto('/');
122+
await ionPageVisible(page, 'home');
123+
124+
await ionNav(page, 'ion-item', 'Replace Action');
125+
await ionPageVisible(page, 'page1');
126+
127+
// Push to page2
128+
await page.locator('#go-to-page2').click();
129+
await ionPageVisible(page, 'page2');
130+
131+
// Replace page2 with page3
132+
await page.locator('#go-to-page3').click();
133+
await ionPageVisible(page, 'page3');
134+
await ionPageDoesNotExist(page, 'page2');
135+
136+
// Replace page3 back to page1 using ionRouter.push
137+
await page.locator('#replace-to-page1').click();
138+
await page.waitForTimeout(500);
139+
await ionPageVisible(page, 'page1');
140+
await expect(page).toHaveURL(/\/replace-action\/page1/);
141+
142+
// Browser back from page1 should go to home, NOT page1 again.
143+
// If replace creates duplicates, browser history would be:
144+
// [home, page1, page1] (page1 at its original slot + page1 replacing page3)
145+
await page.goBack();
146+
await page.waitForTimeout(500);
147+
148+
const url = page.url();
149+
expect(url).not.toContain('/replace-action');
150+
await ionPageVisible(page, 'home');
151+
});
152+
153+
/**
154+
* Tests replace with animations enabled (no ionic:_testing=true).
155+
* Animation timing can affect how history entries are processed.
156+
*/
157+
test('replace should not create duplicate history entries with animations', async ({ page }) => {
158+
await page.goto('/');
159+
await ionPageVisible(page, 'home');
160+
161+
await ionNav(page, 'ion-item', 'Replace Action');
162+
await ionPageVisible(page, 'page1');
163+
164+
await page.locator('#go-to-page2').click();
165+
await ionPageVisible(page, 'page2');
166+
167+
await page.locator('#go-to-page3').click();
168+
await ionPageVisible(page, 'page3');
169+
170+
// Wait for replace animation and cleanup
171+
await page.waitForTimeout(500);
172+
await ionPageDoesNotExist(page, 'page2');
173+
174+
// Browser back from page3 -> page1
175+
await page.goBack();
176+
await page.waitForTimeout(500);
177+
await ionPageVisible(page, 'page1');
178+
179+
// Browser back from page1 -> home (not page1 again)
180+
await page.goBack();
181+
await page.waitForTimeout(500);
182+
await ionPageVisible(page, 'home');
183+
});
184+
});

0 commit comments

Comments
 (0)