Skip to content

dogfood: the client router prefetches the page you are already on #1106

Description

@vivek7405

Problem

The client router prefetches the URL the user is already on, so any app with a persistent nav issues two requests for one navigation instead of one.

The sequence, verified live on webjs.dev:

  1. Hover a sidebar link. Intent prefetch fires a GET /docs/client-router (correct).
  2. Click. The navigation consumes that prefetch and swaps (correct, one request so far).
  3. The swap completes, webjs:navigate fires, and the anchor now under the cursor is the link to /docs/client-router, which is the page just landed on. A fresh pointerover reaches onPrefetchIntent, nothing rejects it, and the router issues a second GET /docs/client-router.

Observed event order on a real page, captured by listening to the router's own events:

webjs:prefetch     https://webjs.dev/docs/client-router
webjs:before-cache https://webjs.dev/docs/getting-started
webjs:navigate     https://webjs.dev/docs/client-router
webjs:prefetch     https://webjs.dev/docs/client-router   <- prefetching the current page

The wasted request is never useful. A prefetch of the current URL cannot serve a future navigation to it (the router short-circuits a same-URL click), and the entry sits in prefetchCache occupying one of the capped slots until its TTL expires.

Reproduces on any nav that survives the swap and keeps the active link under the pointer. Measured (hover, dwell, click, count requests for the target URL):

Navigation Requests
docs sidebar to /docs/client-router 2
marketing header to /compare 2
marketing header /blog to /changelog 2
footer to /why-webjs 1

The footer case is 1 because the footer anchor is not re-hit-tested after the swap, so no fresh pointerover fires. That is incidental, not a design difference: the footer is one layout reflow away from behaving the same.

Found by dogfooding the docs after they moved to webjs.dev/docs (#1098). The docs sidebar makes it fire on every navigation, which is what surfaced it, but the bug predates that move and is not specific to the docs.

Design / approach

Reject a prefetch whose target resolves to the current page. A prefetch of the page you are on has no consumer by construction, so this is a pure subtraction with no behaviour to trade off.

Put the guard in prefetch(href) rather than in eligibleAnchorHref. eligibleAnchorHref is deliberately shared with the click path (see its comment about eligibility never drifting between the two), and a click on the current page's link must stay router-handled rather than falling through to a full browser navigation. prefetch() is the single funnel every mode reaches (intent, viewport, render, touchstart), so one check there covers them all.

Compare on the same normalization the cache already uses. cacheKey(href) is computed at the top of prefetch(); comparing cacheKey(href) === cacheKey(location.href) reuses whatever query/hash normalization the cache does, so the guard cannot drift from the cache's notion of "same page". A raw string compare of href against location.href would miss a trailing-slash or query-order variant that the cache treats as identical.

Note there is an existing partial version of this idea in eligibleAnchorHref: it rejects a same-page link when it carries a hash (url.pathname === location.pathname && url.search === location.search && url.hash). The hashless same-page case was simply never covered.

Implementation notes (for the implementing agent)

Where to edit:

  • packages/core/src/router-client.js, function prefetch(href) (around L1580, the guard block that already handles !enabled, prefetchSaysSaveData(), prefetchInflight.has(key), prefetchQueued.has(key), and the fresh-entry TTL check). Add the current-page rejection alongside those, after const key = cacheKey(href) so the normalized key is available.
  • Do NOT change eligibleAnchorHref (around L1490). It is shared with the click path; rejecting the current page there would stop the router handling a click on the active link and hand it to the browser as a full page load.
  • cacheKey is defined in the same file; use it for both sides of the comparison.

Landmines / gotchas:

  • The render prefetch mode scans the document on load and on every webjs:navigate (refreshPrefetchObservers, wired at L331). After a navigation that scan sees the active link, so the guard must live in prefetch() to catch that path too, not only the pointer path.
  • location.href is already updated by the time the post-navigation prefetch fires (the router pushes state before webjs:navigate), so the comparison is against the correct URL. Verify this rather than assuming it: if the guard is placed somewhere that runs before the history update, it will compare against the previous page and do nothing.
  • A prefetch of the current page is also queued through prefetchQueue when the concurrency gate is full, so the guard must run before the queueing branch or a queued self-prefetch still fires later.
  • @webjsdev/core ships a built dist/ that shadows src/ in the browser. After editing, rebuild with node scripts/build-framework-dist.js from packages/core, or the browser keeps running the old bundle and the fix looks like it did nothing.

Invariants to respect:

  • AGENTS.md code-workflow rule 1: a unit test is necessary but NOT sufficient for a client-router change. The headline behaviour needs a browser or e2e assertion.
  • The prefetch-network-budget convention: snappy is the goal, but never at the cost of over-fetching, and under-fetch when in doubt. This change is strictly in that direction.
  • Do not alter click-path eligibility. A click on the current page's link must remain router-handled.

Tests + docs surfaces:

  • Unit: packages/core/test/routing/router-client.test.js, near the existing prefetch tests. The internal _prefetchTake is already exported for tests; check whether prefetch needs a similar export or whether the behaviour is observable through the webjs:prefetch event.
  • E2E (required, per the rule above): test/e2e/e2e.test.mjs drives the blog example in a real browser and already has prefetch cases. Assert that navigating to a page and leaving the pointer on its own nav link issues exactly ONE request for that URL. Counting requests for the target URL across hover, click, and settle is the assertion that would have caught this.
  • Docs: .agents/skills/webjs/references/client-router-and-streaming.md documents the prefetch strategies; add the current-page exclusion to that description if it enumerates the rejection rules.

Acceptance criteria

  • Hovering and clicking a nav link issues exactly one request for the target URL, with the pointer left resting on the now-active link
  • The render and viewport prefetch modes also skip the current page, not only the intent path
  • A queued prefetch (concurrency gate full) for the current page is never issued
  • Clicking the current page's own link is still handled by the router, not a full browser navigation
  • An e2e assertion counts requests across a real navigation, not just a unit test on the helper
  • A counterfactual proves the new test fails when the guard is removed

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

Status
Todo

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions