Skip to content

Commit f5e6026

Browse files
ShaneKShaneK
committed
fix(react-router): preserve non-parameterized views on browser back to retain React state
1 parent 04e9c97 commit f5e6026

7 files changed

Lines changed: 195 additions & 16 deletions

File tree

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

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -646,15 +646,17 @@ export const IonRouter = ({ children, registerHistoryListener }: PropsWithChildr
646646
// history entry, using replaceState would create a duplicate browser history
647647
// entry (the previous and current entries would both have the same URL).
648648
// Navigate back to the previous entry instead to avoid the duplicate.
649-
// Use 'back' direction so the leaving view is unmounted (same as replace).
649+
// Keep routeAction as 'replace' so StackManager correctly unmounts the
650+
// leaving view through handleLeavingViewUnmount rather than treating it
651+
// as a browser-back pop (which preserves views for back/forward history).
650652
if (routeAction === 'replace') {
651653
const prevEntry = locationHistory.current.previous();
652654
const currentEntry = locationHistory.current.current();
653655
const prevPath = prevEntry ? prevEntry.pathname + (prevEntry.search || '') : undefined;
654656
if (prevEntry && currentEntry && prevEntry !== currentEntry && prevPath === path) {
655657
incomingRouteParams.current = {
656658
...prevEntry,
657-
routeAction: 'pop',
659+
routeAction: 'replace',
658660
routeDirection: 'back',
659661
routeAnimation,
660662
};

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

Lines changed: 97 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -53,11 +53,42 @@ const hideIonPageElement = (element: HTMLElement | undefined): void => {
5353
const showIonPageElement = (element: HTMLElement | undefined): void => {
5454
if (element) {
5555
element.style.removeProperty('visibility');
56+
// core transitions (md.transition.ts, ios.transition.ts) leave inline styles
57+
// on the leaving page after animation: `display: none` plus the final
58+
// keyframe values for `transform` and `opacity` (MD only). Preserved views
59+
// must clear all of these on re-entry so they render in the correct
60+
// position when the back direction entering animation is a no-op.
61+
element.style.removeProperty('display');
62+
element.style.removeProperty('transform');
63+
element.style.removeProperty('opacity');
5664
element.classList.remove('ion-page-hidden');
5765
element.removeAttribute('aria-hidden');
5866
}
5967
};
6068

69+
/**
70+
* A leaf view is "preservable" on browser-back (pop) when its React state
71+
* should survive a forward-pop round-trip. Non-parameterized leaf paths
72+
* resolve to the same view item on re-entry, so keeping them mounted retains
73+
* user-visible state (scroll, inputs, cleared lists, etc.).
74+
*
75+
* Excluded:
76+
* - Parameterized routes (`/users/:id`): each param value gets a distinct
77+
* view item, so preserving them accumulates hidden views in the DOM.
78+
* - Wildcard container routes (`/tabs/*`, `*`): wrap nested outlets and must
79+
* be destroyed so nested outlet state rebuilds cleanly on re-entry.
80+
*/
81+
const isViewItemPreservableOnPop = (viewItem: ViewItem | undefined): boolean => {
82+
const path = viewItem?.reactElement?.props?.path as string | undefined;
83+
if (!path) {
84+
return false;
85+
}
86+
if (path === '*' || path.endsWith('/*')) {
87+
return false;
88+
}
89+
return !path.includes(':');
90+
};
91+
6192
export class StackManager extends React.PureComponent<StackManagerProps> {
6293
id: string; // Unique id for the router outlet aka outletId
6394
context!: React.ContextType<typeof RouteManagerContext>;
@@ -535,17 +566,66 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
535566
this.transitionPage(routeInfo, enteringViewItem, leavingViewItem, undefined, false, shouldSkipAnimation);
536567

537568
if (shouldUnmountLeavingViewItem && leavingViewItem && enteringViewItem !== leavingViewItem) {
538-
// For non-replace actions (back nav), set mount=false here to hide the view.
539-
// For replace actions, handleLeavingViewUnmount sets mount=false only after
540-
// its container-to-container guard passes, avoiding zombie state.
541-
if (routeInfo.routeAction !== 'replace') {
569+
// For replace actions, skip setting mount=false here. handleLeavingViewUnmount
570+
// sets it only after its container-to-container guard passes, avoiding zombie state.
571+
//
572+
// For pop (browser back) on preservable routes (see isViewItemPreservableOnPop),
573+
// keep the view alive (hidden by the transition) so its React state survives a
574+
// forward-pop round-trip. ionViewDidLeave already fired via the transitionPage()
575+
// call above. Swipe-to-go-back still destroys views through the skipTransition
576+
// path earlier in this method, matching native gesture behavior.
577+
//
578+
// handleLeavingViewUnmount below is a no-op for non-replace actions (early return),
579+
// so pop-preserved views pass through it untouched.
580+
const shouldPreserveLeavingView =
581+
routeInfo.routeAction === 'pop' && isViewItemPreservableOnPop(leavingViewItem);
582+
if (routeInfo.routeAction !== 'replace' && !shouldPreserveLeavingView) {
542583
leavingViewItem.mount = false;
543584
}
544585
this.handleLeavingViewUnmount(routeInfo, enteringViewItem, leavingViewItem);
545586
}
546587

547588
// Clean up orphaned sibling views after replace actions (redirects)
548589
this.cleanupOrphanedSiblingViews(routeInfo, enteringViewItem, leavingViewItem);
590+
591+
// On a fresh push, browser forward history is invalidated. Any views we
592+
// previously preserved on pop (to support forward navigation) are now
593+
// unreachable and should be unmounted so they don't accumulate in the DOM.
594+
this.cleanupPreservedViewsOnPush(routeInfo, enteringViewItem, leavingViewItem);
595+
}
596+
597+
/**
598+
* Unmounts any previously-preserved leaf views in this outlet when a push
599+
* action invalidates the forward history path. Runs only on `push` (not
600+
* replace/pop) and skips the entering and leaving view items.
601+
*/
602+
private cleanupPreservedViewsOnPush(
603+
routeInfo: RouteInfo,
604+
enteringViewItem: ViewItem,
605+
leavingViewItem: ViewItem | undefined
606+
): void {
607+
if (routeInfo.routeAction !== 'push') {
608+
return;
609+
}
610+
611+
const allViews = this.context.getViewItemsForOutlet(this.id);
612+
for (const viewItem of allViews) {
613+
if (viewItem === enteringViewItem || viewItem === leavingViewItem) {
614+
continue;
615+
}
616+
if (!viewItem.mount) {
617+
continue;
618+
}
619+
if (!isViewItemPreservableOnPop(viewItem)) {
620+
continue;
621+
}
622+
viewItem.mount = false;
623+
const viewToUnmount = viewItem;
624+
setTimeout(() => {
625+
this.context.unMountViewItem(viewToUnmount);
626+
this.forceUpdate();
627+
}, VIEW_UNMOUNT_DELAY_MS);
628+
}
549629
}
550630

551631
/**
@@ -779,12 +859,16 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
779859

780860
// Don't unmount if entering and leaving are the same view item
781861
if (shouldUnmountLeavingViewItem && leavingViewItem && enteringViewItem !== leavingViewItem) {
782-
if (routeInfo.routeAction !== 'replace') {
862+
const shouldPreserveLeavingView =
863+
routeInfo.routeAction === 'pop' && isViewItemPreservableOnPop(leavingViewItem);
864+
if (routeInfo.routeAction !== 'replace' && !shouldPreserveLeavingView) {
783865
leavingViewItem.mount = false;
784866
}
785867
this.handleLeavingViewUnmount(routeInfo, enteringViewItem, leavingViewItem);
786868
}
787869

870+
this.cleanupPreservedViewsOnPush(routeInfo, enteringViewItem, leavingViewItem);
871+
788872
this.forceUpdate();
789873
return;
790874
}
@@ -816,12 +900,16 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
816900
this.transitionPage(routeInfo, latestEnteringView, latestLeavingView ?? undefined, undefined, false, shouldSkipAnimation);
817901

818902
if (shouldUnmountLeavingViewItem && latestLeavingView && latestEnteringView !== latestLeavingView) {
819-
if (routeInfo.routeAction !== 'replace') {
903+
const shouldPreserveLeavingView =
904+
routeInfo.routeAction === 'pop' && isViewItemPreservableOnPop(latestLeavingView);
905+
if (routeInfo.routeAction !== 'replace' && !shouldPreserveLeavingView) {
820906
latestLeavingView.mount = false;
821907
}
822908
this.handleLeavingViewUnmount(routeInfo, latestEnteringView, latestLeavingView);
823909
}
824910

911+
this.cleanupPreservedViewsOnPush(routeInfo, latestEnteringView, latestLeavingView ?? undefined);
912+
825913
this.forceUpdate();
826914
} else {
827915
/**
@@ -1045,7 +1133,9 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
10451133
// No view or route found - likely leaving to another outlet
10461134
if (leavingViewItem) {
10471135
hideIonPageElement(leavingViewItem.ionPageElement);
1048-
if (shouldUnmountLeavingViewItem) {
1136+
const shouldPreserveLeavingView =
1137+
routeInfo.routeAction === 'pop' && isViewItemPreservableOnPop(leavingViewItem);
1138+
if (shouldUnmountLeavingViewItem && !shouldPreserveLeavingView) {
10491139
leavingViewItem.mount = false;
10501140
}
10511141
}

‎packages/react-router/test/base/tests/e2e/playwright/content-change-navigation.spec.ts‎

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
import { test, expect } from '@playwright/test';
2+
import { ionPageVisible, ionGoBack, ionGoForward, withTestingMode } from './utils/test-utils';
3+
4+
/**
5+
* Verifies that StackManager preserves non-parameterized leaf views on
6+
* browser-back (pop) so React state survives forward-pop round-trips.
7+
* The test page deliberately changes its rendered content between navigations
8+
* (items -> empty) to prove the same view instance is reused.
9+
*/
10+
test.describe('Content Change Navigation', () => {
11+
12+
test('clearing content then navigating away should preserve cleared state on revisit', async ({ page }) => {
13+
await page.goto(withTestingMode('/content-change-navigation'));
14+
await ionPageVisible(page, 'content-nav-home');
15+
16+
// Navigate to list page - items should be visible
17+
await page.locator('[data-testid="go-to-list"]').click();
18+
await ionPageVisible(page, 'list-page');
19+
await expect(page.locator('ion-item')).toHaveCount(3);
20+
21+
// Clear items and navigate to home
22+
await page.locator('[data-testid="clear-and-navigate"]').click();
23+
await ionPageVisible(page, 'content-nav-home');
24+
25+
// Navigate to list page again -- the view was preserved (forward push does
26+
// not destroy the leaving view), so React state persists: items are still
27+
// empty and the empty-page variant renders.
28+
await page.locator('[data-testid="go-to-list"]').click();
29+
await ionPageVisible(page, 'list-empty-page');
30+
await expect(page.locator('[data-testid="empty-view"]')).toBeVisible();
31+
});
32+
33+
test('browser back should preserve React state of the leaving view', async ({ page }) => {
34+
await page.goto(withTestingMode('/content-change-navigation'));
35+
await ionPageVisible(page, 'content-nav-home');
36+
37+
// Navigate to list page
38+
await page.locator('[data-testid="go-to-list"]').click();
39+
await ionPageVisible(page, 'list-page');
40+
await expect(page.locator('ion-item')).toHaveCount(3);
41+
42+
// Clear items, then use the routerLink to go home (forward push)
43+
await page.locator('[data-testid="clear-and-navigate"]').click();
44+
await ionPageVisible(page, 'content-nav-home');
45+
46+
// Browser back to the list -- pop preserves the non-parameterized view,
47+
// so the cleared (empty) state survives the round-trip.
48+
await ionGoBack(page, '/content-change-navigation/list');
49+
await ionPageVisible(page, 'list-empty-page');
50+
await expect(page.locator('[data-testid="empty-view"]')).toBeVisible();
51+
await expect(page.locator('ion-item')).toHaveCount(0);
52+
});
53+
54+
test('browser back on non-parameterized route with animations should show entering page', async ({ page }) => {
55+
// NO withTestingMode -- animations must run to test the real transition path
56+
await page.goto('/content-change-navigation');
57+
await ionPageVisible(page, 'content-nav-home');
58+
59+
await page.locator('[data-testid="go-to-list"]').click();
60+
await ionPageVisible(page, 'list-page');
61+
62+
// Browser back with animations enabled; ionPageVisible auto-retries until
63+
// the entering page finishes its transition.
64+
await page.goBack();
65+
await ionPageVisible(page, 'content-nav-home');
66+
});
67+
68+
test('browser forward should restore list view after back', async ({ page }) => {
69+
await page.goto(withTestingMode('/content-change-navigation'));
70+
await ionPageVisible(page, 'content-nav-home');
71+
72+
// Navigate to list page
73+
await page.locator('[data-testid="go-to-list"]').click();
74+
await ionPageVisible(page, 'list-page');
75+
await expect(page.locator('ion-item')).toHaveCount(3);
76+
77+
// Browser back to home
78+
await ionGoBack(page, '/content-change-navigation/home');
79+
await ionPageVisible(page, 'content-nav-home');
80+
81+
// Browser forward to list -- view preserved, items still present
82+
await ionGoForward(page, '/content-change-navigation/list');
83+
await ionPageVisible(page, 'list-page');
84+
await expect(page.locator('ion-item')).toHaveCount(3);
85+
});
86+
87+
});

‎packages/react-router/test/base/tests/e2e/playwright/param-swipe-back.spec.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -46,7 +46,7 @@ test.describe('Param Back Navigation (#27900)', () => {
4646
// 5. Back → /user/alex (this is the step that broke in #27900)
4747
await ionGoBack(page, '/param-swipe-back/user/alex');
4848
await ionPageVisible(page, 'psb-user-alex');
49-
await ionPageDoesNotExist(page, 'psb-middle');
49+
await ionPageHidden(page, 'psb-middle');
5050
});
5151

5252
test('Repro A: swipe back through param routes with non-param in between', async ({ page }) => {

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -59,7 +59,7 @@ test.describe('Tabs', () => {
5959
await ionPageVisible(page, 'tab1child1');
6060

6161
await ionGoBack(page, '/tabs/tab1');
62-
await ionPageDoesNotExist(page, 'tab1child1');
62+
await ionPageHidden(page, 'tab1child1');
6363
await ionPageVisible(page, 'tab1');
6464
await expect(page.locator('ion-tab-button.tab-selected')).toContainText('Tab1');
6565
});
@@ -102,7 +102,7 @@ test.describe('Tabs', () => {
102102
await ionPageVisible(page, 'tab1child1');
103103

104104
await ionBackClick(page, 'tab1child1');
105-
await ionPageDoesNotExist(page, 'tab1child1');
105+
await ionPageHidden(page, 'tab1child1');
106106
await ionPageVisible(page, 'tab1');
107107

108108
await expect(page).toHaveURL(/\/tabs\/tab1/);

‎packages/react-router/test/base/tests/e2e/specs/tab-history-isolation.cy.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ describe('Tab History Isolation', () => {
5151
cy.ionPageVisible('tab-a-details');
5252

5353
cy.ionBackClick('tab-a-details');
54-
cy.ionPageDoesNotExist('tab-a-details');
54+
cy.ionPageHidden('tab-a-details');
5555
cy.ionPageVisible('tab-a');
5656

5757
cy.url().should('include', '/tab-history-isolation/a');
@@ -71,7 +71,7 @@ describe('Tab History Isolation', () => {
7171
cy.ionPageVisible('tab-b-details');
7272

7373
cy.ionBackClick('tab-b-details');
74-
cy.ionPageDoesNotExist('tab-b-details');
74+
cy.ionPageHidden('tab-b-details');
7575
cy.ionPageVisible('tab-b');
7676

7777
cy.url().should('include', '/tab-history-isolation/b');
@@ -104,7 +104,7 @@ describe('Tab History Isolation', () => {
104104
cy.ionPageVisible('tab-a-details');
105105

106106
cy.ionBackClick('tab-a-details');
107-
cy.ionPageDoesNotExist('tab-a-details');
107+
cy.ionPageHidden('tab-a-details');
108108
cy.ionPageVisible('tab-a');
109109
});
110110

‎packages/react-router/test/base/tests/e2e/specs/tabs.cy.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ describe('Tabs', () => {
4949
cy.ionPageVisible('tab1child1');
5050

5151
cy.ionBackClick('tab1child1');
52-
cy.ionPageDoesNotExist('tab1child1');
52+
cy.ionPageHidden('tab1child1');
5353
cy.ionPageVisible('tab1');
5454

5555
cy.url().should('include', '/tabs/tab1');

0 commit comments

Comments
 (0)