fix(react-router): skip page clone when nothing will animate

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

@@ -5,7 +5,7 @@
  */
 
 import type { RouteInfo, StackContextState, ViewItem } from '@ionic/react';
-import { IonRoute, RouteManagerContext, StackContext, createDebugLogger, generateId } from '@ionic/react';
+import { IonRoute, RouteManagerContext, StackContext, createDebugLogger, generateId, getConfig } from '@ionic/react';
 import React from 'react';
 import type { RouteObject } from 'react-router-dom';
 import { Route, UNSAFE_RouteContext as RouteContext, matchRoutes } from 'react-router-dom';
@@ -1513,8 +1513,37 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
   ) {
     const myGeneration = ++this.transitionGeneration;
 
+    const routerOutlet = this.routerOutletElement!;
+
+    const routeInfoFallbackDirection =
+      routeInfo.routeDirection === 'none' || routeInfo.routeDirection === 'root' ? undefined : routeInfo.routeDirection;
+    const directionToUse = direction ?? routeInfoFallbackDirection;
+
+    /**
+     * The cases where we pass `commit()` a duration of 0. It's a function so each
+     * caller reads `skipTransition` as of when it runs, since the swipe gesture can
+     * set it after we get here.
+     */
+    const isInstantCommit = () => this.skipTransition || skipAnimation || directionToUse === undefined;
+
+    /**
+     * Whether `commit()` will run an animation. Mirrors the check in core's
+     * `router-outlet.tsx`, so keep the two in sync: an instant commit never
+     * animates, and otherwise the outlet's `animated` prop and the global
+     * `animated` config both have to allow it (`ionic:_testing` turns it off).
+     */
+    const willCommitAnimate = () => {
+      if (isInstantCommit()) {
+        return false;
+      }
+
+      const config = getConfig();
+      return !!routerOutlet.animated && (config ? config.getBoolean('animated', true) : true);
+    };
+
     const runCommit = async (enteringEl: HTMLElement, leavingEl?: HTMLElement) => {
       const skipTransition = this.skipTransition;
+      const commitDuration = isInstantCommit() ? 0 : undefined;
 
       /**
        * If the transition was handled
@@ -1551,8 +1580,6 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
         }
       }
 
-      const commitDuration = skipTransition || skipAnimation || directionToUse === undefined ? 0 : undefined;
-
       // Race commit against a timeout to recover from hangs
       const commitPromise = routerOutlet.commit(enteringEl, leavingEl, {
         duration: commitDuration,
@@ -1589,27 +1616,25 @@ export class StackManager extends React.PureComponent<StackManagerProps> {
       }
     };
 
-    const routerOutlet = this.routerOutletElement!;
-
-    const routeInfoFallbackDirection =
-      routeInfo.routeDirection === 'none' || routeInfo.routeDirection === 'root' ? undefined : routeInfo.routeDirection;
-    const directionToUse = direction ?? routeInfoFallbackDirection;
-
     if (enteringViewItem && enteringViewItem.ionPageElement && this.routerOutletElement) {
       this.transitionEnteringElement = enteringViewItem.ionPageElement;
 
       if (leavingViewItem && leavingViewItem.ionPageElement && enteringViewItem === leavingViewItem) {
         // Clone page for same-view transitions (e.g., /user/1 → /user/2)
         const match = matchComponent(leavingViewItem.reactElement, routeInfo.pathname, undefined, this.outletMountPath);
-        if (match) {
+        if (match && willCommitAnimate()) {
           const newLeavingElement = clonePageElement(leavingViewItem.ionPageElement.outerHTML);
           if (newLeavingElement) {
             this.routerOutletElement.appendChild(newLeavingElement);
             await runCommit(enteringViewItem.ionPageElement, newLeavingElement);
             this.routerOutletElement.removeChild(newLeavingElement);
           }
         } else {
-          // Route no longer matches (e.g., /user/1 → /settings)
+          /**
+           * Either the route no longer matches (e.g., /user/1 → /settings), or
+           * nothing will animate, so the clone would duplicate the page in the
+           * DOM for no benefit.
+           */
           await runCommit(enteringViewItem.ionPageElement, undefined);
         }
       } else {

packages/react-router/test/base/tests/e2e/playwright/nested-params.spec.ts

@@ -1,5 +1,5 @@
 import { test, expect } from '@playwright/test';
-import { ionPageVisible, withTestingMode } from './utils/test-utils';
+import { ionPageVisible, trackPeakMatchCount, withTestingMode } from './utils/test-utils';
 
 test.describe('Nested Params', () => {
 
@@ -44,4 +44,23 @@ test.describe('Nested Params', () => {
     await expect(page.getByText('Layout sees user: 42')).toBeVisible();
   });
 
+  // A duplicate details page, even briefly, fails this spec's text assertions on
+  // a strict mode violation.
+  test('should not duplicate the details page while switching params', async ({ page }) => {
+    await page.goto(withTestingMode('/nested-params'));
+    await ionPageVisible(page, 'nested-params-landing');
+
+    await page.locator('#go-to-user-99').click();
+    await expect(page.getByText('Details view user: 99')).toBeVisible();
+
+    await page.locator('[data-pageid="nested-params-user-99"]:not(.ion-page-hidden) #back-to-landing').click();
+    await ionPageVisible(page, 'nested-params-landing');
+
+    const peakDetailsPages = await trackPeakMatchCount(page, '[data-testid="user-details-param"]', '42');
+
+    await page.locator('#go-to-user-42').click();
+    await expect(page.getByText('Details view user: 42')).toBeVisible();
+
+    expect(await peakDetailsPages()).toBe(1);
+  });
 });

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

@@ -1,5 +1,5 @@
 import { test, expect } from '@playwright/test';
-import { ionPageVisible, ionTabClick, withTestingMode } from './utils/test-utils';
+import { ionPageVisible, ionTabClick, trackPeakMatchCount, withTestingMode } from './utils/test-utils';
 
 test.describe('Tab Lifecycle Events', () => {
   test.beforeEach(async ({ page }) => {
@@ -70,4 +70,21 @@ test.describe('Tab Lifecycle Events', () => {
     expect(events).toContain('home:ionViewWillEnter');
     expect(events).toContain('home:ionViewDidEnter');
   });
+
+  // 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 }) => {
+    await page.goto(withTestingMode('/tab-lifecycle/home'));
+    await ionPageVisible(page, 'tab-lifecycle-home');
+
+    await page.locator('#go-outside').click();
+    await ionPageVisible(page, 'tab-lifecycle-outside');
+
+    const peakHomePages = await trackPeakMatchCount(page, 'div.ion-page[data-pageid="tab-lifecycle-home"]');
+
+    await page.locator('#go-back-to-tabs').click();
+    await ionPageVisible(page, 'tab-lifecycle-home');
+
+    expect(await peakHomePages()).toBe(1);
+  });
 });

packages/react-router/test/base/tests/e2e/playwright/utils/test-utils.ts

@@ -17,6 +17,52 @@ export function withTestingMode(path: string): string {
   return `${path}${separator}ionic:_testing=true`;
 }
 
+let peakCounterId = 0;
+
+/**
+ * Start recording the largest number of elements matching `selector` (optionally
+ * narrowed to those containing `containsText`) that ever coexist. The returned
+ * function stops recording and resolves with the peak.
+ *
+ * Narrowing by text matters because several parameterized layouts can be in the
+ * DOM at once, so it's what limits the count to the page under test.
+ *
+ * Start tracking after the last navigation. The counter lives on `window`, so a
+ * `page.goto()` in between wipes it and the returned function will throw.
+ */
+export async function trackPeakMatchCount(
+  page: Page,
+  selector: string,
+  containsText?: string
+): Promise<() => Promise<number>> {
+  const key = `__peakMatchCount${peakCounterId++}`;
+
+  await page.evaluate(
+    ({ selector, containsText, key }) => {
+      const state = window as any;
+      const count = () => {
+        const matches = Array.from(document.querySelectorAll(selector));
+        return containsText ? matches.filter((el) => el.textContent?.includes(containsText)).length : matches.length;
+      };
+
+      state[key] = count();
+      const observer = new MutationObserver(() => {
+        state[key] = Math.max(state[key], count());
+      });
+      observer.observe(document.body, { subtree: true, childList: true, characterData: true });
+      state[`${key}Stop`] = () => observer.disconnect();
+    },
+    { selector, containsText, key }
+  );
+
+  return () =>
+    page.evaluate((k) => {
+      const state = window as any;
+      state[`${k}Stop`]();
+      return state[k] as number;
+    }, key);
+}
+
 /**
  * Assert that a page is visible and not hidden or invisible.
  * Equivalent to Cypress `cy.ionPageVisible(pageId)`.