Skip to content

Commit 9837b67

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

File tree

6 files changed

+94
-34
lines changed

6 files changed

+94
-34
lines changed

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

Lines changed: 50 additions & 13 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);
@@ -865,8 +875,10 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
865875
this.ionPageWaitTimeout = undefined;
866876
}
867877

878+
console.log(`[handlePageTransition] READY outletId=${this.id} pathname=${routeInfo.pathname} entering=${enteringViewItem.ionPageElement?.getAttribute('data-pageid')} leaving=${leavingViewItem?.ionPageElement?.getAttribute('data-pageid')} ionPageInDoc=${ionPageIsInDocument}`);
868879
this.handleReadyEnteringView(routeInfo, enteringViewItem, leavingViewItem, shouldUnmountLeavingViewItem);
869880
} else if (enteringViewItem && !ionPageIsInDocument) {
881+
console.log(`[handlePageTransition] WAITING outletId=${this.id} pathname=${routeInfo.pathname} enteringHasEl=${!!enteringViewItem.ionPageElement} ionPageInDoc=${ionPageIsInDocument}`);
870882
// Wait for ion-page to mount
871883
// This handles both: no ionPageElement, or stale ionPageElement (not in document)
872884
// Clear stale reference if the element is no longer in the document
@@ -961,6 +973,7 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
961973
return;
962974
}
963975
}
976+
console.log(`[registerIonPage] outletId=${this.id} page=${page.getAttribute('data-pageid')} pageClasses="${page.className}" pathname=${routeInfo.pathname}`);
964977
this.handlePageTransition(routeInfo);
965978
}
966979

@@ -1145,13 +1158,35 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
11451158
}
11461159
}
11471160

1148-
await routerOutlet.commit(enteringEl, leavingEl, {
1149-
duration: skipTransition || skipAnimation || directionToUse === undefined ? 0 : undefined,
1161+
const commitDuration = skipTransition || skipAnimation || directionToUse === undefined ? 0 : undefined;
1162+
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}`);
1163+
const commitStart = Date.now();
1164+
1165+
// Race commit against a timeout to detect hangs
1166+
const commitPromise = routerOutlet.commit(enteringEl, leavingEl, {
1167+
duration: commitDuration,
11501168
direction: directionToUse,
11511169
showGoBack: !!routeInfo.pushedByRoute,
11521170
progressAnimation,
11531171
animationBuilder: routeInfo.routeAnimation,
11541172
});
1173+
1174+
const timeoutMs = 5000;
1175+
const timeoutPromise = new Promise<'timeout'>((resolve) => setTimeout(() => resolve('timeout'), timeoutMs));
1176+
const result = await Promise.race([commitPromise.then(() => 'done' as const), timeoutPromise]);
1177+
1178+
if (result === 'timeout') {
1179+
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}"`);
1180+
// Force entering page visible even though commit hung
1181+
enteringEl.classList.remove('ion-page-invisible');
1182+
console.log(`[runCommit] forced ion-page-invisible removal on ${enteringEl.getAttribute('data-pageid')}, classes now: "${enteringEl.className}"`);
1183+
} else {
1184+
console.log(`[runCommit] AFTER commit resolved in ${Date.now() - commitStart}ms entering=${enteringEl.getAttribute('data-pageid')} enteringClasses="${enteringEl.className}"`);
1185+
}
1186+
1187+
if (!progressAnimation) {
1188+
enteringEl.classList.remove('ion-page-invisible');
1189+
}
11551190
};
11561191

11571192
const routerOutlet = this.routerOutletElement!;
@@ -1181,6 +1216,8 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
11811216
// flicker caused by commit() briefly unhiding the leaving page
11821217
const isNonAnimatedTransition = directionToUse === undefined && !progressAnimation;
11831218

1219+
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')}`);
1220+
11841221
if (isNonAnimatedTransition && leavingEl) {
11851222
/**
11861223
* Flicker prevention for non-animated transitions:
@@ -1297,8 +1334,8 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
12971334
this.transitionRafIds.push(outerRafId);
12981335
});
12991336
} else {
1337+
console.log(`[transitionPage] taking commit() path for entering=${enteringViewItem.ionPageElement?.getAttribute('data-pageid')} leaving=${leavingEl?.getAttribute('data-pageid')}`);
13001338
await runCommit(enteringViewItem.ionPageElement, leavingEl);
1301-
// For animated transitions, hide leaving element after commit completes
13021339
if (leavingEl && !progressAnimation) {
13031340
leavingEl.classList.add('ion-page-hidden');
13041341
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');

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

Lines changed: 11 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -17,15 +17,14 @@ describe('Non-linear POP Forward Navigation', () => {
1717

1818
// Browser back - this is a non-linear POP because settings route has no pushedByRoute.
1919
// The else branch should push the current location key onto forwardStack.
20-
cy.go('back');
20+
cy.ionGoBack('/routing/tabs/home/details/1');
2121
cy.ionPageVisible('home-details-page-1');
2222
cy.get('ion-tab-button.tab-selected').contains('Home');
2323

2424
// Browser forward - should be detected as forward navigation via forwardStack.
2525
// Without the fix, forwardStack is empty so this falls into the wrong branch
2626
// (else-if, treating it as back navigation with wrong animation).
27-
cy.go('forward');
28-
cy.wait(500);
27+
cy.ionGoForward('/routing/tabs/settings');
2928
cy.ionPageVisible('settings-page');
3029
cy.get('ion-tab-button.tab-selected').contains('Settings');
3130
});
@@ -43,27 +42,25 @@ describe('Non-linear POP Forward Navigation', () => {
4342
cy.ionPageVisible('settings-page');
4443

4544
// Browser back (non-linear POP)
46-
cy.go('back');
45+
cy.ionGoBack('/routing/tabs/home/details/1');
4746
cy.ionPageVisible('home-details-page-1');
4847

4948
// Browser forward
50-
cy.go('forward');
51-
cy.wait(500);
49+
cy.ionGoForward('/routing/tabs/settings');
5250
cy.ionPageVisible('settings-page');
5351

5452
// Browser back again - without the fix, the forward stack is corrupted from
5553
// the previous misclassification, causing this back to be treated as forward.
56-
cy.go('back');
54+
cy.ionGoBack('/routing/tabs/home/details/1');
5755
cy.ionPageVisible('home-details-page-1');
5856
cy.get('ion-tab-button.tab-selected').contains('Home');
5957

6058
// One more forward/back cycle to verify stack integrity
61-
cy.go('forward');
62-
cy.wait(500);
59+
cy.ionGoForward('/routing/tabs/settings');
6360
cy.ionPageVisible('settings-page');
6461
cy.get('ion-tab-button.tab-selected').contains('Settings');
6562

66-
cy.go('back');
63+
cy.ionGoBack('/routing/tabs/home/details/1');
6764
cy.ionPageVisible('home-details-page-1');
6865
cy.get('ion-tab-button.tab-selected').contains('Home');
6966
});
@@ -81,22 +78,21 @@ describe('Non-linear POP Forward Navigation', () => {
8178
cy.ionPageVisible('settings-page');
8279

8380
// Browser back (non-linear POP)
84-
cy.go('back');
81+
cy.ionGoBack('/routing/tabs/home/details/1');
8582
cy.ionPageVisible('home-details-page-1');
8683

8784
// Browser forward to Settings
88-
cy.go('forward');
89-
cy.wait(500);
85+
cy.ionGoForward('/routing/tabs/settings');
9086
cy.ionPageVisible('settings-page');
9187

9288
// Browser back to D1
93-
cy.go('back');
89+
cy.ionGoBack('/routing/tabs/home/details/1');
9490
cy.ionPageVisible('home-details-page-1');
9591

9692
// Browser back to Home. The D1 route lost its pushedByRoute when it was
9793
// recreated via a non-linear POP, so this back also goes through the
9894
// non-linear else branch.
99-
cy.go('back');
95+
cy.ionGoBack('/routing/tabs/home');
10096
cy.ionPageVisible('home-page');
10197
cy.contains('[data-pageid=home-page]', '"routeAction":"pop"');
10298
cy.contains('[data-pageid=home-page]', '"pathname":"/routing/tabs/home"');

‎packages/react-router/test/base/tests/e2e/support/commands.js‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -89,6 +89,22 @@ Cypress.Commands.add('ionTabClick', (tabText) => {
8989
cy.contains('ion-tab-button', tabText).click({ force: true });
9090
});
9191

92+
Cypress.Commands.add('ionGoBack', (expectedUrlPart) => {
93+
cy.go('back');
94+
if (expectedUrlPart) {
95+
cy.url().should('include', expectedUrlPart);
96+
}
97+
cy.wait(300);
98+
});
99+
100+
Cypress.Commands.add('ionGoForward', (expectedUrlPart) => {
101+
cy.go('forward');
102+
if (expectedUrlPart) {
103+
cy.url().should('include', expectedUrlPart);
104+
}
105+
cy.wait(300);
106+
});
107+
92108
Cypress.Commands.add('ionBackClick', (pageId) => {
93109
cy.get(`div.ion-page[data-pageid=${pageId}]`)
94110
.should('be.visible', true)

0 commit comments

Comments
 (0)