Skip to content

Commit a550b61

Browse files
committed
fix(react-router): scope push cleanup to explicitly preserved views
1 parent f5e6026 commit a550b61

2 files changed

Lines changed: 148 additions & 8 deletions

File tree

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

Lines changed: 115 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,13 @@ const isViewVisible = (el: HTMLElement) =>
4545

4646
const hideIonPageElement = (element: HTMLElement | undefined): void => {
4747
if (element) {
48+
if (element.id === 'section-a' || element.id === 'section-b') {
49+
// eslint-disable-next-line no-console
50+
console.log('[HideIonPageElement]', JSON.stringify({
51+
id: element.id,
52+
stack: new Error().stack?.split('\n').slice(1, 6).map((s) => s.trim()),
53+
}));
54+
}
4855
element.classList.add('ion-page-hidden');
4956
element.setAttribute('aria-hidden', 'true');
5057
}
@@ -66,6 +73,42 @@ const showIonPageElement = (element: HTMLElement | undefined): void => {
6673
}
6774
};
6875

76+
/**
77+
* Variant of `showIonPageElement` for the swipe-back gesture start. Clears
78+
* `display: none` and the hidden class/attribute so the entering view is
79+
* visible, but intentionally keeps any inline `transform` and `opacity` set
80+
* by core's prior forward transition. The gesture's progress animation starts
81+
* from that pose, so clearing them here would cause a visible jump before
82+
* core's progress animation takes over.
83+
*/
84+
const revealIonPageForSwipeBack = (element: HTMLElement | undefined): void => {
85+
if (element) {
86+
const before = {
87+
id: element.id,
88+
inlineDisplay: element.style.display,
89+
hasHiddenClass: element.classList.contains('ion-page-hidden'),
90+
ariaHidden: element.getAttribute('aria-hidden'),
91+
computedDisplay: getComputedStyle(element).display,
92+
};
93+
element.style.removeProperty('display');
94+
element.classList.remove('ion-page-hidden');
95+
element.removeAttribute('aria-hidden');
96+
// eslint-disable-next-line no-console
97+
console.log('[SwipeBackReveal]', JSON.stringify({
98+
before,
99+
after: {
100+
inlineDisplay: element.style.display,
101+
hasHiddenClass: element.classList.contains('ion-page-hidden'),
102+
ariaHidden: element.getAttribute('aria-hidden'),
103+
computedDisplay: getComputedStyle(element).display,
104+
},
105+
}));
106+
} else {
107+
// eslint-disable-next-line no-console
108+
console.log('[SwipeBackReveal] element is undefined');
109+
}
110+
};
111+
69112
/**
70113
* A leaf view is "preservable" on browser-back (pop) when its React state
71114
* should survive a forward-pop round-trip. Non-parameterized leaf paths
@@ -113,6 +156,15 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
113156
* duplicate transitions during rapid navigation (e.g., Navigate redirects)
114157
*/
115158
private lastTransition?: { enteringId: string; leavingId?: string };
159+
/**
160+
* Views that have been explicitly kept alive by the pop-preserve logic
161+
* (shouldPreserveLeavingView) so a future forward-pop can restore their React
162+
* state. These are candidates for cleanup when a fresh push invalidates the
163+
* forward-history path that made them reachable. Views mounted through
164+
* normal forward-push (which keeps the leaving view alive by default) are
165+
* NOT tracked here.
166+
*/
167+
private preservedViewItems = new Set<ViewItem>();
116168
/** Tracks whether the component is mounted to guard async transition paths. */
117169
private _isMounted = false;
118170
/** In-flight requestAnimationFrame IDs from transitionPage, cancelled on unmount. */
@@ -505,6 +557,9 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
505557
if (!enteringViewItem.mount) {
506558
enteringViewItem.mount = true;
507559
}
560+
// A view that becomes the entering view is no longer a stale preserved view.
561+
// It's back in the active navigation path, so drop it from the cleanup set.
562+
this.preservedViewItems.delete(enteringViewItem);
508563

509564
// Check visibility state BEFORE showing entering view
510565
const enteringWasVisible = enteringViewItem.ionPageElement && isViewVisible(enteringViewItem.ionPageElement);
@@ -581,6 +636,8 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
581636
routeInfo.routeAction === 'pop' && isViewItemPreservableOnPop(leavingViewItem);
582637
if (routeInfo.routeAction !== 'replace' && !shouldPreserveLeavingView) {
583638
leavingViewItem.mount = false;
639+
} else if (shouldPreserveLeavingView) {
640+
this.preservedViewItems.add(leavingViewItem);
584641
}
585642
this.handleLeavingViewUnmount(routeInfo, enteringViewItem, leavingViewItem);
586643
}
@@ -595,9 +652,11 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
595652
}
596653

