ReviewOS

stacks/stx

navigate(): declared second arg is { replace?: boolean }, implemented as a forceReload boolean

#1807
Closed glennmichael123 opened this 14 days ago · 1 comment
14 days ago

Verified against @stacksjs/stx at 0.2.153 (local build of main, HEAD 0dcb6fbd0e).

TL;DR

navigate()'s second parameter is declared as an options object with a replace flag. The implementation takes a boolean forceReload. The declared call compiles, and then does the opposite of what it says: a full document load that pushes a history entry, rather than a client-side replace.

Declared

packages/stx/stx.d.ts:101 (hand-written; copied verbatim to dist/stx.d.ts):

declare function navigate(_url: string, _options?: { replace?: boolean }): void

Implemented

packages/stx/src/signals.ts:1027:

function navigate(url, forceReload) {
  if (forceReload) {
    window.location.href = url;
    return;
  }
  if (window.stxRouter && typeof window.stxRouter.navigate === 'function') {
    window.stxRouter.navigate(url);
    …

There is no replace behaviour anywhere in this function.

Why it matters

navigate is in STX_AUTO_IMPORTS (src/client-script.ts:316), so this ambient declaration is the contract a <script client> author writes against.

navigate('/dashboard', { replace: true })
  • type-checks cleanly
  • { replace: true } is a truthy object, so it takes the forceReload branch
  • result: window.location.href = url — a full page load, which adds a history entry

So the one call the type system invites is the one that both defeats the SPA router and produces the opposite history behaviour from the one requested. It fails silently — no error, just a slower navigation and a wrong back-button stack.

Suggested fix

Whichever direction is intended, the two should agree. If the boolean is the API:

declare function navigate(_url: string, _forceReload?: boolean): void

If replace is the intended API, it needs implementing — window.location.replace(url) for the full-load path, and history.replaceState for the router path.

Worth noting there is a second, unrelated navigate at packages/stx/src/composables/use-router.ts:41 with the signature navigate(url: string): void. Two exported functions of the same name and different arities is its own source of confusion, though not what this issue is about.

Same class as #1707 / #1796 / #1797 and the useSearchParams mismatch I filed alongside this one — shipped declarations disagreeing with their implementations.

14 days ago

Fixed in e22f5810ca. Implemented replace rather than downgrading the declaration, as you suggested.

Worth recording why it was never implemented: replace wasn't implementable from signals.ts alone. window.stxRouter.navigate is (url, pushState, force), and its history writes happen at three sites after the fetch/swap resolves — all hardcoded to pushState. So whoever wrote the declaration described the API they wanted and the runtime silently reinterpreted the argument as a boolean, because there was no replace mode underneath to forward to.

Fixed on both sides. The router gained a writeHistory() helper with the mode travelling in the existing pushState slot (false still means "leave history alone", true pushes, 'replace' replaces) rather than threading a fourth argument through swap() to each site.

The legacy positional boolean still means "full reload" at runtime — it's what actually shipped and the docs still show it — but it stays out of the declared type, so nothing that type-checks today changes meaning.

This also fixed a second live defect the same confusion caused, which I don't think anyone had noticed: hot-reload.ts called window.navigate(url, false, true) in the router's three-argument shape. The runtime helper takes two, so force was dropped, the same-URL early return fired, and the dev server's HMR "reload" was a no-op on every page that had the router.

5 of 8 tests fail against the old implementation.

Sign in to comment on this issue.