fix(router): skip non-HTML responses and extensions; document data-no-router

vivek7405 · vivek7405 · commit 9af54bf6a9aa · 2026-04-18T17:13:29.000+05:30
Three gaps in the same-origin interception logic that produced a
broken/hung/blank page:

1. Non-HTML extensions — /foo.pdf, /data.zip, /feed.xml, /posts.json,
   /avatar.png, /clip.mp4. The router would fetch-and-swap binary or
   non-HTML bytes as if they were a page. Now onClick() short-circuits
   when the pathname matches a conservative NON_HTML_EXTENSIONS regex
   covering documents, archives, feeds, images, media, fonts.

2. JSON / SSE / PDF served to same-origin paths without a telltale
   extension — /api/posts, /events, /docs/report. After the fetch, the
   router now rejects any response whose Content-Type isn't text/html
   and falls back to location.href. SSE responses (text/event-stream)
   no longer hang on resp.text().

3. Auth flows (/logout, /auth/google) — documented data-no-router as
   the explicit escape hatch for semantics (module-level state must be
   wiped by a full reload, which SPA nav leaves behind). AGENTS.md
   Client Router section + docs/client-router now call this out.

New internal export `_isNonHtmlPath(pathname)` so the extension list
can be unit-tested. Eight regression tests added to
test/router-client.test.js covering the extension predicate and
Content-Type guard (JSON, SSE, PDF, missing CT, and a negative
control for text/html).

