Skip to content

Commit e398970

Browse files
ShaneKShaneK
committed
test(react-router): trying to make tests more reliable
1 parent bceeff6 commit e398970

File tree

6 files changed

+125
-67
lines changed

6 files changed

+125
-67
lines changed

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

Lines changed: 81 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -28,18 +28,20 @@ import { extractRouteChildren, getRoutesChildren, isNavigateElement } from './ut
2828
const VIEW_UNMOUNT_DELAY_MS = 250;
2929

3030
/**
31-
* Delay in milliseconds to wait for an IonPage element to be mounted before
32-
* proceeding with a page transition.
31+
* Delay (ms) to wait for an IonPage to mount before proceeding with a
32+
* page transition. Only container routes (nested outlets with no direct
33+
* IonPage) actually hit this timeout; normal routes clear it early via
34+
* registerIonPage, so a larger value here doesn't affect the happy path.
3335
*/
34-
const ION_PAGE_WAIT_TIMEOUT_MS = 50;
36+
const ION_PAGE_WAIT_TIMEOUT_MS = 300;
3537

3638
interface StackManagerProps {
3739
routeInfo: RouteInfo;
3840
id?: string;
3941
}
4042

4143
const isViewVisible = (el: HTMLElement) =>
42-
!el.classList.contains('ion-page-invisible') && !el.classList.contains('ion-page-hidden') && el.style.display !== 'none';
44+
!el.classList.contains('ion-page-invisible') && !el.classList.contains('ion-page-hidden') && el.style.visibility !== 'hidden';
4345

4446
const hideIonPageElement = (element: HTMLElement | undefined): void => {
4547
if (element) {
@@ -50,7 +52,7 @@ const hideIonPageElement = (element: HTMLElement | undefined): void => {
5052

5153
const showIonPageElement = (element: HTMLElement | undefined): void => {
5254
if (element) {
53-
element.style.removeProperty('display');
55+
element.style.removeProperty('visibility');
5456
element.classList.remove('ion-page-hidden');
5557
element.removeAttribute('aria-hidden');
5658
}
@@ -446,6 +448,8 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
446448

447449
const shouldSkipAnimation = this.applySkipAnimationIfNeeded(enteringViewItem, leavingViewItem);
448450

451+
console.log(`[handleReadyEnteringView] outletId=${this.id} pathname=${routeInfo.pathname} lastPathname=${routeInfo.lastPathname} action=${routeInfo.routeAction} direction=${routeInfo.routeDirection} skipAnimation=${shouldSkipAnimation} entering=${enteringViewItem.ionPageElement?.getAttribute('data-pageid')} enteringClasses=${enteringViewItem.ionPageElement?.className} leaving=${leavingViewItem?.ionPageElement?.getAttribute('data-pageid')} leavingClasses=${leavingViewItem?.ionPageElement?.className} leavingVisibility=${leavingViewItem?.ionPageElement?.style.visibility} shouldUnmount=${shouldUnmountLeavingViewItem}`);
452+
449453
this.transitionPage(routeInfo, enteringViewItem, leavingViewItem, undefined, false, shouldSkipAnimation);
450454

451455
if (shouldUnmountLeavingViewItem && leavingViewItem && enteringViewItem !== leavingViewItem) {
@@ -586,10 +590,12 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
586590
* nested scrollbars (each page has its own IonContent). Top-level outlets
587591
* are unaffected and animate normally.
588592
*
589-
* Uses inline display:none rather than ion-page-hidden class because core's
590-
* beforeTransition() removes ion-page-hidden via setPageHidden().
591-
* Inline display:none survives that removal, keeping the page hidden
592-
* until React unmounts it after ionViewDidLeave fires.
593+
* Uses inline visibility:hidden rather than ion-page-hidden class because
594+
* core's beforeTransition() removes ion-page-hidden via setPageHidden().
595+
* Inline visibility:hidden survives that removal, keeping the page hidden
596+
* until React unmounts it after ionViewDidLeave fires. Unlike display:none,
597+
* visibility:hidden preserves element geometry so commit() animations
598+
* can resolve normally.
593599
*/
594600
private applySkipAnimationIfNeeded(
595601
enteringViewItem: ViewItem,
@@ -599,7 +605,8 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
599605
const shouldSkip = isNestedOutlet && !!leavingViewItem && enteringViewItem !== leavingViewItem;
600606

601607
if (shouldSkip && leavingViewItem?.ionPageElement) {
602-
leavingViewItem.ionPageElement.style.setProperty('display', 'none');
608+
console.log(`[applySkipAnimation] hiding leaving=${leavingViewItem.ionPageElement.getAttribute('data-pageid')} via visibility:hidden, entering=${enteringViewItem.ionPageElement?.getAttribute('data-pageid')}`);
609+
leavingViewItem.ionPageElement.style.setProperty('visibility', 'hidden');
603610
leavingViewItem.ionPageElement.setAttribute('aria-hidden', 'true');
604611
}
605612

@@ -660,13 +667,16 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
660667
this.ionPageWaitTimeout = undefined;
661668

662669
if (!this.waitingForIonPage) {
670+
console.log(`[handleWaitingForIonPage] timeout fired but no longer waiting, outletId=${this.id}`);
663671
return;
664672
}
665673
this.waitingForIonPage = false;
666674

667675
const latestEnteringView = this.context.findViewItemByRouteInfo(routeInfo, this.id) ?? enteringViewItem;
668676
const latestLeavingView = this.context.findLeavingViewItemByRouteInfo(routeInfo, this.id) ?? leavingViewItem;
669677

678+
console.log(`[handleWaitingForIonPage] timeout fired outletId=${this.id} pathname=${routeInfo.pathname} hasIonPageEl=${!!latestEnteringView?.ionPageElement} entering=${latestEnteringView?.ionPageElement?.getAttribute('data-pageid')} leaving=${latestLeavingView?.ionPageElement?.getAttribute('data-pageid')}`);
679+
670680
if (latestEnteringView?.ionPageElement) {
671681
const shouldSkipAnimation = this.applySkipAnimationIfNeeded(latestEnteringView, latestLeavingView ?? undefined);
672682
this.transitionPage(routeInfo, latestEnteringView, latestLeavingView ?? undefined, undefined, false, shouldSkipAnimation);
@@ -693,6 +703,21 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
693703
}
694704
});
695705
this.forceUpdate();
706+
707+
// Safety net: after forceUpdate triggers a React render cycle, check if
708+
// any pages in this outlet are stuck with ion-page-invisible. This can
709+
// happen when view lookup fails (e.g., wildcard-to-index transitions
710+
// where the view item gets corrupted). The forceUpdate above causes
711+
// React to render the correct component, but ion-page-invisible may
712+
// persist if no transition runs for that page.
713+
setTimeout(() => {
714+
if (!this._isMounted || !this.routerOutletElement) return;
715+
const stuckPages = this.routerOutletElement.querySelectorAll(':scope > .ion-page-invisible');
716+
stuckPages.forEach((page) => {
717+
console.log(`[handleWaitingForIonPage] clearing stuck ion-page-invisible on ${page.getAttribute('data-pageid')}`);
718+
page.classList.remove('ion-page-invisible');
719+
});
720+
}, ION_PAGE_WAIT_TIMEOUT_MS);
696721
}
697722
}, ION_PAGE_WAIT_TIMEOUT_MS);
698723

@@ -865,8 +890,10 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
865890
this.ionPageWaitTimeout = undefined;
866891
}
867892

893+
console.log(`[handlePageTransition] READY outletId=${this.id} pathname=${routeInfo.pathname} entering=${enteringViewItem.ionPageElement?.getAttribute('data-pageid')} leaving=${leavingViewItem?.ionPageElement?.getAttribute('data-pageid')} ionPageInDoc=${ionPageIsInDocument}`);
868894
this.handleReadyEnteringView(routeInfo, enteringViewItem, leavingViewItem, shouldUnmountLeavingViewItem);
869895
} else if (enteringViewItem && !ionPageIsInDocument) {
896+
console.log(`[handlePageTransition] WAITING outletId=${this.id} pathname=${routeInfo.pathname} enteringHasEl=${!!enteringViewItem.ionPageElement} ionPageInDoc=${ionPageIsInDocument}`);
870897
// Wait for ion-page to mount
871898
// This handles both: no ionPageElement, or stale ionPageElement (not in document)
872899
// Clear stale reference if the element is no longer in the document
@@ -961,6 +988,7 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
961988
return;
962989
}
963990
}
991+
console.log(`[registerIonPage] outletId=${this.id} page=${page.getAttribute('data-pageid')} pageClasses="${page.className}" pathname=${routeInfo.pathname}`);
964992
this.handlePageTransition(routeInfo);
965993
}
966994

@@ -1145,13 +1173,35 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
11451173
}
11461174
}
11471175

