From d8c56a25d1a3b22c6dbc652fc70695f5003cc3b5 Mon Sep 17 00:00:00 2001 From: ShaneK Date: Mon, 28 Sep 2026 09:59:57 -0700 Subject: [PATCH] fix(react-router): dispatch view lifecycle events on non-animated transitions --- .../src/ReactRouter/StackManager.tsx | 55 +++++++++++---- .../direction-none-back/DirectionNoneBack.tsx | 15 ++++ .../src/pages/tab-lifecycle/TabLifecycle.tsx | 22 +++--- .../react-router/test/base/src/utils/index.ts | 1 + .../test/base/src/utils/lifecycleEvents.ts | 5 ++ .../direction-none-lifecycle.spec.ts | 44 ++++++++++++ .../e2e/playwright/tab-lifecycle.spec.ts | 68 +++++++++++++++++-- .../tests/e2e/playwright/utils/test-utils.ts | 35 ++++++++++ 8 files changed, 211 insertions(+), 34 deletions(-) create mode 100644 packages/react-router/test/base/src/utils/lifecycleEvents.ts create mode 100644 packages/react-router/test/base/tests/e2e/playwright/direction-none-lifecycle.spec.ts diff --git a/packages/react-router/src/ReactRouter/StackManager.tsx b/packages/react-router/src/ReactRouter/StackManager.tsx index 69ca358dc84..1aad4abbd3d 100644 --- a/packages/react-router/src/ReactRouter/StackManager.tsx +++ b/packages/react-router/src/ReactRouter/StackManager.tsx @@ -88,6 +88,15 @@ const revealIonPageForSwipeBack = (element: HTMLElement | undefined): void => { } }; +type ViewLifecycleEvent = 'ionViewWillEnter' | 'ionViewDidEnter' | 'ionViewWillLeave' | 'ionViewDidLeave'; + +/** Dispatches a view lifecycle event the way core's `lifecycle()` does. */ +const dispatchLifecycleEvent = (element: HTMLElement | undefined, eventName: ViewLifecycleEvent): void => { + if (element) { + element.dispatchEvent(new CustomEvent(eventName, { bubbles: false, cancelable: false })); + } +}; + /** * A leaf view is "preservable" on browser-back (pop) when its React state * should survive a forward-pop round-trip. Non-parameterized leaf paths @@ -357,12 +366,8 @@ export class StackManager extends React.PureComponent { const allViewsInOutlet = this.context.getViewItemsForOutlet(this.id); allViewsInOutlet.forEach((viewItem) => { if (viewItem.ionPageElement && isViewVisible(viewItem.ionPageElement)) { - viewItem.ionPageElement.dispatchEvent( - new CustomEvent('ionViewWillLeave', { bubbles: false, cancelable: false }) - ); - viewItem.ionPageElement.dispatchEvent( - new CustomEvent('ionViewDidLeave', { bubbles: false, cancelable: false }) - ); + dispatchLifecycleEvent(viewItem.ionPageElement, 'ionViewWillLeave'); + dispatchLifecycleEvent(viewItem.ionPageElement, 'ionViewDidLeave'); } }); @@ -399,12 +404,8 @@ export class StackManager extends React.PureComponent { return; } if (viewItem.ionPageElement && isViewVisible(viewItem.ionPageElement)) { - viewItem.ionPageElement.dispatchEvent( - new CustomEvent('ionViewWillLeave', { bubbles: false, cancelable: false }) - ); - viewItem.ionPageElement.dispatchEvent( - new CustomEvent('ionViewDidLeave', { bubbles: false, cancelable: false }) - ); + dispatchLifecycleEvent(viewItem.ionPageElement, 'ionViewWillLeave'); + dispatchLifecycleEvent(viewItem.ionPageElement, 'ionViewDidLeave'); } this.context.unMountViewItem(viewItem); }); @@ -1737,10 +1738,36 @@ export class StackManager extends React.PureComponent { // Bail out if the component unmounted during waitForComponentsReady if (!this._isMounted) return; + const isCurrent = myGeneration === this.transitionGeneration; + // A page the newest transition is entering is not leaving after all. + const isLeaving = isCurrent || leavingEl !== this.transitionEnteringElement; + // Already hidden means the leave events have fired. This checks the class rather + // than `isViewVisible` because a nested outlet marks its leaving page + // `visibility: hidden` before we get here and still needs `ionViewDidLeave`. + const announceLeaving = isLeaving && !leavingEl.classList.contains('ion-page-hidden'); + + /** + * Dispatch the lifecycle events, since we skipped `commit()`. Only the + * newest transition fires the entering events, and the class swap follows + * all four so the ordering matches core's `transition()`. + * + * These run after `waitForComponentsReady` because on a first mount the + * page has not attached its listeners yet. + */ + if (announceLeaving) { + dispatchLifecycleEvent(leavingEl, 'ionViewWillLeave'); + } + if (isCurrent) { + dispatchLifecycleEvent(enteringEl, 'ionViewWillEnter'); + dispatchLifecycleEvent(enteringEl, 'ionViewDidEnter'); + } + if (announceLeaving) { + dispatchLifecycleEvent(leavingEl, 'ionViewDidLeave'); + } + // Swap visibility synchronously - show entering, hide leaving - // Skip hiding if a newer transition already made leavingEl the entering view enteringEl.classList.remove('ion-page-invisible'); - if (myGeneration === this.transitionGeneration || leavingEl !== this.transitionEnteringElement) { + if (isLeaving) { leavingEl.classList.add('ion-page-hidden'); leavingEl.setAttribute('aria-hidden', 'true'); } diff --git a/packages/react-router/test/base/src/pages/direction-none-back/DirectionNoneBack.tsx b/packages/react-router/test/base/src/pages/direction-none-back/DirectionNoneBack.tsx index 078ef707d19..4d29266cc27 100644 --- a/packages/react-router/test/base/src/pages/direction-none-back/DirectionNoneBack.tsx +++ b/packages/react-router/test/base/src/pages/direction-none-back/DirectionNoneBack.tsx @@ -8,11 +8,16 @@ import { IonRouterOutlet, IonBackButton, IonButtons, + useIonViewDidEnter, + useIonViewDidLeave, + useIonViewWillEnter, + useIonViewWillLeave, } from '@ionic/react'; import React from 'react'; import { Route, Navigate } from 'react-router-dom'; import TestDescription from '../../components/TestDescription'; +import { pushLifecycleEvent } from '../../utils'; /** * Tests that IonBackButton works correctly after navigating with @@ -20,6 +25,11 @@ import TestDescription from '../../components/TestDescription'; * determine the previous page, not fall back to defaultHref. */ const PageA: React.FC = () => { + useIonViewWillEnter(() => pushLifecycleEvent('a:ionViewWillEnter')); + useIonViewDidEnter(() => pushLifecycleEvent('a:ionViewDidEnter')); + useIonViewWillLeave(() => pushLifecycleEvent('a:ionViewWillLeave')); + useIonViewDidLeave(() => pushLifecycleEvent('a:ionViewDidLeave')); + return ( @@ -41,6 +51,11 @@ const PageA: React.FC = () => { }; const PageB: React.FC = () => { + useIonViewWillEnter(() => pushLifecycleEvent('b:ionViewWillEnter')); + useIonViewDidEnter(() => pushLifecycleEvent('b:ionViewDidEnter')); + useIonViewWillLeave(() => pushLifecycleEvent('b:ionViewWillLeave')); + useIonViewDidLeave(() => pushLifecycleEvent('b:ionViewDidLeave')); + return ( diff --git a/packages/react-router/test/base/src/pages/tab-lifecycle/TabLifecycle.tsx b/packages/react-router/test/base/src/pages/tab-lifecycle/TabLifecycle.tsx index 00f176838d5..e64543783b4 100644 --- a/packages/react-router/test/base/src/pages/tab-lifecycle/TabLifecycle.tsx +++ b/packages/react-router/test/base/src/pages/tab-lifecycle/TabLifecycle.tsx @@ -21,11 +21,7 @@ import React from 'react'; import { Route, Navigate } from 'react-router'; import TestDescription from '../../components/TestDescription'; - -const pushEvent = (event: string) => { - (window as any).lifecycleEvents = (window as any).lifecycleEvents || []; - (window as any).lifecycleEvents.push(event); -}; +import { pushLifecycleEvent } from '../../utils'; const TabLifecycle: React.FC = () => { return ( @@ -50,10 +46,10 @@ const TabLifecycle: React.FC = () => { }; const HomeTab: React.FC = () => { - useIonViewWillEnter(() => pushEvent('home:ionViewWillEnter')); - useIonViewDidEnter(() => pushEvent('home:ionViewDidEnter')); - useIonViewWillLeave(() => pushEvent('home:ionViewWillLeave')); - useIonViewDidLeave(() => pushEvent('home:ionViewDidLeave')); + useIonViewWillEnter(() => pushLifecycleEvent('home:ionViewWillEnter')); + useIonViewDidEnter(() => pushLifecycleEvent('home:ionViewDidEnter')); + useIonViewWillLeave(() => pushLifecycleEvent('home:ionViewWillLeave')); + useIonViewDidLeave(() => pushLifecycleEvent('home:ionViewDidLeave')); return ( @@ -73,10 +69,10 @@ const HomeTab: React.FC = () => { }; const SettingsTab: React.FC = () => { - useIonViewWillEnter(() => pushEvent('settings:ionViewWillEnter')); - useIonViewDidEnter(() => pushEvent('settings:ionViewDidEnter')); - useIonViewWillLeave(() => pushEvent('settings:ionViewWillLeave')); - useIonViewDidLeave(() => pushEvent('settings:ionViewDidLeave')); + useIonViewWillEnter(() => pushLifecycleEvent('settings:ionViewWillEnter')); + useIonViewDidEnter(() => pushLifecycleEvent('settings:ionViewDidEnter')); + useIonViewWillLeave(() => pushLifecycleEvent('settings:ionViewWillLeave')); + useIonViewDidLeave(() => pushLifecycleEvent('settings:ionViewDidLeave')); return ( diff --git a/packages/react-router/test/base/src/utils/index.ts b/packages/react-router/test/base/src/utils/index.ts index 5c9612f4969..45db8939265 100644 --- a/packages/react-router/test/base/src/utils/index.ts +++ b/packages/react-router/test/base/src/utils/index.ts @@ -1,2 +1,3 @@ export * from './generateId'; +export * from './lifecycleEvents'; export * from './dev'; diff --git a/packages/react-router/test/base/src/utils/lifecycleEvents.ts b/packages/react-router/test/base/src/utils/lifecycleEvents.ts new file mode 100644 index 00000000000..416d3691989 --- /dev/null +++ b/packages/react-router/test/base/src/utils/lifecycleEvents.ts @@ -0,0 +1,5 @@ +/** Records a view lifecycle event on `window.lifecycleEvents` for a spec to assert on. */ +export const pushLifecycleEvent = (event: string) => { + (window as any).lifecycleEvents = (window as any).lifecycleEvents || []; + (window as any).lifecycleEvents.push(event); +}; diff --git a/packages/react-router/test/base/tests/e2e/playwright/direction-none-lifecycle.spec.ts b/packages/react-router/test/base/tests/e2e/playwright/direction-none-lifecycle.spec.ts new file mode 100644 index 00000000000..7beacdcaf8b --- /dev/null +++ b/packages/react-router/test/base/tests/e2e/playwright/direction-none-lifecycle.spec.ts @@ -0,0 +1,44 @@ +import { test, expect, type Page } from '@playwright/test'; +import { ionPageVisible, resetLifecycleEvents, settledLifecycleEvents, withTestingMode } from './utils/test-utils'; + +/** + * A navigation with routerDirection="none" is not animated, but it must still + * fire the four view lifecycle events, in the same order an animated one does. + */ +test.describe('routerDirection="none" lifecycle events', () => { + const expectedEvents = ['a:ionViewWillLeave', 'b:ionViewWillEnter', 'b:ionViewDidEnter', 'a:ionViewDidLeave']; + + const goToPageA = async (page: Page) => { + await page.goto(withTestingMode('/direction-none-back/a')); + await ionPageVisible(page, 'direction-none-page-a'); + await resetLifecycleEvents(page); + }; + + test('should fire enter and leave events on a routerDirection="none" navigation', async ({ page }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/31479', + }); + + await goToPageA(page); + + await page.locator('#go-none').click(); + await ionPageVisible(page, 'direction-none-page-b'); + + expect(await settledLifecycleEvents(page)).toEqual(expectedEvents); + }); + + /** + * The control. A forward navigation keeps its direction, so it takes the + * regular transition path, and its event order is the one the test above + * has to match. + */ + test('should fire the same events for a forward navigation', async ({ page }) => { + await goToPageA(page); + + await page.locator('#go-forward').click(); + await ionPageVisible(page, 'direction-none-page-b'); + + expect(await settledLifecycleEvents(page)).toEqual(expectedEvents); + }); +}); diff --git a/packages/react-router/test/base/tests/e2e/playwright/tab-lifecycle.spec.ts b/packages/react-router/test/base/tests/e2e/playwright/tab-lifecycle.spec.ts index 7287d4c75df..3e479c9cb05 100644 --- a/packages/react-router/test/base/tests/e2e/playwright/tab-lifecycle.spec.ts +++ b/packages/react-router/test/base/tests/e2e/playwright/tab-lifecycle.spec.ts @@ -1,5 +1,12 @@ import { test, expect } from '@playwright/test'; -import { ionPageVisible, ionTabClick, trackPeakMatchCount, withTestingMode } from './utils/test-utils'; +import { + ionPageVisible, + ionTabClick, + resetLifecycleEvents, + settledLifecycleEvents, + trackPeakMatchCount, + withTestingMode, +} from './utils/test-utils'; test.describe('Tab Lifecycle Events', () => { test.beforeEach(async ({ page }) => { @@ -17,12 +24,12 @@ test.describe('Tab Lifecycle Events', () => { await page.goto(withTestingMode('/tab-lifecycle/home')); await ionPageVisible(page, 'tab-lifecycle-home'); - await page.evaluate(() => { (window as any).lifecycleEvents = []; }); + await resetLifecycleEvents(page); await page.locator('#go-outside').click(); await ionPageVisible(page, 'tab-lifecycle-outside'); - const events = await page.evaluate(() => (window as any).lifecycleEvents as string[]); + const events = await settledLifecycleEvents(page); expect(events).toContain('home:ionViewWillLeave'); expect(events).toContain('home:ionViewDidLeave'); }); @@ -39,12 +46,12 @@ test.describe('Tab Lifecycle Events', () => { await ionTabClick(page, 'Settings'); await ionPageVisible(page, 'tab-lifecycle-settings'); - await page.evaluate(() => { (window as any).lifecycleEvents = []; }); + await resetLifecycleEvents(page); await page.locator('#go-outside-settings').click(); await ionPageVisible(page, 'tab-lifecycle-outside'); - const events = await page.evaluate(() => (window as any).lifecycleEvents as string[]); + const events = await settledLifecycleEvents(page); expect(events).toContain('settings:ionViewWillLeave'); expect(events).toContain('settings:ionViewDidLeave'); }); @@ -61,16 +68,63 @@ test.describe('Tab Lifecycle Events', () => { await page.locator('#go-outside').click(); await ionPageVisible(page, 'tab-lifecycle-outside'); - await page.evaluate(() => { (window as any).lifecycleEvents = []; }); + await resetLifecycleEvents(page); await page.locator('#go-back-to-tabs').click(); await ionPageVisible(page, 'tab-lifecycle-home'); - const events = await page.evaluate(() => (window as any).lifecycleEvents as string[]); + const events = await settledLifecycleEvents(page); expect(events).toContain('home:ionViewWillEnter'); expect(events).toContain('home:ionViewDidEnter'); }); + test('should fire enter and leave events when switching tabs', async ({ page }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/31479', + }); + + await page.goto(withTestingMode('/tab-lifecycle/home')); + await ionPageVisible(page, 'tab-lifecycle-home'); + + await resetLifecycleEvents(page); + + await ionTabClick(page, 'Settings'); + await ionPageVisible(page, 'tab-lifecycle-settings'); + + expect(await settledLifecycleEvents(page)).toEqual([ + 'home:ionViewWillLeave', + 'settings:ionViewWillEnter', + 'settings:ionViewDidEnter', + 'home:ionViewDidLeave', + ]); + }); + + test('should fire enter and leave events when switching back to a visited tab', async ({ page }, testInfo) => { + testInfo.annotations.push({ + type: 'issue', + description: 'https://github.com/ionic-team/ionic-framework/issues/31479', + }); + + await page.goto(withTestingMode('/tab-lifecycle/home')); + await ionPageVisible(page, 'tab-lifecycle-home'); + + await ionTabClick(page, 'Settings'); + await ionPageVisible(page, 'tab-lifecycle-settings'); + + await resetLifecycleEvents(page); + + await ionTabClick(page, 'Home'); + await ionPageVisible(page, 'tab-lifecycle-home'); + + expect(await settledLifecycleEvents(page)).toEqual([ + 'settings:ionViewWillLeave', + 'home:ionViewWillEnter', + 'home:ionViewDidEnter', + 'settings:ionViewDidLeave', + ]); + }); + // A duplicate tab page, even briefly, fails this spec's page assertions on a // strict mode violation. test('should not duplicate the tab page in the DOM while returning to the tabs', async ({ page }) => { diff --git a/packages/react-router/test/base/tests/e2e/playwright/utils/test-utils.ts b/packages/react-router/test/base/tests/e2e/playwright/utils/test-utils.ts index e651cab081c..7efd5018827 100644 --- a/packages/react-router/test/base/tests/e2e/playwright/utils/test-utils.ts +++ b/packages/react-router/test/base/tests/e2e/playwright/utils/test-utils.ts @@ -17,6 +17,41 @@ export function withTestingMode(path: string): string { return `${path}${separator}ionic:_testing=true`; } +/** Clear the recorded events so an assertion only sees the navigation under test. */ +export async function resetLifecycleEvents(page: Page): Promise { + await page.evaluate(() => { + (window as any).lifecycleEvents = []; + }); +} + +/** + * Read `window.lifecycleEvents` once two consecutive reads match, so an event + * arriving just behind the expected ones is included rather than missed. + * + * An empty array never settles, so only use this where events are expected. + */ +export async function settledLifecycleEvents(page: Page): Promise { + let previous: string[] | undefined; + let current: string[] = []; + + try { + await expect + .poll(async () => { + current = await page.evaluate(() => ((window as any).lifecycleEvents ?? []) as string[]); + const settled = current.length > 0 && previous !== undefined && current.join('|') === previous.join('|'); + previous = current; + return settled; + }) + .toBe(true); + } catch (error) { + // The poll only yields a boolean, so report what was actually seen. A failing + // `page.evaluate` ends up here too, so keep the original as the cause. + throw new Error(`Lifecycle events never settled. Last read: ${JSON.stringify(current)}`, { cause: error }); + } + + return current; +} + let peakCounterId = 0; /**