Tests: 246 → 255 unit, 21 browser.
diff --git a/AGENTS.md b/AGENTS.md
@@ -1469,9 +1469,21 @@ await navigate('/login', { replace: true }); // replace history entry
 <a href="/legacy" data-no-router>Full reload</a>
 ```
 
+**Use `data-no-router` for:**
+- **Auth flows** — `/logout`, `/auth/google`, OAuth redirect chains. A full reload wipes in-memory module state (cached user data, auth tokens), which SPA navigation leaves behind. Module state surviving a "logout" is a real bug class.
+- **Print views / embed pages** — anywhere you want a clean-slate render without the existing layout.
+- **Experimental routes** backed by a different client runtime that needs a full boot.
+
+**What the router auto-skips** (no `data-no-router` needed):
+- Links with `download`, `target` other than `_self`, or a modifier-key click.
+- Cross-origin hrefs.
+- Pure hash fragments on the same page.
+- Hrefs ending in non-HTML extensions (`.pdf`, `.zip`, `.json`, `.xml`, images, media, archives, documents) — the browser handles them natively.
+- Responses whose `Content-Type` isn't `text/html` — falls back to a full navigation so JSON APIs, SSE streams, feeds, and mis-served downloads behave correctly.
+
 **Loading indicator:** `<html>` gets `data-navigating` attribute during fetch — use CSS to show a progress bar.
 
-**When to use:** always — it's enabled by default in the scaffold layout. **When NOT to use:** links with `download`, `target="_blank"`, external origins — these are automatically skipped.
+**When to use:** always — it's enabled by default in the scaffold layout.
 
 ### Raw-text templates
 
diff --git a/docs/app/docs/client-router/page.ts b/docs/app/docs/client-router/page.ts
@@ -10,13 +10,25 @@ export default function ClientRouter() {
     <h2>When to use</h2>
     <p>It's enabled automatically when you import <code>webjs/client-router</code> in your layout (the scaffold does this for you). You don't need to do anything — every <code>&lt;a&gt;</code> link in your app becomes a client-side navigation.</p>
 
-    <h2>When NOT to use</h2>
+    <h2>When the router auto-skips a link</h2>
+    <p>These are handled natively by the browser — no fetch, no swap:</p>
     <ul>
-      <li>For links to external sites — they're ignored automatically (different origin).</li>
-      <li>For file downloads — links with <code>download</code> attribute are ignored.</li>
-      <li>For links that should force a full reload — add <code>data-no-router</code> to the <code>&lt;a&gt;</code> tag.</li>
+      <li>Cross-origin hrefs.</li>
+      <li>Links with a <code>download</code> attribute, a <code>target</code> other than <code>_self</code>, or clicked with a modifier key (⌘/Ctrl/Shift/Alt).</li>
+      <li>Pure hash fragments on the same page (lets the browser jump to the anchor).</li>
+      <li>Hrefs whose path ends in a non-HTML extension: <code>.pdf</code>, <code>.zip</code>, <code>.json</code>, <code>.xml</code>, images, media, archives, documents. The browser opens them in a viewer, triggers a download, or renders the feed directly.</li>
+      <li>Responses whose <code>Content-Type</code> isn't <code>text/html</code> (JSON APIs, SSE streams, mis-served downloads). The router notices after the fetch and falls back to a full navigation.</li>
     </ul>
 
+    <h2>Explicit opt-out with <code>data-no-router</code></h2>
+    <p>Add <code>data-no-router</code> to force a full page navigation for links the router would otherwise intercept. Use this for:</p>
+    <ul>
+      <li><strong>Auth flows</strong> — <code>/logout</code>, <code>/auth/google</code>, OAuth redirect chains. A full reload wipes in-memory module state (cached user data, auth tokens); SPA navigation leaves it behind.</li>
+      <li><strong>Print views / embed pages</strong> — anywhere you want a clean-slate render without the existing layout.</li>
+      <li><strong>Experimental routes</strong> backed by a different client runtime that needs a full boot.</li>
+    </ul>
+    <pre>&lt;a href="/logout" data-no-router&gt;Log out&lt;/a&gt;</pre>
+
     <h2>How it works</h2>
     <ol>
       <li>Intercepts clicks on <code>&lt;a&gt;</code> tags (same origin, no modifier keys, no <code>target</code>, no <code>download</code>). Works across shadow DOM boundaries via <code>composedPath()</code>.</li>
@@ -41,12 +53,7 @@ await navigate('/about');
 // Replace current history entry
 await navigate('/login', { replace: true });</pre>
 
-    <h2>Opting out</h2>
-    <p>Add <code>data-no-router</code> to any link to force a full page navigation:</p>
-
-    <pre>&lt;a href="/legacy-page" data-no-router&gt;Full reload&lt;/a&gt;</pre>
-
-    <p>To disable the router entirely:</p>
+    <h2>Disabling the router entirely</h2>
     <pre>import { disableClientRouter } from 'webjs/client-router';
 disableClientRouter();</pre>
 
diff --git a/packages/core/src/router-client.js b/packages/core/src/router-client.js
@@ -112,6 +112,17 @@ enableClientRouter();
  * Internal
  * ==================================================================== */
 
+/**
+ * Pathnames with these extensions are never HTML pages. The client router
+ * should let the browser handle them natively — so PDFs open in a viewer,
+ * JSON/XML feeds render as text, archives trigger the download prompt,
+ * and images open in a new tab. Intercepting would fetch-and-swap binary
+ * bytes as HTML, producing a blank/garbled page with no way to recover.
+ *
+ * This is intentionally conservative — anything not listed still routes.
+ */
+const NON_HTML_EXTENSIONS = /\.(?:pdf|zip|tar|gz|7z|rar|dmg|exe|msi|deb|rpm|apk|ipa|xlsx?|docx?|pptx?|csv|odt|ods|odp|rtf|epub|mobi|xml|json|rss|atom|txt|md|wasm|mp3|mp4|mov|avi|webm|ogg|flac|wav|m4a|m4v|mkv|png|jpe?g|gif|webp|avif|bmp|ico|svg|tiff?|heic)$/i;
+
 /** @param {MouseEvent} e */
 function onClick(e) {
   if (!enabled) return;
@@ -134,6 +145,9 @@ function onClick(e) {
   if (url.origin !== location.origin) return;
   // Skip hash-only changes on the same page.
   if (url.pathname === location.pathname && url.search === location.search && url.hash) return;
+  // Skip paths whose extension clearly isn't an HTML page — let the
+  // browser handle downloads / feeds / images natively.
+  if (NON_HTML_EXTENSIONS.test(url.pathname)) return;
 
   e.preventDefault();
   performNavigation(href, false);
@@ -180,6 +194,19 @@ async function performNavigation(href, isPopState) {
       location.href = href;
       return;
     }
+    // Content-Type guard — if the server didn't return HTML, the router
+    // can't swap it safely. Common mismatches:
+    //   • API routes: application/json, application/vnd.api+json
+    //   • Feeds:      application/xml, text/xml, application/rss+xml
+    //   • Downloads:  application/pdf, application/zip, etc.
+    //   • SSE:        text/event-stream (would hang on resp.text())
+    // Fall back to a full navigation so the browser handles the MIME
+    // natively (viewer, download prompt, JSON tree, etc.).
+    const ctype = resp.headers.get('content-type') || '';
+    if (!/^text\/html\b/i.test(ctype)) {
+      location.href = href;
+      return;
+    }
     const html = await resp.text();
     const doc = parseHTML(html);
     if (!doc) {
@@ -472,3 +499,15 @@ export {
   addNewHeadElements as _addNewHeadElements,
   mergeHead as _mergeHead,
 };
+
+/**
+ * Predicate used by the onClick handler to decide whether a same-origin
+ * href should bypass the router (let the browser handle it natively).
+ * Exposed for unit testing.
+ *
+ * @param {string} pathname
+ * @returns {boolean}
+ */
+export function _isNonHtmlPath(pathname) {
+  return NON_HTML_EXTENSIONS.test(pathname);
+}
diff --git a/test/router-client.test.js b/test/router-client.test.js
@@ -10,7 +10,7 @@ import { test, before } from 'node:test';
 import assert from 'node:assert/strict';
 import { parseHTML } from 'linkedom';
 
-let _find, _addNewHead, _merge;
+let _find, _addNewHead, _merge, _isNonHtmlPath, navigate;
 
 before(async () => {
   const { window } = parseHTML('<!doctype html><html><head></head><body></body></html>');
@@ -31,8 +31,13 @@ before(async () => {
   globalThis.CustomEvent = window.CustomEvent;
   globalThis.DOMParser = window.DOMParser;
 
-  ({ _findLayoutShell: _find, _addNewHeadElements: _addNewHead, _mergeHead: _merge } =
-    await import('../packages/core/src/router-client.js'));
+  ({
+    _findLayoutShell: _find,
+    _addNewHeadElements: _addNewHead,
+    _mergeHead: _merge,
+    _isNonHtmlPath,
+    navigate,
+  } = await import('../packages/core/src/router-client.js'));
 });
 
 test('findLayoutShell: detects data-layout element on direct body child', () => {
@@ -177,3 +182,151 @@ test('mergeHead: re-creates script elements so they execute', () => {
   assert.notStrictEqual(added, s, 'script re-created so browser executes it');
   assert.equal(added.getAttribute('type'), 'module');
 });
+
+/* ------------ extension-based skip (pre-emptive) ------------ */
+
+test('isNonHtmlPath: skips downloads and documents', () => {
+  assert.equal(_isNonHtmlPath('/exports/report.pdf'), true);
+  assert.equal(_isNonHtmlPath('/files/archive.zip'), true);
+  assert.equal(_isNonHtmlPath('/data/records.csv'), true);
+  assert.equal(_isNonHtmlPath('/Download.DOCX'), true, 'case-insensitive');
+});
+
+test('isNonHtmlPath: skips feeds and api-like extensions', () => {
+  assert.equal(_isNonHtmlPath('/feed.xml'), true);
+  assert.equal(_isNonHtmlPath('/feed.rss'), true);
+  assert.equal(_isNonHtmlPath('/posts.json'), true);
+  assert.equal(_isNonHtmlPath('/robots.txt'), true);
+});
+
+test('isNonHtmlPath: skips images and media', () => {
+  assert.equal(_isNonHtmlPath('/avatar.png'), true);
+  assert.equal(_isNonHtmlPath('/logo.svg'), true);
+  assert.equal(_isNonHtmlPath('/hero.webp'), true);
+  assert.equal(_isNonHtmlPath('/clip.mp4'), true);
+  assert.equal(_isNonHtmlPath('/theme.mp3'), true);
+});
+
+test('isNonHtmlPath: does NOT skip normal page paths', () => {
+  assert.equal(_isNonHtmlPath('/'), false);
+  assert.equal(_isNonHtmlPath('/blog/post-slug'), false);
+  assert.equal(_isNonHtmlPath('/dashboard'), false);
+  // A route that happens to include a dot in a segment but no extension.
+  assert.equal(_isNonHtmlPath('/users/john.smith/profile'), false);
+});
+
+/* ------------ Content-Type guard on navigate() ------------ */
+
+function installNavigationMocks({ contentType, body = '', ok = true }) {
+  const originalFetch = globalThis.fetch;
+  const originalLocation = globalThis.location;
+  const originalHistory = globalThis.history;
+  const originalScrollTo = globalThis.scrollTo;
+  /** @type {{ href: string | null, assigns: string[] }} */
+  const redirect = { href: null, assigns: [] };
+
+  globalThis.fetch = async () => ({
+    ok,
+    status: ok ? 200 : 500,
+    headers: { get: (k) => (k.toLowerCase() === 'content-type' ? contentType : null) },
+    text: async () => body,
+  });
+
+  // Replace location with a spy that captures href assignments.
+  globalThis.location = /** @type any */ ({
+    origin: 'http://localhost',
+    href: 'http://localhost/',
+    get pathname() { return '/'; },
+    get search() { return ''; },
+  });
+  Object.defineProperty(globalThis.location, 'href', {
+    configurable: true,
+    get() { return 'http://localhost/'; },
+    set(v) { redirect.href = v; redirect.assigns.push(v); },
+  });
+
+  // Stubs for APIs the happy-path swap calls — without them the swap
+  // throws, the catch-all falls back to location.href, and we can't
+  // distinguish "Content-Type guard fired" from "environment is missing
+  // browser APIs".
+  globalThis.history = /** @type any */ ({ pushState: () => {}, replaceState: () => {} });
+  globalThis.scrollTo = /** @type any */ (() => {});
+
+  return {
+    redirect,
+    restore() {
+      globalThis.fetch = originalFetch;
+      globalThis.location = originalLocation;
+      globalThis.history = originalHistory;
+      globalThis.scrollTo = originalScrollTo;
+    },
+  };
+}
+
+test('navigate: JSON response triggers full-page fallback (no DOM swap)', async () => {
+  const { redirect, restore } = installNavigationMocks({
+    contentType: 'application/json; charset=utf-8',
+    body: '{"posts":[]}',
+  });
+  try {
+    await navigate('http://localhost/api/posts');
+    assert.equal(redirect.href, 'http://localhost/api/posts',
+      'JSON response should trigger location.href assignment');
+  } finally {
+    restore();
+  }
+});
+
+test('navigate: text/event-stream triggers full-page fallback', async () => {
+  const { redirect, restore } = installNavigationMocks({
+    contentType: 'text/event-stream',
+    body: '',
+  });
+  try {
+    await navigate('http://localhost/events');
+    assert.equal(redirect.href, 'http://localhost/events');
+  } finally {
+    restore();
+  }
+});
+
+test('navigate: application/pdf triggers full-page fallback', async () => {
+  const { redirect, restore } = installNavigationMocks({
+    contentType: 'application/pdf',
+    body: '%PDF-1.4\n...',
+  });
+  try {
+    await navigate('http://localhost/docs/report');
+    assert.equal(redirect.href, 'http://localhost/docs/report');
+  } finally {
+    restore();
+  }
+});
+
+test('navigate: text/html response proceeds with router swap (no fallback)', async () => {
+  const { redirect, restore } = installNavigationMocks({
+    contentType: 'text/html; charset=utf-8',
+    body: '<!doctype html><html><head><title>ok</title></head><body><div data-layout="x">content</div></body></html>',
+  });
+  try {
+    await navigate('http://localhost/ok');
+    assert.equal(redirect.href, null,
+      'text/html response should not trigger location.href fallback');
+  } finally {
+    restore();
+  }
+});
+
+test('navigate: response without content-type falls back safely', async () => {
+  const { redirect, restore } = installNavigationMocks({
+    contentType: '',
+    body: '',
+  });
+  try {
+    await navigate('http://localhost/weird');
+    assert.equal(redirect.href, 'http://localhost/weird',
+      'missing Content-Type is not assumed to be HTML');
+  } finally {
+    restore();
+  }
+});