1148-
await routerOutlet.commit(enteringEl, leavingEl, {
1149-
duration: skipTransition || skipAnimation || directionToUse === undefined ? 0 : undefined,
1176+
const commitDuration = skipTransition || skipAnimation || directionToUse === undefined ? 0 : undefined;
1177+
console.log(`[runCommit] BEFORE commit entering=${enteringEl.getAttribute('data-pageid')} enteringClasses="${enteringEl.className}" leaving=${leavingEl?.getAttribute('data-pageid')} leavingClasses="${leavingEl?.className}" leavingDisplay="${leavingEl?.style.display}" leavingVisibility="${leavingEl?.style.visibility}" duration=${commitDuration} direction=${directionToUse} skipTransition=${skipTransition} skipAnimation=${skipAnimation}`);
1178+
const commitStart = Date.now();
1179+
1180+
// Race commit against a timeout to detect hangs
1181+
const commitPromise = routerOutlet.commit(enteringEl, leavingEl, {
1182+
duration: commitDuration,
11501183
direction: directionToUse,
11511184
showGoBack: !!routeInfo.pushedByRoute,
11521185
progressAnimation,
11531186
animationBuilder: routeInfo.routeAnimation,
11541187
});
1188+
1189+
const timeoutMs = 5000;
1190+
const timeoutPromise = new Promise<'timeout'>((resolve) => setTimeout(() => resolve('timeout'), timeoutMs));
1191+
const result = await Promise.race([commitPromise.then(() => 'done' as const), timeoutPromise]);
1192+
1193+
if (result === 'timeout') {
1194+
console.error(`[runCommit] TIMEOUT commit hung for ${timeoutMs}ms! entering=${enteringEl.getAttribute('data-pageid')} enteringClasses="${enteringEl.className}" leaving=${leavingEl?.getAttribute('data-pageid')} leavingClasses="${leavingEl?.className}" leavingDisplay="${leavingEl?.style.display}" leavingVisibility="${leavingEl?.style.visibility}"`);
1195+
// Force entering page visible even though commit hung
1196+
enteringEl.classList.remove('ion-page-invisible');
1197+
console.log(`[runCommit] forced ion-page-invisible removal on ${enteringEl.getAttribute('data-pageid')}, classes now: "${enteringEl.className}"`);
1198+
} else {
1199+
console.log(`[runCommit] AFTER commit resolved in ${Date.now() - commitStart}ms entering=${enteringEl.getAttribute('data-pageid')} enteringClasses="${enteringEl.className}"`);
1200+
}
1201+
1202+
if (!progressAnimation) {
1203+
enteringEl.classList.remove('ion-page-invisible');
1204+
}
11551205
};
11561206

11571207
const routerOutlet = this.routerOutletElement!;
@@ -1181,24 +1231,27 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
11811231
// flicker caused by commit() briefly unhiding the leaving page
11821232
const isNonAnimatedTransition = directionToUse === undefined && !progressAnimation;
11831233

1234+
console.log(`[transitionPage] outletId=${this.id} directionToUse=${directionToUse} isNonAnimatedTransition=${isNonAnimatedTransition} skipAnimation=${skipAnimation} hasLeavingEl=${!!leavingEl} entering=${enteringViewItem.ionPageElement?.getAttribute('data-pageid')} leaving=${leavingEl?.getAttribute('data-pageid')}`);
1235+
11841236
if (isNonAnimatedTransition && leavingEl) {
11851237
/**
1186-
* Flicker prevention for non-animated transitions:
1187-
* Skip commit() entirely for simple visibility swaps (like tab switches).
1188-
* commit() runs animation logic that can cause intermediate paints even with
1189-
* duration: 0. Instead, we directly swap visibility classes and wait for
1190-
* components to be ready before showing the entering element.
1238+
* Skip commit() for non-animated transitions (like tab switches).
1239+
* commit() runs animation logic that can cause intermediate paints
1240+
* even with duration: 0. Instead, swap visibility synchronously.
1241+
*
1242+
* Synchronous DOM class changes are batched into a single browser
1243+
* paint, so there's no gap frame where neither page is visible and
1244+
* no overlap frame where both pages are visible.
11911245
*/
11921246
const enteringEl = enteringViewItem.ionPageElement;
11931247

11941248
// Ensure entering element has proper base classes
11951249
enteringEl.classList.add('ion-page');
1196-
// Only add ion-page-invisible if not already visible (e.g., tab switches)
1197-
if (!isViewVisible(enteringEl)) {
1198-
enteringEl.classList.add('ion-page-invisible');
1199-
}
1200-
enteringEl.classList.remove('ion-page-hidden');
1201-
enteringEl.removeAttribute('aria-hidden');
1250+
1251+
// Clear ALL hidden state from entering element. showIonPageElement
1252+
// removes visibility:hidden (from applySkipAnimationIfNeeded),
1253+
// ion-page-hidden, and aria-hidden in one call.
1254+
showIonPageElement(enteringEl);
12021255

12031256
// Handle can-go-back class since we're skipping commit() which normally sets this
12041257
if (routeInfo.pushedByRoute) {
@@ -1274,31 +1327,13 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
12741327
// Bail out if the component unmounted during waitForComponentsReady
12751328
if (!this._isMounted) return;
12761329

1277-
// Swap visibility in sync with browser's render cycle
1278-
await new Promise<void>((resolve) => {
1279-
const outerRafId = requestAnimationFrame(() => {
1280-
this.transitionRafIds = this.transitionRafIds.filter((id) => id !== outerRafId);
1281-
if (!this._isMounted) {
1282-
resolve();
1283-
return;
1284-
}
1285-
enteringEl.classList.remove('ion-page-invisible');
1286-
// Second rAF ensures entering is painted before hiding leaving
1287-
const innerRafId = requestAnimationFrame(() => {
1288-
this.transitionRafIds = this.transitionRafIds.filter((id) => id !== innerRafId);
1289-
if (this._isMounted) {
1290-
leavingEl.classList.add('ion-page-hidden');
1291-
leavingEl.setAttribute('aria-hidden', 'true');
1292-
}
1293-
resolve();
1294-
});
1295-
this.transitionRafIds.push(innerRafId);
1296-
});
1297-
this.transitionRafIds.push(outerRafId);
1298-
});
1330+
// Swap visibility synchronously - show entering, hide leaving
1331+
enteringEl.classList.remove('ion-page-invisible');
1332+
leavingEl.classList.add('ion-page-hidden');
1333+
leavingEl.setAttribute('aria-hidden', 'true');
12991334
} else {
1335+
console.log(`[transitionPage] taking commit() path for entering=${enteringViewItem.ionPageElement?.getAttribute('data-pageid')} leaving=${leavingEl?.getAttribute('data-pageid')}`);
13001336
await runCommit(enteringViewItem.ionPageElement, leavingEl);
1301-
// For animated transitions, hide leaving element after commit completes
13021337
if (leavingEl && !progressAnimation) {
13031338
leavingEl.classList.add('ion-page-hidden');
13041339
leavingEl.setAttribute('aria-hidden', 'true');

‎packages/react-router/test/base/cypress.config.ts‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,10 @@ export default defineConfig({
55
pageLoadTimeout: 6000000000,
66
screenshotOnRunFailure: false,
77
defaultCommandTimeout: 10000,
8+
retries: {
9+
runMode: 2,
10+
openMode: 0,
11+
},
812
fixturesFolder: 'tests/e2e/fixtures',
913
screenshotsFolder: 'tests/e2e/screenshots',
1014
videosFolder: 'tests/e2e/videos',

‎packages/react-router/test/base/tests/e2e/specs/index-param-priority.cy.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -141,6 +141,8 @@ describe('Index Param Priority', () => {
141141
cy.get('[data-testid="notfound-page-label"]').should('contain', 'Page not found');
142142

143143
cy.get('#back-to-index-from-notfound').click();
144+
cy.url().should('include', '/index-param-priority');
145+
cy.wait(300);
144146
cy.ionPageVisible('index-param-priority-index');
145147
cy.get('[data-testid="index-page-label"]').should('contain', 'This is the index page');
146148
});

‎packages/react-router/test/base/tests/e2e/specs/index-route-reuse.cy.js‎

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,10 @@ describe('Index Route Reuse - Nested Outlet Index Routes', () => {
2323

2424
// Switch to Tab 2
2525
cy.ionTabClick('Tab 2');
26+
cy.url().should('include', '/index-route-reuse/tab2');
2627
cy.ionPageVisible('irr-tab2-home');
2728
cy.get('[data-testid="irr-tab2-home-content"]').should('be.visible');
2829
cy.get('[data-testid="irr-tab2-home-content"]').should('contain', 'Tab 2 Index Route Content');
29-
30-
// Verify URL changed to tab2
31-
cy.url().should('include', '/index-route-reuse/tab2');
3230
});
3331

3432
it('should show tab3 index content when switching to tab3', () => {
@@ -37,12 +35,10 @@ describe('Index Route Reuse - Nested Outlet Index Routes', () => {
3735

3836
// Switch to Tab 3
3937
cy.ionTabClick('Tab 3');
38+
cy.url().should('include', '/index-route-reuse/tab3');
4039
cy.ionPageVisible('irr-tab3-home');
4140
cy.get('[data-testid="irr-tab3-home-content"]').should('be.visible');
4241
cy.get('[data-testid="irr-tab3-home-content"]').should('contain', 'Tab 3 Index Route Content');
43-
44-
// Verify URL changed to tab3
45-
cy.url().should('include', '/index-route-reuse/tab3');
4642
});
4743

4844
it('should correctly show each tab index when cycling through all tabs', () => {
@@ -52,18 +48,21 @@ describe('Index Route Reuse - Nested Outlet Index Routes', () => {
5248

5349
// Tab 1 -> Tab 2
5450
cy.ionTabClick('Tab 2');
51+
cy.url().should('include', '/index-route-reuse/tab2');
5552
cy.ionPageVisible('irr-tab2-home');
5653
cy.get('[data-testid="irr-tab2-home-content"]').should('be.visible');
5754
cy.get('[data-testid="irr-tab2-home-content"]').should('contain', 'Tab 2 Index Route Content');
5855

5956
// Tab 2 -> Tab 3
6057
cy.ionTabClick('Tab 3');
58+
cy.url().should('include', '/index-route-reuse/tab3');
6159
cy.ionPageVisible('irr-tab3-home');
6260
cy.get('[data-testid="irr-tab3-home-content"]').should('be.visible');
6361
cy.get('[data-testid="irr-tab3-home-content"]').should('contain', 'Tab 3 Index Route Content');
6462

6563
// Tab 3 -> Tab 1 (back to start)
6664
cy.ionTabClick('Tab 1');
65+
cy.url().should('include', '/index-route-reuse/tab1');
6766
cy.ionPageVisible('irr-tab1-home');
6867
cy.get('[data-testid="irr-tab1-home-content"]').should('be.visible');
6968
cy.get('[data-testid="irr-tab1-home-content"]').should('contain', 'Tab 1 Index Route Content');
@@ -80,11 +79,13 @@ describe('Index Route Reuse - Nested Outlet Index Routes', () => {
8079

8180
// Switch to Tab 2
8281
cy.ionTabClick('Tab 2');
82+
cy.url().should('include', '/index-route-reuse/tab2');
8383
cy.ionPageVisible('irr-tab2-home');
8484
cy.get('[data-testid="irr-tab2-home-content"]').should('be.visible');
8585

8686
// Switch back to Tab 1 - should show detail (preserved history)
8787
cy.ionTabClick('Tab 1');
88+
cy.url().should('include', '/index-route-reuse/tab1');
8889
cy.ionPageVisible('irr-tab1-detail');
8990
cy.get('[data-testid="irr-tab1-detail-content"]').should('be.visible');
9091
});
@@ -95,17 +96,21 @@ describe('Index Route Reuse - Nested Outlet Index Routes', () => {
9596

9697
// Rapid switching: Tab1 -> Tab2 -> Tab3 -> Tab2 -> Tab1
9798
cy.ionTabClick('Tab 2');
99+
cy.url().should('include', '/index-route-reuse/tab2');
98100
cy.ionPageVisible('irr-tab2-home');
99101

100102
cy.ionTabClick('Tab 3');
103+
cy.url().should('include', '/index-route-reuse/tab3');
101104
cy.ionPageVisible('irr-tab3-home');
102105

103106
cy.ionTabClick('Tab 2');
107+
cy.url().should('include', '/index-route-reuse/tab2');
104108
cy.ionPageVisible('irr-tab2-home');
105109
cy.get('[data-testid="irr-tab2-home-content"]').should('be.visible');
106110
cy.get('[data-testid="irr-tab2-home-content"]').should('contain', 'Tab 2 Index Route Content');
107111

108112
cy.ionTabClick('Tab 1');
113+
cy.url().should('include', '/index-route-reuse/tab1');
109114
cy.ionPageVisible('irr-tab1-home');
110115
cy.get('[data-testid="irr-tab1-home-content"]').should('be.visible');
111116
cy.get('[data-testid="irr-tab1-home-content"]').should('contain', 'Tab 1 Index Route Content');

0 commit comments

Comments
 (0)