597654
/**
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.
655+
* Unmounts views previously kept alive by the pop-preserve logic when a fresh
656+
* push invalidates the forward-history path that made them reachable. Only
657+
* iterates views explicitly tracked in `preservedViewItems` so that views
658+
* naturally mounted through forward-push (the default leaving-view behavior)
659+
* are left untouched.
601660
*/
602661
private cleanupPreservedViewsOnPush(
603662
routeInfo: RouteInfo,
@@ -607,19 +666,21 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
607666
if (routeInfo.routeAction !== 'push') {
608667
return;
609668
}
669+
if (this.preservedViewItems.size === 0) {
670+
return;
671+
}
610672

611-
const allViews = this.context.getViewItemsForOutlet(this.id);
612-
for (const viewItem of allViews) {
673+
for (const viewItem of Array.from(this.preservedViewItems)) {
613674
if (viewItem === enteringViewItem || viewItem === leavingViewItem) {
675+
this.preservedViewItems.delete(viewItem);
614676
continue;
615677
}
616678
if (!viewItem.mount) {
617-
continue;
618-
}
619-
if (!isViewItemPreservableOnPop(viewItem)) {
679+
this.preservedViewItems.delete(viewItem);
620680
continue;
621681
}
622682
viewItem.mount = false;
683+
this.preservedViewItems.delete(viewItem);
623684
const viewToUnmount = viewItem;
624685
setTimeout(() => {
625686
this.context.unMountViewItem(viewToUnmount);
@@ -863,6 +924,8 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
863924
routeInfo.routeAction === 'pop' && isViewItemPreservableOnPop(leavingViewItem);
864925
if (routeInfo.routeAction !== 'replace' && !shouldPreserveLeavingView) {
865926
leavingViewItem.mount = false;
927+
} else if (shouldPreserveLeavingView) {
928+
this.preservedViewItems.add(leavingViewItem);
866929
}
867930
this.handleLeavingViewUnmount(routeInfo, enteringViewItem, leavingViewItem);
868931
}
@@ -904,6 +967,8 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
904967
routeInfo.routeAction === 'pop' && isViewItemPreservableOnPop(latestLeavingView);
905968
if (routeInfo.routeAction !== 'replace' && !shouldPreserveLeavingView) {
906969
latestLeavingView.mount = false;
970+
} else if (shouldPreserveLeavingView) {
971+
this.preservedViewItems.add(latestLeavingView);
907972
}
908973
this.handleLeavingViewUnmount(routeInfo, latestEnteringView, latestLeavingView);
909974
}
@@ -1010,6 +1075,7 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
10101075
this.outOfScopeUnmountTimeout = undefined;
10111076
}
10121077
this.waitingForIonPage = false;
1078+
this.preservedViewItems.clear();
10131079

10141080
// Hide all views in this outlet before clearing.
10151081
// This is critical for nested outlets - when the parent component unmounts,
@@ -1137,6 +1203,8 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
11371203
routeInfo.routeAction === 'pop' && isViewItemPreservableOnPop(leavingViewItem);
11381204
if (shouldUnmountLeavingViewItem && !shouldPreserveLeavingView) {
11391205
leavingViewItem.mount = false;
1206+
} else if (shouldUnmountLeavingViewItem && shouldPreserveLeavingView) {
1207+
this.preservedViewItems.add(leavingViewItem);
11401208
}
11411209
}
11421210
}
@@ -1324,6 +1392,18 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
13241392
enteringViewItem.routeData.match.pattern.path !== routeInfo.pathname &&
13251393
enteringViewItem.routeData.match.pathname !== routeInfo.pathname;
13261394

1395+
// eslint-disable-next-line no-console
1396+
console.log('[SwipeBackCanStart]', JSON.stringify({
1397+
outletId: this.id,
1398+
routePathname: routeInfo.pathname,
1399+
swipeBackPathname: swipeBackRouteInfo?.pathname,
1400+
enteringViewId: enteringViewItem?.id,
1401+
enteringViewPath: enteringViewItem?.reactElement?.props?.path,
1402+
enteringMount: enteringViewItem?.mount,
1403+
ionPageInDocument,
1404+
canStartSwipe,
1405+
}));
1406+
13271407
return canStartSwipe;
13281408
};
13291409

@@ -1333,18 +1413,45 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
13331413
const enteringViewItem = this.findEnteringViewForSwipe(swipeBackRouteInfo);
13341414
const leavingViewItem = this.context.findViewItemByRouteInfo(routeInfo, this.id, false);
13351415

1416+
// eslint-disable-next-line no-console
1417+
console.log('[SwipeBackOnStart:entry]', JSON.stringify({
1418+
outletId: this.id,
1419+
routePathname: routeInfo.pathname,
1420+
swipeBackPathname: swipeBackRouteInfo?.pathname,
1421+
enteringViewId: enteringViewItem?.id,
1422+
enteringViewPath: enteringViewItem?.reactElement?.props?.path,
1423+
enteringMount: enteringViewItem?.mount,
1424+
hasEnteringIonPageElement: !!enteringViewItem?.ionPageElement,
1425+
leavingViewId: leavingViewItem?.id,
1426+
}));
1427+
13361428
// Ensure the entering view is mounted so React keeps rendering it during the gesture.
13371429
// This is important when the view was previously marked for unmount but its
13381430
// ionPageElement is still in the DOM.
13391431
if (enteringViewItem && !enteringViewItem.mount) {
13401432
enteringViewItem.mount = true;
13411433
}
13421434

