fix(router): support anchor fragments in href

Author: ShaneK

core/src/components/router/router.tsx

@@ -9,7 +9,7 @@ import type { NavigationHookResult } from '../route/route-interface';
 
 import { ROUTER_INTENT_BACK, ROUTER_INTENT_FORWARD, ROUTER_INTENT_NONE } from './utils/constants';
 import { printRedirects, printRoutes } from './utils/debug';
-import { readNavState, waitUntilNavNode, writeNavState } from './utils/dom';
+import { readNavState, scrollToFragment, waitUntilNavNode, writeNavState } from './utils/dom';
 import type { RouteChain, RouterDirection, RouterEventDetail } from './utils/interface';
 import { findChainForIDs, findChainForSegments, findRouteRedirect } from './utils/matching';
 import { readRedirects, readRoutes } from './utils/parser';
@@ -24,6 +24,7 @@ export class Router implements ComponentInterface {
   private state = 0;
   private lastState = 0;
   private waitPromise?: Promise<void>;
+  private fragmentScrollToken = 0;
 
   @Element() el!: HTMLElement;
 
@@ -67,11 +68,18 @@ export class Router implements ComponentInterface {
       if (typeof canProceed === 'object') {
         const { redirect } = canProceed;
         const path = parsePath(redirect);
-        this.setSegments(path.segments, ROUTER_INTENT_NONE, path.queryString);
-        await this.writeNavStateRoot(path.segments, ROUTER_INTENT_NONE);
+        this.setSegments(path.segments, ROUTER_INTENT_NONE, path.queryString, path.fragment);
+        const result = await this.writeNavStateRoot(path.segments, ROUTER_INTENT_NONE);
+        if (result) {
+          this.maybeScrollToFragment();
+        }
       }
-    } else {
-      await this.onRoutesChanged();
+      return;
+    }
+
+    const result = await this.onRoutesChanged();
+    if (result) {
+      this.maybeScrollToFragment();
     }
   }
 
@@ -93,7 +101,12 @@ export class Router implements ComponentInterface {
         return false;
       }
     }
-    return this.writeNavStateRoot(segments, direction);
+    const result = await this.writeNavStateRoot(segments, direction);
+    if (result) {
+      this.maybeScrollToFragment();
+    }
+
+    return result;
   }
 
   @Listen('ionBackButton', { target: 'document' })
@@ -132,7 +145,7 @@ export class Router implements ComponentInterface {
       const currentPath = this.previousPath ?? '/';
       // Convert currentPath to an URL by pre-pending a protocol and a host to resolve the relative path.
       const url = new URL(path, `https://host/${currentPath}`);
-      path = url.pathname + url.search;
+      path = url.pathname + url.search + url.hash;
     }
 
     let parsedPath = parsePath(path);
@@ -146,8 +159,13 @@ export class Router implements ComponentInterface {
       }
     }
 
-    this.setSegments(parsedPath.segments, direction, parsedPath.queryString);
-    return this.writeNavStateRoot(parsedPath.segments, direction, animation);
+    this.setSegments(parsedPath.segments, direction, parsedPath.queryString, parsedPath.fragment);
+    const result = await this.writeNavStateRoot(parsedPath.segments, direction, animation);
+    if (result) {
+      this.maybeScrollToFragment();
+    }
+
+    return result;
   }
 
   /** Go back to previous page in the window.history. */
@@ -188,7 +206,12 @@ export class Router implements ComponentInterface {
       return false;
     }
 
-    this.setSegments(segments, direction);
+    // navChanged is an outlet-driven URL sync. Only keep the fragment when
+    // the path is unchanged; on a real navigation it refers to an anchor on
+    // the page being left and would be stale.
+    const newPath = generatePath(segments);
+    const fragment = newPath === this.previousPath ? this.getFragment() : undefined;
+    this.setSegments(segments, direction, undefined, fragment);
 
     await this.safeWriteNavState(outlet, chain, ROUTER_INTENT_NONE, segments, null, ids.length);
     return true;
