Skip to content

Commit f01e9a7

Browse files
authored
fix(react-router): dispatch view lifecycle events on non-animated transitions (#31497)
Issue number: resolves #31479 --------- <!-- Please do not submit updates to dependencies unless it fixes an issue. --> <!-- Please try to limit your pull request to one type (bugfix, feature, etc). Submit multiple pull requests if needed. --> ## What is the current behavior? Currently, switching tabs or following a link with `routerDirection="none"` fires none of the four view lifecycle events, so a page that loads its data in `useIonViewWillEnter` renders empty. The non-animated branch of `StackManager.transitionPage()` skips `routerOutlet.commit()` and swaps the page classes itself to avoid intermediate paints, and core dispatches all four events from inside `commit()`, so they never fire. A capturing listener on the `ion-router-outlet` gets nothing either, so it isn't a problem with the hooks. ## What is the new behavior? That branch now dispatches the four events itself, in the same order core's `transition()` uses. The class swap moved to after all four, so the leaving page is still on screen for its leave events and the entering page is revealed only once they've fired. That's what core does on the animated path, where `beforeTransition` un-hides the leaving page and `ion-page-hidden` only goes back on after `commit()` resolves, and it matters because `ion-page-hidden` is `display: none`, so a `useIonViewDidLeave` handler reading `scrollTop` off the outgoing page was getting 0. The two dispatch sites that already existed in this file for out-of-scope and root navigation now share the same helper. ## Does this introduce a breaking change? - [ ] Yes - [X] No ## Other information The events used to come from core here. This path called `commit(enteringEl, undefined, { duration: 0 })` until [12f0d5e](12f0d5e) dropped it to fix a white flash during tab switches, and with no leaving element core took its `noAnimation` branch and fired the enter pair, so the enter half of this is a regression from that commit. Putting the call back would get the events plus focus and z-index handling from core for free, but it risks the flash it was removed for, so this dispatches them directly instead. The patch on the issue fires the enter events unconditionally. This gates them on the generation check so a superseded transition doesn't announce an entry, and it skips the leave events when the leaving page already has `ion-page-hidden`, because two transitions sharing a leaving element would otherwise run its `useIonViewDidLeave` teardown twice. The check is the class rather than `isViewVisible` since a nested outlet marks its leaving page `visibility: hidden` before we get here and still needs `ionViewDidLeave` to unmount. Preview: - [Tab lifecycle](https://ionic-framework-git-fix-rr6-lifecycle-non-animated-ionic1.vercel.app/react-router/tab-lifecycle/home) - [routerDirection="none"](https://ionic-framework-git-fix-rr6-lifecycle-non-animated-ionic1.vercel.app/react-router/direction-none-back/a) ## Current Dev Build ``` 9.0.6-dev.11790615145.1d8c56a2 ```
1 parent 058ba05 commit f01e9a7

10 files changed

Lines changed: 223 additions & 34 deletions

File tree

‎packages/react-router/package-lock.json‎

Lines changed: 1 addition & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎packages/react-router/package.json‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
"dist/"
3838
],
3939
"dependencies": {
40+
"@ionic/core": "^9.0.5",
4041
"@ionic/react": "^9.0.5",
4142
"tslib": "*"
4243
},

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

Lines changed: 51 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,12 @@
44
* particularly with animations and swipe gestures.
55
*/
66

7+
import {
8+
LIFECYCLE_DID_ENTER,
9+
LIFECYCLE_DID_LEAVE,
10+
LIFECYCLE_WILL_ENTER,
11+
LIFECYCLE_WILL_LEAVE,
12+
} from '@ionic/core/components';
713
import type { RouteInfo, StackContextState, ViewItem } from '@ionic/react';
814
import { IonRoute, RouteManagerContext, StackContext, createDebugLogger, generateId, getConfig } from '@ionic/react';
915
import React from 'react';
@@ -89,6 +95,19 @@ const revealIonPageForSwipeBack = (element: HTMLElement | undefined): void => {
8995
}
9096
};
9197