1435+
// Reveal synchronously. `transitionPage` defers this behind async commit,
1436+
// but the gesture's first progress frame fires in the same tick as onStart,
1437+
// so an async reveal leaves the entering page hidden until the next frame.
1438+
revealIonPageForSwipeBack(enteringViewItem?.ionPageElement);
1439+
13431440
// When the gesture starts, kick off a transition controlled via swipe gesture
13441441
if (enteringViewItem && leavingViewItem) {
13451442
await this.transitionPage(routeInfo, enteringViewItem, leavingViewItem, 'back', true);
13461443
}
13471444

1445+
// eslint-disable-next-line no-console
1446+
console.log('[SwipeBackOnStart:exit]', JSON.stringify({
1447+
outletId: this.id,
1448+
enteringFinalComputedDisplay: enteringViewItem?.ionPageElement
1449+
? getComputedStyle(enteringViewItem.ionPageElement).display
1450+
: null,
1451+
enteringFinalInlineDisplay: enteringViewItem?.ionPageElement?.style.display ?? null,
1452+
enteringFinalHiddenClass: enteringViewItem?.ionPageElement?.classList.contains('ion-page-hidden') ?? null,
1453+
}));
1454+
13481455
return Promise.resolve();
13491456
};
13501457

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,14 @@ const IOS_MODE = 'ionic:mode=ios';
2323
*/
2424
test.describe('ionPage outlet swipe-to-go-back', () => {
2525
test('section-a content should be visible during swipe-back from section-b', async ({ page }) => {
26+
page.on('console', (msg) => {
27+
const text = msg.text();
28+
if (text.startsWith('[SwipeBack') || text.startsWith('[HideIonPageElement]')) {
29+
// eslint-disable-next-line no-console
30+
console.log('PAGE:', text);
31+
}
32+
});
33+
2634
// Navigate to modal-aria-hidden test page (has sibling ionPage outlets)
2735
await page.goto(`/modal-aria-hidden?${IOS_MODE}`);
2836
await ionPageVisible(page, 'modal-page-a');
@@ -33,16 +41,41 @@ test.describe('ionPage outlet swipe-to-go-back', () => {
3341
await page.locator('#navigateToB').click();
3442
await ionPageVisible(page, 'modal-page-b');
3543

44+
// Pre-swipe: dump section-a state
45+
const preSwipe = await page.locator('#section-a').evaluate((el: HTMLElement) => ({
46+
inlineDisplay: el.style.display,
47+
hasHiddenClass: el.classList.contains('ion-page-hidden'),
48+
ariaHidden: el.getAttribute('aria-hidden'),
49+
classList: Array.from(el.classList),
50+
computedDisplay: getComputedStyle(el).display,
51+
}));
52+
// eslint-disable-next-line no-console
53+
console.log('PRE_SWIPE section-a:', JSON.stringify(preSwipe));
54+
3655
// Start a swipe-back gesture and hold mid-way
3756
const outlet = page.locator('ion-router-outlet#modal-aria-hidden-root');
3857
const box = await outlet.boundingBox();
3958
if (!box) throw new Error('Root outlet not found');
59+
// eslint-disable-next-line no-console
60+
console.log('OUTLET_BOX:', JSON.stringify(box));
4061

4162
await page.mouse.move(box.x, box.y + box.height / 2);
4263
await page.mouse.down();
4364
await page.mouse.move(box.x + box.width / 2, box.y + box.height / 2, { steps: 10 });
4465
await page.waitForTimeout(300);
4566

67+
// Post-swipe: dump section-a state before the assertion
68+
const postSwipe = await page.locator('#section-a').evaluate((el: HTMLElement) => ({
69+
inlineDisplay: el.style.display,
70+
hasHiddenClass: el.classList.contains('ion-page-hidden'),
71+
ariaHidden: el.getAttribute('aria-hidden'),
72+
classList: Array.from(el.classList),
73+
computedDisplay: getComputedStyle(el).display,
74+
outerBounds: el.getBoundingClientRect().toJSON(),
75+
}));
76+
// eslint-disable-next-line no-console
77+
console.log('POST_SWIPE section-a:', JSON.stringify(postSwipe));
78+
4679
// Mid-swipe: section-a should be visible (not display:none) with its child content
4780
const sectionA = page.locator('#section-a');
4881
const computedDisplay = await sectionA.evaluate((el: HTMLElement) => getComputedStyle(el).display);

0 commit comments

Comments
 (0)