@@ -245,8 +268,8 @@ export class Router implements ComponentInterface {
     let redirectFrom: string[] | null = null;
 
     if (redirect) {
-      const { segments: toSegments, queryString } = redirect.to!;
-      this.setSegments(toSegments, direction, queryString);
+      const { segments: toSegments, queryString, fragment } = redirect.to!;
+      this.setSegments(toSegments, direction, queryString, fragment);
       redirectFrom = redirect.from;
       segments = toSegments;
     }
@@ -361,15 +384,49 @@ export class Router implements ComponentInterface {
     return changed;
   }
 
-  private setSegments(segments: string[], direction: RouterDirection, queryString?: string) {
+  private setSegments(segments: string[], direction: RouterDirection, queryString?: string, fragment?: string) {
     this.state++;
-    writeSegments(window.history, this.root, this.useHash, segments, direction, this.state, queryString);
+    // Every URL write invalidates any in-flight fragment scroll: a newer nav
+    // (with or without a fragment, successful or not) should always supersede.
+    this.fragmentScrollToken++;
+    writeSegments(
+      window.history,
+      this.root,
+      this.useHash,
+      segments,
+      direction,
+      this.state,
+      queryString,
+      fragment
+    );
   }
 
   private getSegments(): string[] | null {
     return readSegments(window.location, this.root, this.useHash);
   }
 
+  private getFragment(): string | undefined {
+    // In hash mode the URL fragment trails a second `#` (e.g. `#/path#anchor`);
+    // parse the routing portion to extract it.
+    const raw = this.useHash
+      ? parsePath(window.location.hash.slice(1)).fragment
+      : window.location.hash.slice(1);
+    return raw ? raw : undefined;
+  }
+
+  /**
+   * Fires a best-effort scroll to the current URL fragment. The scroll bails
+   * if a newer `setSegments` advances `fragmentScrollToken` mid-flight.
+   */
+  private maybeScrollToFragment() {
+    const fragment = this.getFragment();
+    if (!fragment) return;
+    const token = this.fragmentScrollToken;
+    // Fire-and-forget; the returned promise resolves only after the scroll
+    // animation completes, which the caller does not need to await.
+    scrollToFragment(fragment, () => token === this.fragmentScrollToken).catch(() => {});
+  }
+
   private routeChangeEvent(toSegments: string[], redirectFromSegments: string[] | null): RouterEventDetail | null {
     const from = this.previousPath;
     const to = generatePath(toSegments);

core/src/components/router/test/basic/index.html

@@ -26,7 +26,10 @@
           <p><a href='#/two/second-page'>Go to page 2</a></p>
           <p><a href='#/two/three/hola'>Go to page 3 (hola)</a></p>
           <p><a href='#/two/three/something'>Go to page 3 (something)</a></p>
-
+          <p><ion-router-link id="link-with-fragment" href="/two/second-page#anchor">Page 2 with fragment</ion-router-link></p>
+          <p><ion-router-link id="link-with-query-and-fragment" href="/two/three/hola?flag=true#anchor">Page 3 with query and fragment</ion-router-link></p>
+          <div style="height: 2000px;">page-one spacer</div>
+          <h2 id="anchor">page-one anchor (must lose to the active page's anchor)</h2>
         </ion-content>`;
         }
       }
@@ -42,6 +45,9 @@
         <ion-content>
           <p><a href='#/two/three/hola'>Go to page 3 (hola)</a></p>
           <p><a href='#/two/three/hello'>Go to page 3 (hello)</a></p>
+          <div style="height: 2000px;">spacer</div>
+          <h2 id="anchor">Anchor target</h2>
+          <div style="height: 500px;">trailing spacer</div>
         </ion-content>`;
         }
       }
@@ -60,6 +66,7 @@
           <p><a href='#/two/second-page'>Go to page 2</a></p>
           <p><a href='#/two/'>Go to page 1</a></p>
           <ion-button id="btn-rel" href="./relative?param=1">Page 3 (relative)</ion-button>
+          <ion-button id="btn-rel-with-fragment" href="./relative#anchor">Page 3 (relative with fragment)</ion-button>
           <ion-button id="btn-abs" href="/two/three/absolute">Page 3 (absolute)</ion-button>
         </ion-content>`;
         }

core/src/components/router/test/basic/router.e2e.ts

@@ -1,6 +1,21 @@
 import { expect } from '@playwright/test';
+import type { E2EPage } from '@utils/test/playwright';
 import { configs, test } from '@utils/test/playwright';
 
+/**
+ * Waits until `page-two`'s `ion-content` has scrolled past the fixture's 2000px
+ * spacer. The anchor target sits below the spacer, so a successful fragment
+ * scroll must move `scrollTop` well past it; a regression that scrolled by
+ * only a handful of pixels would fail this threshold.
+ */
+const waitForAnchorScrolled = (page: E2EPage) =>
+  page.waitForFunction(async () => {
+    const content = document.querySelector('page-two ion-content') as HTMLIonContentElement | null;
+    if (!content) return false;
+    const scrollEl = await content.getScrollElement();
+    return scrollEl.scrollTop > 1500;
+  });
+
 /**
  * This behavior does not vary across modes/directions.
  */
@@ -27,6 +42,166 @@ configs({ modes: ['ios'], directions: ['ltr'] }).forEach(({ title, config }) =>
 
       expect(page.url()).toContain('#/two/three/absolute');
     });
+
+    test('should route when ion-router-link href contains a fragment', async ({ page }, testInfo) => {
+      testInfo.annotations.push({
+        type: 'issue',
+        description: 'https://github.com/ionic-team/ionic-framework/issues/19365',
+      });
+      const errors: string[] = [];
+      page.on('pageerror', (e) => errors.push(e.message));
+      page.on('console', (msg) => {
+        if (msg.type() === 'error') errors.push(msg.text());
+      });
+
+      await page.goto(`/src/components/router/test/basic#/two`, config);
+      await page.click('#link-with-fragment');
+
+      await expect(page.locator('page-two')).toBeVisible();
+      expect(page.url()).toContain('#/two/second-page#anchor');
+      expect(errors.filter((m) => m.includes('not part of the routing set'))).toEqual([]);
+    });
+
+    test('should route when ion-router-link href contains both query and fragment', async ({ page }, testInfo) => {
+      testInfo.annotations.push({
+        type: 'issue',
+        description: 'https://github.com/ionic-team/ionic-framework/issues/19365',
+      });
+      await page.goto(`/src/components/router/test/basic#/two`, config);
+      await page.click('#link-with-query-and-fragment');
+
+      await expect(page.locator('page-three')).toBeVisible();
+      expect(page.url()).toContain('#/two/three/hola?flag=true#anchor');
+    });
+
+    test('should preserve the fragment when push() resolves a relative path', async ({ page }, testInfo) => {
+      testInfo.annotations.push({
+        type: 'issue',
+        description: 'https://github.com/ionic-team/ionic-framework/issues/19365',
+      });
+      await page.goto(`/src/components/router/test/basic#/two/three/hola`, config);
+      await page.click('#btn-rel-with-fragment');
+
+      expect(page.url()).toContain('#/two/three/relative#anchor');
+    });
+
+    test('should scroll to the fragment target after navigating', async ({ page }, testInfo) => {
+      testInfo.annotations.push({
+        type: 'issue',
+        description: 'https://github.com/ionic-team/ionic-framework/issues/19365',
+      });
+      await page.goto(`/src/components/router/test/basic#/two`, config);
+      await page.click('#link-with-fragment');
+
+      await expect(page.locator('page-two #anchor')).toBeVisible();
+      await waitForAnchorScrolled(page);
+    });
+
+    test('should scroll to the fragment target on initial deep-link load', async ({ page }, testInfo) => {
+      testInfo.annotations.push({
+        type: 'issue',
+        description: 'https://github.com/ionic-team/ionic-framework/issues/19365',
+      });
+      // Land on the fixture without a fragment first so the test helper can
+      // attach its query params (it appends them after the hash, which would
+      // otherwise pollute the fragment). Once loaded we replaceState to a URL
+      // that includes the fragment, then reload to simulate a true cold open.
+      await page.goto(`/src/components/router/test/basic#/two`, config);
+      await page.evaluate(() => {
+        const { origin, pathname, search } = window.location;
+        window.history.replaceState({}, '', `${origin}${pathname}${search}#/two/second-page#anchor`);
+      });
+      await page.reload();
+
+      await expect(page.locator('page-two #anchor')).toBeVisible();
+      await waitForAnchorScrolled(page);
+    });
+
+    test('should scope the fragment lookup to the active page', async ({ page }, testInfo) => {
+      testInfo.annotations.push({
+        type: 'issue',
+        description: 'https://github.com/ionic-team/ionic-framework/issues/19365',
+      });
+      // page-one and page-two both expose `id="anchor"`. page-one is kept in
+      // the DOM as `.ion-page-hidden` after the push; a document-wide
+      // `getElementById` would return its anchor first. The router must scope
+      // the lookup to the active page so page-two's anchor wins.
+      await page.goto(`/src/components/router/test/basic#/two`, config);
+      await page.click('#link-with-fragment');
+
+      await expect(page.locator('page-two #anchor')).toBeVisible();
+      await waitForAnchorScrolled(page);
+
+      // page-one is still in the DOM but should not have been scrolled.
+      const pageOneScrollTop = await page.evaluate(async () => {
+        const content = document.querySelector('page-one ion-content') as HTMLIonContentElement | null;
+        if (!content) return 0;
+        const scrollEl = await content.getScrollElement();
+        return scrollEl.scrollTop;
+      });
+      expect(pageOneScrollTop).toBeLessThan(100);
+    });
+
+    test('should drop a stale fragment when navChanged fires for a different path', async ({ page }, testInfo) => {
+      testInfo.annotations.push({
+        type: 'issue',
+        description: 'https://github.com/ionic-team/ionic-framework/issues/19365',
+      });
+      // Land on a URL with a fragment, then trigger a tab switch. The tab
+      // outlet emits `navChanged` for the new path; the fragment referred to
+      // an anchor on the previous page and must not survive the rewrite.
+      await page.goto(`/src/components/router/test/basic#/two/second-page#anchor`, config);
+      await expect(page.locator('page-two')).toBeVisible();
+
+      await page.click('#tab-button-tab-one');
+
+      await expect(page.locator('tab-one')).toBeVisible();
+      await page.waitForFunction(() => !window.location.hash.includes('#anchor'));
+      expect(page.url()).not.toContain('#anchor');
+    });
+
+    test('should cancel an in-flight fragment scroll when a newer navigation supersedes it', async ({
+      page,
+    }, testInfo) => {
+      testInfo.annotations.push({
+        type: 'issue',
+        description: 'https://github.com/ionic-team/ionic-framework/issues/19365',
+      });
+      // Two rapid pushes: the first targets a fragment (begins polling +
+      // smooth scroll), the second arrives before the first lands and clears
+      // the fragment. The cancellation token must abort the first scroll so
+      // we end up at the top of the page, not parked at #anchor.
+      await page.goto(`/src/components/router/test/basic#/two`, config);
+      await expect(page.locator('page-one')).toBeVisible();
+
+      await page.evaluate(async () => {
+        const router = document.querySelector('ion-router') as HTMLIonRouterElement;
+        router.push('/two/second-page#anchor');
+        await router.push('/two/second-page');
+      });
+
+      await expect(page.locator('page-two')).toBeVisible();
+      // Wait for page-two's scrollTop to stabilise across two consecutive
+      // frames. A scroll triggered by the un-cancelled first push would
+      // still be animating when the assertion runs.
+      await page.waitForFunction(async () => {
+        const content = document.querySelector('page-two ion-content') as HTMLIonContentElement | null;
+        if (!content) return false;
+        const scrollEl = await content.getScrollElement();
+        const first = scrollEl.scrollTop;
+        await new Promise((resolve) => requestAnimationFrame(() => requestAnimationFrame(resolve)));
+        return scrollEl.scrollTop === first;
+      });
+
+      const pageTwoScrollTop = await page.evaluate(async () => {
+        const content = document.querySelector('page-two ion-content') as HTMLIonContentElement | null;
+        if (!content) return -1;
+        const scrollEl = await content.getScrollElement();
+        return scrollEl.scrollTop;
+      });
+      expect(pageTwoScrollTop).toBeLessThan(100);
+      expect(page.url()).not.toContain('#anchor');
+    });
   });
 
   test.describe(title('router: tabs'), () => {