98+
type ViewLifecycleEvent =
99+
| typeof LIFECYCLE_WILL_ENTER
100+
| typeof LIFECYCLE_DID_ENTER
101+
| typeof LIFECYCLE_WILL_LEAVE
102+
| typeof LIFECYCLE_DID_LEAVE;
103+
104+
/** Dispatches a view lifecycle event the way core's `lifecycle()` does. */
105+
const dispatchLifecycleEvent = (element: HTMLElement | undefined, eventName: ViewLifecycleEvent): void => {
106+
if (element) {
107+
element.dispatchEvent(new CustomEvent(eventName, { bubbles: false, cancelable: false }));
108+
}
109+
};
110+
92111
/**
93112
* A leaf view is "preservable" on browser-back (pop) when its React state
94113
* should survive a forward-pop round-trip. Non-parameterized leaf paths
@@ -367,12 +386,8 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
367386
const allViewsInOutlet = this.context.getViewItemsForOutlet(this.id);
368387
allViewsInOutlet.forEach((viewItem) => {
369388
if (viewItem.ionPageElement && isViewVisible(viewItem.ionPageElement)) {
370-
viewItem.ionPageElement.dispatchEvent(
371-
new CustomEvent('ionViewWillLeave', { bubbles: false, cancelable: false })
372-
);
373-
viewItem.ionPageElement.dispatchEvent(
374-
new CustomEvent('ionViewDidLeave', { bubbles: false, cancelable: false })
375-
);
389+
dispatchLifecycleEvent(viewItem.ionPageElement, LIFECYCLE_WILL_LEAVE);
390+
dispatchLifecycleEvent(viewItem.ionPageElement, LIFECYCLE_DID_LEAVE);
376391
}
377392
});
378393

@@ -409,12 +424,8 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
409424
return;
410425
}
411426
if (viewItem.ionPageElement && isViewVisible(viewItem.ionPageElement)) {
412-
viewItem.ionPageElement.dispatchEvent(
413-
new CustomEvent('ionViewWillLeave', { bubbles: false, cancelable: false })
414-
);
415-
viewItem.ionPageElement.dispatchEvent(
416-
new CustomEvent('ionViewDidLeave', { bubbles: false, cancelable: false })
417-
);
427+
dispatchLifecycleEvent(viewItem.ionPageElement, LIFECYCLE_WILL_LEAVE);
428+
dispatchLifecycleEvent(viewItem.ionPageElement, LIFECYCLE_DID_LEAVE);
418429
}
419430
this.context.unMountViewItem(viewItem);
420431
});
@@ -1782,10 +1793,36 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
17821793
// Bail out if the component unmounted during waitForComponentsReady
17831794
if (!this._isMounted) return;
17841795

1796+
const isCurrent = myGeneration === this.transitionGeneration;
1797+
// A page the newest transition is entering is not leaving after all.
1798+
const isLeaving = isCurrent || leavingEl !== this.transitionEnteringElement;
1799+
// Already hidden means the leave events have fired. This checks the class rather
1800+
// than `isViewVisible` because a nested outlet marks its leaving page
1801+
// `visibility: hidden` before we get here and still needs `ionViewDidLeave`.
1802+
const announceLeaving = isLeaving && !leavingEl.classList.contains('ion-page-hidden');
1803+
1804+
/**
1805+
* Dispatch the lifecycle events, since we skipped `commit()`. Only the
1806+
* newest transition fires the entering events, and the class swap follows
1807+
* all four so the ordering matches core's `transition()`.
1808+
*
1809+
* These run after `waitForComponentsReady` because on a first mount the
1810+
* page has not attached its listeners yet.
1811+
*/
1812+
if (announceLeaving) {
1813+
dispatchLifecycleEvent(leavingEl, LIFECYCLE_WILL_LEAVE);
1814+
}
1815+
if (isCurrent) {
1816+
dispatchLifecycleEvent(enteringEl, LIFECYCLE_WILL_ENTER);
1817+
dispatchLifecycleEvent(enteringEl, LIFECYCLE_DID_ENTER);
1818+
}
1819+
if (announceLeaving) {
1820+
dispatchLifecycleEvent(leavingEl, LIFECYCLE_DID_LEAVE);
1821+
}
1822+
17851823
// Swap visibility synchronously - show entering, hide leaving
1786-
// Skip hiding if a newer transition already made leavingEl the entering view
17871824
enteringEl.classList.remove('ion-page-invisible');
1788-
if (myGeneration === this.transitionGeneration || leavingEl !== this.transitionEnteringElement) {
1825+
if (isLeaving) {
17891826
leavingEl.classList.add('ion-page-hidden');
17901827
leavingEl.setAttribute('aria-hidden', 'true');
17911828
}

‎packages/react-router/test/base/src/pages/direction-none-back/DirectionNoneBack.tsx‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,18 +8,28 @@ import {
88
IonRouterOutlet,
99
IonBackButton,
1010
IonButtons,
11+
useIonViewDidEnter,
12+
useIonViewDidLeave,
13+
useIonViewWillEnter,
14+
useIonViewWillLeave,
1115
} from '@ionic/react';
1216
import React from 'react';
1317
import { Route, Navigate } from 'react-router-dom';
1418

1519
import TestDescription from '../../components/TestDescription';
20+
import { pushLifecycleEvent } from '../../utils';
1621

1722
/**
1823
* Tests that IonBackButton works correctly after navigating with
1924
* routerDirection="none". The back button should use history to
2025
* determine the previous page, not fall back to defaultHref.
2126
*/
2227
const PageA: React.FC = () => {
28+
useIonViewWillEnter(() => pushLifecycleEvent('a:ionViewWillEnter'));
29+
useIonViewDidEnter(() => pushLifecycleEvent('a:ionViewDidEnter'));
30+
useIonViewWillLeave(() => pushLifecycleEvent('a:ionViewWillLeave'));
31+
useIonViewDidLeave(() => pushLifecycleEvent('a:ionViewDidLeave'));
32+
2333
return (
2434
<IonPage data-pageid="direction-none-page-a">
2535
<IonHeader>
@@ -41,6 +51,11 @@ const PageA: React.FC = () => {
4151
};
4252

4353
const PageB: React.FC = () => {
54+
useIonViewWillEnter(() => pushLifecycleEvent('b:ionViewWillEnter'));
55+
useIonViewDidEnter(() => pushLifecycleEvent('b:ionViewDidEnter'));
56+
useIonViewWillLeave(() => pushLifecycleEvent('b:ionViewWillLeave'));
57+
useIonViewDidLeave(() => pushLifecycleEvent('b:ionViewDidLeave'));
58+
4459
return (
4560
<IonPage data-pageid="direction-none-page-b">
4661
<IonHeader>

‎packages/react-router/test/base/src/pages/tab-lifecycle/TabLifecycle.tsx‎

Lines changed: 9 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -21,11 +21,7 @@ import React from 'react';
2121
import { Route, Navigate } from 'react-router';
2222

2323
import TestDescription from '../../components/TestDescription';
24-
25-
const pushEvent = (event: string) => {
26-
(window as any).lifecycleEvents = (window as any).lifecycleEvents || [];
27-
(window as any).lifecycleEvents.push(event);
28-
};
24+
import { pushLifecycleEvent } from '../../utils';
2925

3026
const TabLifecycle: React.FC = () => {
3127
return (
@@ -50,10 +46,10 @@ const TabLifecycle: React.FC = () => {
5046
};
5147

5248
const HomeTab: React.FC = () => {
53-
useIonViewWillEnter(() => pushEvent('home:ionViewWillEnter'));
54-
useIonViewDidEnter(() => pushEvent('home:ionViewDidEnter'));
55-
useIonViewWillLeave(() => pushEvent('home:ionViewWillLeave'));
56-
useIonViewDidLeave(() => pushEvent('home:ionViewDidLeave'));
49+
useIonViewWillEnter(() => pushLifecycleEvent('home:ionViewWillEnter'));
50+
useIonViewDidEnter(() => pushLifecycleEvent('home:ionViewDidEnter'));
51+
useIonViewWillLeave(() => pushLifecycleEvent('home:ionViewWillLeave'));
52+
useIonViewDidLeave(() => pushLifecycleEvent('home:ionViewDidLeave'));
5753

5854
return (
5955
<IonPage data-pageid="tab-lifecycle-home">
@@ -73,10 +69,10 @@ const HomeTab: React.FC = () => {
7369
};
7470

7571
const SettingsTab: React.FC = () => {
76-
useIonViewWillEnter(() => pushEvent('settings:ionViewWillEnter'));
77-
useIonViewDidEnter(() => pushEvent('settings:ionViewDidEnter'));
78-
useIonViewWillLeave(() => pushEvent('settings:ionViewWillLeave'));
79-
useIonViewDidLeave(() => pushEvent('settings:ionViewDidLeave'));
72+
useIonViewWillEnter(() => pushLifecycleEvent('settings:ionViewWillEnter'));
73+
useIonViewDidEnter(() => pushLifecycleEvent('settings:ionViewDidEnter'));
74+
useIonViewWillLeave(() => pushLifecycleEvent('settings:ionViewWillLeave'));
75+
useIonViewDidLeave(() => pushLifecycleEvent('settings:ionViewDidLeave'));
8076

8177
return (
8278
<IonPage data-pageid="tab-lifecycle-settings">
Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,2 +1,3 @@
11
export * from './generateId';
2+
export * from './lifecycleEvents';
23
export * from './dev';
Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
/** Records a view lifecycle event on `window.lifecycleEvents` for a spec to assert on. */
2+
export const pushLifecycleEvent = (event: string) => {
3+
(window as any).lifecycleEvents = (window as any).lifecycleEvents || [];
4+
(window as any).lifecycleEvents.push(event);
5+
};
Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
import { test, expect, type Page } from '@playwright/test';
2+
import { ionPageVisible, resetLifecycleEvents, settledLifecycleEvents, withTestingMode } from './utils/test-utils';
3+
4+
/**
5+
* A navigation with routerDirection="none" is not animated, but it must still
6+
* fire the four view lifecycle events, in the same order an animated one does.
7+
*/
8+
test.describe('routerDirection="none" lifecycle events', () => {
9+
const expectedEvents = ['a:ionViewWillLeave', 'b:ionViewWillEnter', 'b:ionViewDidEnter', 'a:ionViewDidLeave'];
10+
11+
const goToPageA = async (page: Page) => {
12+
await page.goto(withTestingMode('/direction-none-back/a'));
13+
await ionPageVisible(page, 'direction-none-page-a');
14+
await resetLifecycleEvents(page);
15+
};
16+
17+
test('should fire enter and leave events on a routerDirection="none" navigation', async ({ page }, testInfo) => {
18+
testInfo.annotations.push({
19+
type: 'issue',
20+
description: 'https://github.com/ionic-team/ionic-framework/issues/31479',
21+
});
22+
23+
await goToPageA(page);
24+
25+
await page.locator('#go-none').click();
26+
await ionPageVisible(page, 'direction-none-page-b');
27+
28+
expect(await settledLifecycleEvents(page)).toEqual(expectedEvents);
29+
});
30+
31+
/**
32+
* The control. A forward navigation keeps its direction, so it takes the
33+
* regular transition path, and its event order is the one the test above
34+
* has to match.
35+
*/
36+
test('should fire the same events for a forward navigation', async ({ page }) => {
37+
await goToPageA(page);
38+
39+
await page.locator('#go-forward').click();
40+
await ionPageVisible(page, 'direction-none-page-b');
41+
42+
expect(await settledLifecycleEvents(page)).toEqual(expectedEvents);
43+
});
44+
});

‎packages/react-router/test/base/tests/e2e/playwright/tab-lifecycle.spec.ts‎

Lines changed: 61 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,12 @@
11
import { test, expect } from '@playwright/test';
2-
import { ionPageVisible, ionTabClick, trackPeakMatchCount, withTestingMode } from './utils/test-utils';
2+
import {
3+
ionPageVisible,
4+
ionTabClick,
5+
resetLifecycleEvents,
6+
settledLifecycleEvents,
7+
trackPeakMatchCount,
8+
withTestingMode,
9+
} from './utils/test-utils';
310

411
test.describe('Tab Lifecycle Events', () => {
512
test.beforeEach(async ({ page }) => {
@@ -17,12 +24,12 @@ test.describe('Tab Lifecycle Events', () => {
1724
await page.goto(withTestingMode('/tab-lifecycle/home'));
1825
await ionPageVisible(page, 'tab-lifecycle-home');
1926

20-
await page.evaluate(() => { (window as any).lifecycleEvents = []; });
27+
await resetLifecycleEvents(page);
2128

2229
await page.locator('#go-outside').click();
2330
await ionPageVisible(page, 'tab-lifecycle-outside');
2431

25-
const events = await page.evaluate(() => (window as any).lifecycleEvents as string[]);
32+
const events = await settledLifecycleEvents(page);
2633
expect(events).toContain('home:ionViewWillLeave');
2734
expect(events).toContain('home:ionViewDidLeave');
2835
});
@@ -39,12 +46,12 @@ test.describe('Tab Lifecycle Events', () => {
3946
await ionTabClick(page, 'Settings');
4047
await ionPageVisible(page, 'tab-lifecycle-settings');
4148

42-
await page.evaluate(() => { (window as any).lifecycleEvents = []; });
49+
await resetLifecycleEvents(page);
4350

4451
await page.locator('#go-outside-settings').click();
4552
await ionPageVisible(page, 'tab-lifecycle-outside');
4653

47-
const events = await page.evaluate(() => (window as any).lifecycleEvents as string[]);
54+
const events = await settledLifecycleEvents(page);
4855
expect(events).toContain('settings:ionViewWillLeave');
4956
expect(events).toContain('settings:ionViewDidLeave');
5057
});
@@ -61,16 +68,63 @@ test.describe('Tab Lifecycle Events', () => {
6168
await page.locator('#go-outside').click();
6269
await ionPageVisible(page, 'tab-lifecycle-outside');
6370

64-
await page.evaluate(() => { (window as any).lifecycleEvents = []; });
71+
await resetLifecycleEvents(page);
6572

6673
await page.locator('#go-back-to-tabs').click();
6774
await ionPageVisible(page, 'tab-lifecycle-home');
6875

69-
const events = await page.evaluate(() => (window as any).lifecycleEvents as string[]);
76+
const events = await settledLifecycleEvents(page);
7077
expect(events).toContain('home:ionViewWillEnter');
7178
expect(events).toContain('home:ionViewDidEnter');
7279
});
7380

81+
test('should fire enter and leave events when switching tabs', async ({ page }, testInfo) => {
82+
testInfo.annotations.push({
83+
type: 'issue',
84+
description: 'https://github.com/ionic-team/ionic-framework/issues/31479',
85+
});
86+
87+
await page.goto(withTestingMode('/tab-lifecycle/home'));
88+
await ionPageVisible(page, 'tab-lifecycle-home');
89+
90+
await resetLifecycleEvents(page);
91+
92+
await ionTabClick(page, 'Settings');
93+
await ionPageVisible(page, 'tab-lifecycle-settings');
94+
95+
expect(await settledLifecycleEvents(page)).toEqual([
96+
'home:ionViewWillLeave',
97+
'settings:ionViewWillEnter',
98+
'settings:ionViewDidEnter',
99+
'home:ionViewDidLeave',
100+
]);
101+
});
102+
103+
test('should fire enter and leave events when switching back to a visited tab', async ({ page }, testInfo) => {
104+
testInfo.annotations.push({
105+
type: 'issue',
106+
description: 'https://github.com/ionic-team/ionic-framework/issues/31479',
107+
});
108+
109+
await page.goto(withTestingMode('/tab-lifecycle/home'));
110+
await ionPageVisible(page, 'tab-lifecycle-home');
111+
112+
await ionTabClick(page, 'Settings');
113+
await ionPageVisible(page, 'tab-lifecycle-settings');
114+
115+
await resetLifecycleEvents(page);
116+
117+
await ionTabClick(page, 'Home');
118+
await ionPageVisible(page, 'tab-lifecycle-home');
119+
120+
expect(await settledLifecycleEvents(page)).toEqual([
121+
'settings:ionViewWillLeave',
122+
'home:ionViewWillEnter',
123+
'home:ionViewDidEnter',
124+
'settings:ionViewDidLeave',
125+
]);
126+
});
127+
74128
// A duplicate tab page, even briefly, fails this spec's page assertions on a
75129
// strict mode violation.
76130
test('should not duplicate the tab page in the DOM while returning to the tabs', async ({ page }) => {

0 commit comments

Comments
 (0)