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

Fields

No fields configured for issues without a type.

Projects

Status
Todo

Milestone

No milestone

Relationships

None yet

Development

No branches or pull requests

Issue actions