Skip to content

dogfood: test client-router query-param preservation on every nav path #639

Description

@vivek7405

Problem

While testing the blog, query params sometimes appeared NOT to carry forward to the navigated page. The client router needs explicit, locked test coverage for query-string handling across every navigation path, so a genuine drop is caught and the spec-correct cases are documented.

Investigation of packages/core/src/router-client.js shows MOST paths are correct:

  • finalUrl defaults to the full link href (which includes the query string) and drives pushState (lines ~1626, ~1692, ~1701), so a link with a query string keeps it in the URL.
  • The snapshot and prefetch caches key on pathname + search (cacheKey line ~817; prefetch key line ~316), so ?x=1 and ?x=2 are distinct entries.

One path deliberately drops pre-existing params, and it is spec-compliant, not a bug: the GET-form submission resets url.search = '' and rebuilds it from the form body (line ~957), matching the native HTML5 GET-form algorithm (the form fields replace the action's query). The tests must assert this so it is not later "fixed" into a regression.

So this is primarily a test-coverage task that will ALSO surface any genuine drop (a real bug to fix).

Design / approach

Add a browser test that exercises every nav path and asserts location.search (and the pushState URL) after a real soft swap. Model it on the existing packages/core/test/routing/browser/*.test.js files, which mock window.fetch and call navigate() / drive clicks.

Implementation notes (for the implementing agent)

  • Where: new packages/core/test/routing/browser/query-params.test.js. Import { enableClientRouter, navigate } from ../../../src/router-client.js; mock window.fetch to return a <!--wj:children--> body (see nav-scroll-instant.test.js for the exact harness shape).
  • Key code paths in packages/core/src/router-client.js: click handler href = anchor.href (~L350), finalUrl (~L1626/1692), pushState(finalUrl) (~L1701), cacheKey = pathname + search (~L815-817), prefetch key (~L316), GET-form url.search = '' (~L957).
  • Landmines: (a) the GET-form replacement is correct, do NOT call it a bug; (b) a server redirect() legitimately changes finalUrl to the redirect target (params may differ by design); (c) linkedom (the unit DOM) does not do real navigation, so the headline assertions belong in the browser layer, not a unit test.
  • Invariants: no .ts in packages/ (plain JS + JSDoc).
  • Tests + docs: browser test is the headline; if a genuine drop is found and fixed, note the corrected behavior in agent-docs/advanced.md (client router section).

Acceptance criteria

  • A link with a query string preserves location.search after the soft swap AND in the pushState URL
  • Navigating /a?x=1 then /b?y=2 lands with search === '?y=2' (not merged, not dropped)
  • A prefetched URL with a query string is cache-keyed by search and served for the matching params only
  • popstate/back restores the query string of the prior history entry
  • A GET-form submission replaces the query with the form fields (spec-correct; asserted so it is not mistaken for a bug)
  • A server redirect updates finalUrl to the redirect target's search
  • A counterfactual proves each assertion fails if the relevant line is reverted
  • Any genuine drop found by (1)-(4) or (6) is fixed in router-client.js; agent-docs/advanced.md updated if behavior changed

Metadata

Metadata

Assignees

Labels

bugSomething isn't working

Type

No type

Projects

Status
Done

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions