Skip to content

Commit 9826d0c

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

File tree

6 files changed

+106
-82
lines changed

6 files changed

+106
-82
lines changed

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

Lines changed: 62 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
}
@@ -586,10 +588,12 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
586588
* nested scrollbars (each page has its own IonContent). Top-level outlets
587589
* are unaffected and animate normally.
588590
*
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.
591+
* Uses inline visibility:hidden rather than ion-page-hidden class because
592+
* core's beforeTransition() removes ion-page-hidden via setPageHidden().
593+
* Inline visibility:hidden survives that removal, keeping the page hidden
594+
* until React unmounts it after ionViewDidLeave fires. Unlike display:none,
595+
* visibility:hidden preserves element geometry so commit() animations
596+
* can resolve normally.
593597
*/
594598
private applySkipAnimationIfNeeded(
595599
enteringViewItem: ViewItem,
@@ -599,7 +603,7 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
599603
const shouldSkip = isNestedOutlet && !!leavingViewItem && enteringViewItem !== leavingViewItem;
600604

601605
if (shouldSkip && leavingViewItem?.ionPageElement) {
602-
leavingViewItem.ionPageElement.style.setProperty('display', 'none');
606+
leavingViewItem.ionPageElement.style.setProperty('visibility', 'hidden');
603607
leavingViewItem.ionPageElement.setAttribute('aria-hidden', 'true');
604608
}
605609

@@ -693,6 +697,20 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
693697
}
694698
});
695699
this.forceUpdate();
700+
701+
// Safety net: after forceUpdate triggers a React render cycle, check if
702+
// any pages in this outlet are stuck with ion-page-invisible. This can
703+
// happen when view lookup fails (e.g., wildcard-to-index transitions
704+
// where the view item gets corrupted). The forceUpdate above causes
705+
// React to render the correct component, but ion-page-invisible may
706+
// persist if no transition runs for that page.
707+
setTimeout(() => {
708+
if (!this._isMounted || !this.routerOutletElement) return;
709+
const stuckPages = this.routerOutletElement.querySelectorAll(':scope > .ion-page-invisible');
710+
stuckPages.forEach((page) => {
711+
page.classList.remove('ion-page-invisible');
712+
});
713+
}, ION_PAGE_WAIT_TIMEOUT_MS);
696714
}
697715
}, ION_PAGE_WAIT_TIMEOUT_MS);
698716

@@ -1145,13 +1163,29 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
11451163
}
11461164
}
11471165

1148-
await routerOutlet.commit(enteringEl, leavingEl, {
1149-
duration: skipTransition || skipAnimation || directionToUse === undefined ? 0 : undefined,
1166+
const commitDuration = skipTransition || skipAnimation || directionToUse === undefined ? 0 : undefined;
1167+
1168+
// Race commit against a timeout to recover from hangs
1169+
const commitPromise = routerOutlet.commit(enteringEl, leavingEl, {
1170+
duration: commitDuration,
11501171
direction: directionToUse,
11511172
showGoBack: !!routeInfo.pushedByRoute,
11521173
progressAnimation,
11531174
animationBuilder: routeInfo.routeAnimation,
11541175
});
1176+
1177+
const timeoutMs = 5000;
1178+
const timeoutPromise = new Promise<'timeout'>((resolve) => setTimeout(() => resolve('timeout'), timeoutMs));
1179+
const result = await Promise.race([commitPromise.then(() => 'done' as const), timeoutPromise]);
1180+
1181+
if (result === 'timeout') {
1182+
// Force entering page visible even though commit hung
1183+
enteringEl.classList.remove('ion-page-invisible');
1184+
}
1185+
1186+
if (!progressAnimation) {
1187+
enteringEl.classList.remove('ion-page-invisible');
1188+
}
11551189
};
11561190

11571191
const routerOutlet = this.routerOutletElement!;
@@ -1183,22 +1217,23 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
11831217

11841218
if (isNonAnimatedTransition && leavingEl) {
11851219
/**
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.
1220+
* Skip commit() for non-animated transitions (like tab switches).
1221+
* commit() runs animation logic that can cause intermediate paints
1222+
* even with duration: 0. Instead, swap visibility synchronously.
1223+
*
1224+
* Synchronous DOM class changes are batched into a single browser
1225+
* paint, so there's no gap frame where neither page is visible and
1226+
* no overlap frame where both pages are visible.
11911227
*/
11921228
const enteringEl = enteringViewItem.ionPageElement;
11931229

11941230
// Ensure entering element has proper base classes
11951231
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');
1232+
1233+
// Clear ALL hidden state from entering element. showIonPageElement
1234+
// removes visibility:hidden (from applySkipAnimationIfNeeded),
1235+
// ion-page-hidden, and aria-hidden in one call.
1236+
showIonPageElement(enteringEl);
12021237

12031238
// Handle can-go-back class since we're skipping commit() which normally sets this
12041239
if (routeInfo.pushedByRoute) {
@@ -1274,31 +1309,12 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
12741309
// Bail out if the component unmounted during waitForComponentsReady
12751310
if (!this._isMounted) return;
12761311

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-
});
1312+
// Swap visibility synchronously - show entering, hide leaving
1313+
enteringEl.classList.remove('ion-page-invisible');
1314+
leavingEl.classList.add('ion-page-hidden');
1315+
leavingEl.setAttribute('aria-hidden', 'true');
12991316
} else {
13001317
await runCommit(enteringViewItem.ionPageElement, leavingEl);
1301-
// For animated transitions, hide leaving element after commit completes
13021318
if (leavingEl && !progressAnimation) {
13031319
leavingEl.classList.add('ion-page-hidden');
13041320
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"');

0 commit comments

Comments
 (0)