Skip to content

Commit b166c89

Browse files
committed
fix(react-router): prevent stale animated commit from hiding page after concurrent tab switch
1 parent b1286fc commit b166c89

File tree

2 files changed

+51
-4
lines changed

2 files changed

+51
-4
lines changed

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

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,18 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
8686
private transitionRafIds: number[] = [];
8787
/** In-flight MutationObserver from waitForComponentsReady, disconnected on unmount. */
8888
private transitionObserver?: MutationObserver;
89+
/**
90+
* Monotonically increasing counter incremented at the start of each transitionPage call.
91+
* Used to detect when an async commit() resolves after a newer transition has already run,
92+
* preventing the stale commit from hiding an element that the newer transition made visible.
93+
*/
94+
private transitionGeneration = 0;
95+
/**
96+
* The entering element of the most recent transitionPage call.
97+
* Used alongside transitionGeneration to undo incorrect ion-page-hidden applied
98+
* by a stale animated commit that raced with a newer non-animated transition.
99+
*/
100+
private transitionEnteringElement?: HTMLElement;
89101

90102
constructor(props: StackManagerProps) {
91103
super(props);
@@ -1215,6 +1227,8 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
12151227
progressAnimation = false,
12161228
skipAnimation = false
12171229
) {
1230+
const myGeneration = ++this.transitionGeneration;
1231+
12181232
const runCommit = async (enteringEl: HTMLElement, leavingEl?: HTMLElement) => {
12191233
const skipTransition = this.skipTransition;
12201234

@@ -1268,11 +1282,24 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
12681282
const timeoutPromise = new Promise<'timeout'>((resolve) => setTimeout(() => resolve('timeout'), timeoutMs));
12691283
const result = await Promise.race([commitPromise.then(() => 'done' as const), timeoutPromise]);
12701284

1285+
// Bail out if the component unmounted during the commit animation
1286+
if (!this._isMounted) return;
1287+
12711288
if (result === 'timeout') {
12721289
// Force entering page visible even though commit hung
12731290
enteringEl.classList.remove('ion-page-invisible');
12741291
}
12751292

1293+
/**
1294+
* If a newer transitionPage call ran while this commit was in-flight (e.g., a tab
1295+
* switch fired during a forward animation), the core commit may have applied
1296+
* ion-page-hidden to leavingEl even though the newer transition already made it
1297+
* visible. Undo that stale hide so the newer transition's DOM state wins.
1298+
*/
1299+
if (myGeneration !== this.transitionGeneration && leavingEl && leavingEl === this.transitionEnteringElement) {
1300+
showIonPageElement(leavingEl);
1301+
}
1302+
12761303
if (!progressAnimation) {
12771304
enteringEl.classList.remove('ion-page-invisible');
12781305
}
@@ -1285,6 +1312,8 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
12851312
const directionToUse = direction ?? routeInfoFallbackDirection;
12861313

12871314
if (enteringViewItem && enteringViewItem.ionPageElement && this.routerOutletElement) {
1315+
this.transitionEnteringElement = enteringViewItem.ionPageElement;
1316+
12881317
if (leavingViewItem && leavingViewItem.ionPageElement && enteringViewItem === leavingViewItem) {
12891318
// Clone page for same-view transitions (e.g., /user/1 → /user/2)
12901319
const match = matchComponent(leavingViewItem.reactElement, routeInfo.pathname, undefined, this.outletMountPath);
@@ -1400,14 +1429,21 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
14001429
if (!this._isMounted) return;
14011430

14021431
// Swap visibility synchronously - show entering, hide leaving
1432+
// Skip hiding if a newer transition already made leavingEl the entering view
14031433
enteringEl.classList.remove('ion-page-invisible');
1404-
leavingEl.classList.add('ion-page-hidden');
1405-
leavingEl.setAttribute('aria-hidden', 'true');
1434+
if (myGeneration === this.transitionGeneration || leavingEl !== this.transitionEnteringElement) {
1435+
leavingEl.classList.add('ion-page-hidden');
1436+
leavingEl.setAttribute('aria-hidden', 'true');
1437+
}
14061438
} else {
14071439
await runCommit(enteringViewItem.ionPageElement, leavingEl);
14081440
if (leavingEl && !progressAnimation) {
1409-
leavingEl.classList.add('ion-page-hidden');
1410-
leavingEl.setAttribute('aria-hidden', 'true');
1441+
// Skip hiding if a newer transition already made leavingEl the entering view
1442+
// runCommit's generation check has already restored its visibility in that case
1443+
if (myGeneration === this.transitionGeneration || leavingEl !== this.transitionEnteringElement) {
1444+
leavingEl.classList.add('ion-page-hidden');
1445+
leavingEl.setAttribute('aria-hidden', 'true');
1446+
}
14111447
}
14121448
}
14131449
}

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

Lines changed: 11 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,17 @@ test.describe('Tabs', () => {
6464
await expect(page.locator('ion-tab-button.tab-selected')).toContainText('Tab1');
6565
});
6666

67+
// Verifies that an animated forward commit racing with a tab switch does not leave the home page hidden.
68+
// No withTestingMode() - animations must run for the race to be exercisable.
69+
test('should show home page after cross-tab nav then immediate tab click', async ({ page }) => {
70+
await page.goto('/routing/tabs/home');
71+
await ionPageVisible(page, 'home-page');
72+
73+
await ionNav(page, 'ion-item', 'Details 1 on settings');
74+
await ionTabClick(page, 'Home');
75+
await ionPageVisible(page, 'home-page');
76+
});
77+
6778
// Verifies that defaultHref works on direct deep-link load of a tab child page
6879
test('back button defaultHref should work on direct deep-link load of tab child page', async ({ page }) => {
6980
await page.goto(withTestingMode('/tabs/tab1/child'));

0 commit comments

Comments
 (0)