Skip to content

Commit 40f9215

Browse files
feat(composables): useSearchParams mutators take { replace } (#1825)
Every mutator committed through pushState, so the composable could express "remove this param from the URL" but not "remove this param so it's gone" — which is the reason the API gets reached for. Consuming a one-shot value (an OAuth callback result, ?checkout=success, a flash token) leaves the pre-delete URL, still carrying the param, as the previous history entry: Back replays the callback and re-runs whatever consuming it triggered. set, delete and setAll now take an optional { replace?: boolean } that switches the commit to replaceState. Default is unchanged, so nothing that works today changes. Spelled to match navigate(), which already takes { replace } — before this the two halves of the routing API disagreed on whether replacing was expressible at all. All three surfaces move together, because they have drifted before: the client runtime (signals.ts), the module export (composables/use-router.ts, including its own non-delegating implementation), and the ambient declaration (stx.d.ts). The guard is `!!(options && typeof options === 'object' && options.replace)` in both implementations, deliberately, and NOT a plain truthy read of the property. A truthy read asks whether the value HAS a replace property, and every string carries String.prototype.replace — delete(key, 'push') would have replaced, the exact opposite of what it reads as. Optional chaining alone was worse: it diverged BETWEEN the two implementations on the empty string, which short-circuits in one and reaches String.prototype.replace in the other. Same call, two entry points, opposite history semantics. Verified in Chrome against the rebuilt dist, arriving at /login?code=abc123 and consuming the param: default Back -> ?code=abc123 (replayed) { replace: true } Back -> (none) (not replayed) 'push' Back -> ?code=abc123 (pushes, as it reads) true Back -> ?code=abc123 (ignored; the contract is an options object) Tests extend the existing parity suite, which already runs every case against all three implementations: the fake history gained replaceState so a test can tell "the URL changed" from "the URL changed AND the old one is still reachable with Back", plus a truth table over the odd argument shapes above. 67 pass; the replace-specific ones fail without this change. Docs updated where they now teach the wrong thing: the agent guide's canonical RIGHT example was the exact #1825 scenario and prescribed setAll({ project: undefined }), which pushes AND writes the string "undefined" rather than deleting; the PROHIBITED_DOM_PATTERNS row mapping history.replaceState to useSearchParams silently converted a replace into a push; API.md, api/router.md and api/composables-ref.md documented only get/set/setAll with "(pushes history)" and no opt-out, and referred to a type name, SearchParamsHandle, that exists nowhere in the code. Full suite 10754 pass, 1 fail — pre-existing (crosswind-arbitrary-values), confirmed identical before the change. A downstream app is green.
1 parent 68304ba commit 40f9215

9 files changed

Lines changed: 252 additions & 37 deletions

File tree

docs/API.md

Lines changed: 29 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -5367,15 +5367,43 @@ const search = useSearchParams()
53675367
53685368
// Read a param
53695369
const page = search.get('page') // '2'
5370+
search.has('page') // true
53705371
5371-
// Set a single param (pushes to history)
5372+
// Set a single param (pushes a history entry)
53725373
search.set('page', '3')
53735374
53745375
// Set multiple params at once
53755376
search.setAll({ page: '1', sort: 'name', order: 'asc' })
5377+
5378+
// Remove one
5379+
search.delete('page')
53765380
</script>
53775381
```
53785382
5383+
#### Consuming a one-shot param
5384+
5385+
Every mutator takes an optional `{ replace: true }` that swaps the `pushState`
5386+
for a `replaceState`. Reach for it whenever the param is a **one-shot value** —
5387+
an OAuth callback result, `?checkout=success`, a flash token:
5388+
5389+
```html
5390+
<script client>
5391+
const search = useSearchParams()
5392+
5393+
onMount(() => {
5394+
const code = search.get('code')
5395+
if (!code) return
5396+
exchangeForSession(code)
5397+
// Without { replace: true } the URL that still carries `code` stays in
5398+
// history, so pressing Back replays the callback and re-runs the exchange.
5399+
search.delete('code', { replace: true })
5400+
})
5401+
</script>
5402+
```
5403+
5404+
`{ replace }` is spelled the same way [`navigate()`](#navigateurl) spells it, and
5405+
the default is unchanged: a mutation pushes unless you say otherwise.
5406+
53795407
---
53805408
53815409
## Event Listener Composable

docs/api/composables-ref.md

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -289,7 +289,7 @@ const route = useRoute()
289289
### useSearchParams
290290

291291
```typescript
292-
useSearchParams(): SearchParamsHandle
292+
useSearchParams(): SearchParamsRef
293293
```
294294

295295
Reactive URL search parameters. Syncs with `popstate` and `stx:navigate` events.
@@ -307,14 +307,20 @@ search.setAll({ page: '1', sort: 'name' })
307307
</script>
308308
```
309309

310-
**SearchParamsHandle:**
310+
**SearchParamsRef:**
311311

312312
| Property/Method | Type | Description |
313313
|-----------------|------|-------------|
314314
| `data` | `Signal<Record<string, string>>` | Reactive params signal |
315315
| `get(key)` | `(key: string) => string \| undefined` | Read a param |
316-
| `set(key, value)` | `(key: string, value: string) => void` | Set a param (pushes history) |
317-
| `setAll(obj)` | `(obj: Record<string, string>) => void` | Set multiple params |
316+
| `has(key)` | `(key: string) => boolean` | Whether the param is present |
317+
| `set(key, value, options?)` | `(key, value, options?: { replace?: boolean }) => void` | Set a param. Pushes a history entry unless `replace` |
318+
| `delete(key, options?)` | `(key, options?: { replace?: boolean }) => void` | Remove a param. Pushes unless `replace` |
319+
| `setAll(obj, options?)` | `(obj, options?: { replace?: boolean }) => void` | Set several params. Pushes unless `replace` |
320+
321+
Consuming a **one-shot** param — an OAuth callback result, `?checkout=success`,
322+
a flash token — needs `{ replace: true }`. Under the default push, the URL still
323+
carrying the param stays in history, so Back replays the callback.
318324

319325
## DOM
320326

docs/api/router.md

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -446,14 +446,26 @@ route.params // {} (from stxRouter if available)
446446

447447
### useSearchParams()
448448

449-
Reactive URL search parameters with get/set methods. Automatically syncs with `popstate` and `stx:navigate` events.
449+
Reactive URL search parameters. Automatically syncs with `popstate` and `stx:navigate` events.
450450

451451
```ts
452452
const search = useSearchParams()
453453

454454
search.get('page') // '2'
455-
search.set('page', '3') // Updates URL and pushes history
455+
search.has('page') // true
456+
search.set('page', '3') // Updates the URL, pushes a history entry
456457
search.setAll({ page: '1', sort: 'name' }) // Set multiple params
458+
search.delete('page') // Remove one
459+
```
460+
461+
Every mutator takes an optional `{ replace: true }`, which replaces the current
462+
history entry instead of pushing a new one:
463+
464+
```ts
465+
// Consuming a one-shot param — an OAuth callback, ?checkout=success, a flash
466+
// token. Without `replace` the URL still carrying `code` stays in history, so
467+
// Back replays the callback.
468+
search.delete('code', { replace: true })
457469
```
458470

459471
## Next Steps

docs/guide/agent-guide.md

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3941,7 +3941,7 @@ Today that exits 1 and prints `violating blocks: 20`. Gotcha: the line numbers i
39413941
| `el.setAttribute(k, v)` | `:k="v"` | `signals.js:1784-1800` | `index.stx:137,144`, `register.stx:188` |
39423942
| `addEventListener` / `on*=` attr | `@click` / `@change` / `@input`, or `useEventListener()` | `signals.js:1830-1840`, `signals.js:3645-3652` | `dashboard.stx:429`, `StartMenu.stx:52` |
39433943
| `window.location.*`, `location.assign/replace/href=` | `navigate(url)` / `navigate(url, true)` | `signals.js:941-952` | 29 sites across 8 files |
3944-
| `window.history.replaceState` | `useSearchParams().set/setAll` | `signals.js:1004-1016` | `dashboard.stx:593` |
3944+
| `window.history.replaceState` | `useSearchParams().set/delete/setAll` **with `{ replace: true }`** — the default pushes | `signals.js:1004-1016` | `dashboard.stx:593` |
39453945
| `setTimeout` / `setInterval` | `useTimeout()` / `useInterval()` | `signals.js:3271-3300`, `3229-3268` | `settings.stx:203,406,512`, `issue/[id].stx:354`, `AutofixPanel.stx:189` |
39463946
| self-rescheduling poll | `useQuery(url, { refetchInterval })` or `useInterval` | `signals.js:1110-1113` | `AutofixPanel.stx:189` |
39473947
| `document.cookie = …` | `useCookie(name, opts)` | `signals.js:3568-3599` | 21 sites across 8 files |
@@ -4198,17 +4198,25 @@ if (qs.get('project')) {
41984198
}
41994199
```
42004200
4201-
**RIGHT (c):** `useSearchParams()` owns history rewriting (`signals.js:1004-1016`) and resyncs its signal after every write:
4201+
**RIGHT (c):** `useSearchParams()` owns history rewriting and resyncs its signal after every write:
42024202
42034203
```js
42044204
const params = useSearchParams()
42054205
const activeProject = useCookie('bughq_project', { maxAge: 31536000, sameSite: 'Lax' })
42064206
if (params.get('project')) {
42074207
activeProject.set(params.get('project'))
4208-
params.setAll({ project: undefined }) // rewrites the URL, then syncFromUrl()
4208+
// { replace: true } is load-bearing, not tidiness. This is a ONE-SHOT param:
4209+
// it has been consumed into the cookie, so it must not survive a Back press.
4210+
// The default is pushState, which leaves the URL still carrying ?project= as
4211+
// the previous history entry — Back would replay the consumption (#1825).
4212+
params.delete('project', { replace: true })
42094213
}
42104214
```
42114215
4216+
The mirror of the WRONG snippet above is exact: that code reached for
4217+
`history.replaceState` precisely because replace is the correct semantic here,
4218+
and `{ replace: true }` is how the composable expresses it.
4219+
42124220
**External URLs are not an exception.** `resources/views/pricing.stx:26` does `location.assign(data.url)` to reach Stripe. Write `navigate(data.url, true)` — same document load, one declared API, one fewer strict violation.
42134221
42144222
**CHECK:** `grep -rnE 'window\.location|window\.history|location\.(href\s*=|assign|replace)' resources/ --include='*.stx'` → must match only the sites declared under rule 8.11. Today: 29 `window.location`, 11 `location.replace`, 7 `location.assign`, 1 `location.href=`, 1 `window.history`.
@@ -4619,7 +4627,7 @@ grep -rn '<StxLink[^>]*\bprefetch\b' resources/ # must return nothing
46194627
46204628
### 9.7 MUST — programmatic navigation is `navigate()`, never `location.*` or `history.*`
46214629
4622-
**RULE.** In any `<script>` block, use the auto-imported `navigate(url)`. Use `navigate(url, true)` when you genuinely need a document load. Use `goBack()` / `goForward()` for history. Use `useSearchParams().set()` to rewrite the query string. Never write `window.location`, `location.href =`, `location.assign()`, `location.replace()` or `history.replaceState()`.
4630+
**RULE.** In any `<script>` block, use the auto-imported `navigate(url)`. Use `navigate(url, true)` when you genuinely need a document load. Use `goBack()` / `goForward()` for history. Use `useSearchParams().set()` to rewrite the query string, and pass `{ replace: true }` when the URL change must not become a Back destination — consuming a one-shot param such as an OAuth `?code=` needs it, or Back replays the callback (#1825). Never write `window.location`, `location.href =`, `location.assign()`, `location.replace()` or `history.replaceState()`.
46234631
46244632
**WHY.** `navigate` is defined at `signals.js:940-951` and published as a global at `signals.js:4635` (also `window.stx.navigate`, `signals.js:4148`). It delegates to `window.stxRouter.navigate` when the router is live (`signals.js:946-947`) and falls back to `location.href` only when it is not — strictly safer than the hand-written form. `goBack`/`goForward`: `signals.js:953-954`. `useSearchParams()` owns history rewriting: `signals.js:1004-1024`.
46254633

packages/stx/src/composables/index.ts

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -375,7 +375,9 @@ export {
375375
goForward,
376376
useRoute,
377377
useSearchParams,
378+
type NavigateOptions,
378379
type RouteInfo,
380+
type SearchParamsCommitOptions,
379381
type SearchParamsRef,
380382
} from './use-router'
381383

packages/stx/src/composables/use-router.ts

Lines changed: 47 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -21,19 +21,43 @@ export interface RouteInfo {
2121
params: Record<string, string>
2222
}
2323

24+
/**
25+
* How a search-param mutation is written to session history.
26+
*
27+
* Spelled `{ replace }` to match `navigate()`, which already takes it — before
28+
* this the two halves of the routing API disagreed on whether replacing was
29+
* expressible at all.
30+
*/
31+
export interface SearchParamsCommitOptions {
32+
/** Replace the current history entry instead of pushing a new one. */
33+
replace?: boolean
34+
}
35+
2436
export interface SearchParamsRef {
2537
/** Reactive signal containing current search params */
2638
data: { (): Record<string, string>, set: (v: Record<string, string>) => void }
2739
/** Get a single param value. `undefined` when absent. */
2840
get: (key: string) => string | undefined
2941
/** Whether the param is present. */
3042
has: (key: string) => boolean
31-
/** Set a single param and push to history */
32-
set: (key: string, value: string) => void
33-
/** Remove a param and push to history */
34-
delete: (key: string) => void
35-
/** Set multiple params and push to history */
36-
setAll: (obj: Record<string, string>) => void
43+
/**
44+
* Set a single param. Pushes a history entry unless `replace` is passed.
45+
*
46+
* Pass `{ replace: true }` when the URL change should not be a place the user
47+
* can go Back to — see `delete` for the case that makes this necessary.
48+
*/
49+
set: (key: string, value: string, options?: SearchParamsCommitOptions) => void
50+
/**
51+
* Remove a param. Pushes a history entry unless `replace` is passed.
52+
*
53+
* CONSUMING a one-shot param — an OAuth callback result, `?checkout=success`,
54+
* a flash token — needs `{ replace: true }`. With the default push, the URL
55+
* that still carries the param becomes the previous history entry, so Back
56+
* replays the callback and re-runs whatever consuming it triggered (#1825).
57+
*/
58+
delete: (key: string, options?: SearchParamsCommitOptions) => void
59+
/** Set multiple params. Pushes a history entry unless `replace` is passed. */
60+
setAll: (obj: Record<string, string>, options?: SearchParamsCommitOptions) => void
3761
}
3862

3963
export interface NavigateOptions {
@@ -178,8 +202,17 @@ export function useSearchParams(): SearchParamsRef {
178202
window.addEventListener('popstate', sync)
179203
window.addEventListener('stx:navigate', sync)
180204

181-
const commit = (url: URL) => {
182-
window.history.pushState({}, '', url)
205+
const commit = (url: URL, options?: SearchParamsCommitOptions) => {
206+
// Deliberately the same test as the client runtime's commit
207+
// (signals.ts) — `options?.replace` alone diverges from it on '',
208+
// because optional chaining only short-circuits on null/undefined and
209+
// String.prototype.replace is truthy. Same call, two entry points, opposite
210+
// history semantics is exactly what this composable must not have.
211+
const replace = !!(options && typeof options === 'object' && options.replace)
212+
if (replace)
213+
window.history.replaceState({}, '', url)
214+
else
215+
window.history.pushState({}, '', url)
183216
sync()
184217
}
185218
// Own-property reads only: the backing object would otherwise inherit
@@ -190,20 +223,20 @@ export function useSearchParams(): SearchParamsRef {
190223
data: data as any,
191224
get: key => (has(key) ? data()[key] : undefined),
192225
has,
193-
set: (key, value) => {
226+
set: (key, value, options) => {
194227
const url = new URL(window.location.href)
195228
url.searchParams.set(key, value)
196-
commit(url)
229+
commit(url, options)
197230
},
198-
delete: (key) => {
231+
delete: (key, options) => {
199232
const url = new URL(window.location.href)
200233
url.searchParams.delete(key)
201-
commit(url)
234+
commit(url, options)
202235
},
203-
setAll: (obj) => {
236+
setAll: (obj, options) => {
204237
const url = new URL(window.location.href)
205238
for (const [k, v] of Object.entries(obj)) url.searchParams.set(k, v)
206-
commit(url)
239+
commit(url, options)
207240
},
208241
}
209242
}

packages/stx/src/signals.ts

Lines changed: 35 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -1125,31 +1125,58 @@ else if (immediate) {
11251125
var current = params();
11261126
return Object.prototype.hasOwnProperty.call(current, key);
11271127
};
1128-
var commit = function(url) {
1129-
window.history.pushState({}, '', url);
1128+
// pushState by default; replaceState when the caller asks for it (#1825).
1129+
//
1130+
// The canonical reason to delete a param is to CONSUME a one-shot value —
1131+
// an OAuth callback result, ?checkout=success, a flash token — and the
1132+
// point of consuming it is that it must not survive a Back press. Under
1133+
// pushState the pre-delete URL, the one still carrying the param, becomes
1134+
// the previous entry, so Back replays the callback and whatever consuming
1135+
// it triggered runs again.
1136+
//
1137+
// Default stays pushState so nothing that works today changes, and the
1138+
// option is spelled { replace } to match navigate(), which already takes it.
1139+
var commit = function(url, options) {
1140+
// A plain truthy read of the replace property looks equivalent and is
1141+
// not: it asks whether the value HAS a replace property, and every string
1142+
// has String.prototype.replace — so delete(key, 'push') would have
1143+
// replaced, the exact opposite of what it reads as. Requiring an object
1144+
// also keeps this identical to the module implementation in
1145+
// composables/use-router.ts; with optional chaining alone the two
1146+
// disagreed on the empty string, which short-circuits in one and hits
1147+
// String.prototype.replace in the other.
1148+
//
1149+
// NB: no backticks anywhere in this block. This runtime is assembled as a
1150+
// template literal, so a backtick in a COMMENT ends the string and the
1151+
// rest of the runtime is parsed as code.
1152+
var replace = !!(options && typeof options === 'object' && options.replace);
1153+
if (replace)
1154+
window.history.replaceState({}, '', url);
1155+
else
1156+
window.history.pushState({}, '', url);
11301157
syncFromUrl();
11311158
};
11321159
return {
11331160
data: params,
11341161
get: function(key) { return own(key) ? params()[key] : undefined; },
11351162
has: own,
1136-
set: function(key, value) {
1163+
set: function(key, value, options) {
11371164
var url = new URL(window.location.href);
11381165
url.searchParams.set(key, value);
1139-
commit(url);
1166+
commit(url, options);
11401167
},
11411168
// delete and has were DECLARED but absent, so the compiler endorsed calls
11421169
// that threw in the browser; setAll and data were present but undeclared
11431170
// (#1806). Deleting writes history the same way set does, for consistency.
1144-
'delete': function(key) {
1171+
'delete': function(key, options) {
11451172
var url = new URL(window.location.href);
11461173
url.searchParams['delete'](key);
1147-
commit(url);
1174+
commit(url, options);
11481175
},
1149-
setAll: function(obj) {
1176+
setAll: function(obj, options) {
11501177
var url = new URL(window.location.href);
11511178
Object.keys(obj).forEach(function(k) { url.searchParams.set(k, obj[k]); });
1152-
commit(url);
1179+
commit(url, options);
11531180
}
11541181
};
11551182
}

packages/stx/stx.d.ts

Lines changed: 19 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -134,6 +134,12 @@ declare function useRoute(): {
134134
hash: string
135135
}
136136
declare function setRouteParams(_params: Record<string, string>): void
137+
/** How a search-param mutation is written to session history (#1825). */
138+
interface StxSearchParamsCommitOptions {
139+
/** Replace the current history entry instead of pushing a new one. */
140+
replace?: boolean
141+
}
142+
137143
/**
138144
* Reactive access to the URL query string.
139145
*
@@ -148,9 +154,19 @@ declare function useSearchParams(): {
148154
data: StxSignal<Record<string, string>>
149155
get: (_key: string) => string | undefined
150156
has: (_key: string) => boolean
151-
set: (_key: string, _value: string) => void
152-
delete: (_key: string) => void
153-
setAll: (_values: Record<string, string>) => void
157+
/** Set a param. Pushes a history entry unless `{ replace: true }` is passed. */
158+
set: (_key: string, _value: string, _options?: StxSearchParamsCommitOptions) => void
159+
/**
160+
* Remove a param. Pushes a history entry unless `{ replace: true }` is passed.
161+
*
162+
* CONSUMING a one-shot param — an OAuth callback result, `?checkout=success`,
163+
* a flash token — needs `{ replace: true }`. Under the default push, the URL
164+
* that still carries the param becomes the previous history entry, so Back
165+
* replays the callback and re-runs whatever consuming it triggered (#1825).
166+
*/
167+
delete: (_key: string, _options?: StxSearchParamsCommitOptions) => void
168+
/** Set several params. Pushes a history entry unless `{ replace: true }` is passed. */
169+
setAll: (_values: Record<string, string>, _options?: StxSearchParamsCommitOptions) => void
154170
}
155171

156172
// ============================================================================

0 commit comments

Comments
 (0)