From 0510174bc0c908c9d4980274717191fdf46a38c6 Mon Sep 17 00:00:00 2001 From: Nic Polumeyv Date: Mon, 15 Jun 2026 12:08:45 -0400 Subject: [PATCH] fix: make `SvelteURLSearchParams` notifications accurate (#18425) `SvelteURLSearchParams` has two notification bugs that surface through `SvelteURL`. The first is over-notification. Setting `url.href` always rebuilds the params and bumps their version signal, even when the search string didn't change at all. Effects that read `size` (or any params method) re-run on every unrelated `href` write, which is #17218, where it caused infinite effect loops in a router. The fix is a guard in the internal replace path that bails out when the incoming params serialize identically to the current ones. The second is under-notification, and it's sneakier. Every read method tracks the version signal, `get`, `getAll`, `has`, `keys`, `values`, `entries`, `toString`, `size`, the iterator... except `forEach`, which was never overridden. The inherited platform method reads the internal list directly and subscribes to nothing, so a template that renders params via `forEach` never updates: ```js $effect(() => { url.searchParams.forEach((value, key) => entries.push(`${key}=${value}`)); // never re-runs }); ``` We found this one in the wild while making `page.url` a `SvelteURL` in sveltejs/kit#16031. Kit's own test suite renders query params with `forEach`, and the coarse object-identity signal that previously masked the gap went away, leaving stale UI after client-side navigations. The fix is the same one-line tracking pattern every other read method already uses. Fixes #17218 --- - [x] It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs - [x] Prefix your PR title with `feat:`, `fix:`, `chore:`, or `docs:`. - [x] This message body should clearly illustrate what problems it solves. - [x] Ideally, include a test that fails without this PR but passes with it. - [x] If this PR changes code within `packages/svelte/src`, add a changeset (`npx changeset`). ### Tests and linting - [x] Run the tests with `pnpm test` and lint the project with `pnpm lint` --- .changeset/quick-meals-shout.md | 5 ++ .../src/reactivity/url-search-params.js | 15 ++++ .../src/reactivity/url-search-params.test.ts | 21 ++++++ packages/svelte/src/reactivity/url.test.ts | 74 +++++++++++++++++++ 4 files changed, 115 insertions(+) create mode 100644 .changeset/quick-meals-shout.md diff --git a/.changeset/quick-meals-shout.md b/.changeset/quick-meals-shout.md new file mode 100644 index 0000000000..21871edd67 --- /dev/null +++ b/.changeset/quick-meals-shout.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: don't notify `searchParams` subscribers when the URL changes without affecting the search string diff --git a/packages/svelte/src/reactivity/url-search-params.js b/packages/svelte/src/reactivity/url-search-params.js index 38cf3ebe4f..d745bb6b08 100644 --- a/packages/svelte/src/reactivity/url-search-params.js +++ b/packages/svelte/src/reactivity/url-search-params.js @@ -54,6 +54,11 @@ export class SvelteURLSearchParams extends URLSearchParams { */ [REPLACE](params) { if (this.#updating) return; + + // the URL may have changed in a way that leaves the search string untouched — + // don't rebuild the params or notify readers if nothing changed + if (params.toString() === super.toString()) return; + this.#updating = true; for (const key of [...super.keys()]) { @@ -126,6 +131,16 @@ export class SvelteURLSearchParams extends URLSearchParams { return super.keys(); } + /** + * @param {(value: string, key: string, parent: URLSearchParams) => void} callback + * @param {any} [this_arg] + * @returns {void} + */ + forEach(callback, this_arg) { + get(this.#version); + super.forEach(callback, this_arg); + } + /** * @param {string} name * @param {string} value diff --git a/packages/svelte/src/reactivity/url-search-params.test.ts b/packages/svelte/src/reactivity/url-search-params.test.ts index b0c84872b0..765b4020f3 100644 --- a/packages/svelte/src/reactivity/url-search-params.test.ts +++ b/packages/svelte/src/reactivity/url-search-params.test.ts @@ -244,3 +244,24 @@ test('URLSearchParams.toString', () => { test('SvelteURLSearchParams instanceof URLSearchParams', () => { assert.ok(new SvelteURLSearchParams() instanceof URLSearchParams); }); + +test('params.forEach is reactive', () => { + const params = new SvelteURLSearchParams('foo=1'); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + const entries: string[] = []; + params.forEach((value, key) => entries.push(`${key}=${value}`)); + log.push(entries.join('&')); + }); + }); + + flushSync(() => { + params.set('foo', '2'); + }); + + assert.deepEqual(log, ['foo=1', 'foo=2']); + + cleanup(); +}); diff --git a/packages/svelte/src/reactivity/url.test.ts b/packages/svelte/src/reactivity/url.test.ts index a79aa32315..d698116421 100644 --- a/packages/svelte/src/reactivity/url.test.ts +++ b/packages/svelte/src/reactivity/url.test.ts @@ -166,3 +166,77 @@ test('url.search normalizes value', () => { test('SvelteURL instanceof URL', () => { assert.ok(new SvelteURL('https://svelte.dev') instanceof URL); }); + +test('url.searchParams subscribers are not notified by changes that leave the search string untouched', () => { + const url = new SvelteURL('https://svelte.dev/a?foo=bar'); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(url.searchParams.toString()); + }); + }); + + flushSync(() => { + // does not affect the search string + url.pathname = '/b'; + }); + + flushSync(() => { + // neither does this + url.href = 'https://svelte.dev/c?foo=bar#hash'; + }); + + flushSync(() => { + // but this does + url.search = '?foo=baz'; + }); + + assert.deepEqual(log, ['foo=bar', 'foo=baz']); + + cleanup(); +}); + +test('url.searchParams.size is not notified by unrelated href changes', () => { + const url = new SvelteURL('https://svelte.dev/?foo=bar'); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(url.searchParams.size); + }); + }); + + flushSync(() => { + url.href = 'https://svelte.dev/other?foo=bar'; + }); + + flushSync(() => { + url.searchParams.append('baz', 'qux'); + }); + + assert.deepEqual(log, [1, 2]); + + cleanup(); +}); + +test('url.searchParams.forEach re-runs when the search string changes via the URL', () => { + const url = new SvelteURL('https://svelte.dev/?foo=1'); + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + const entries: string[] = []; + url.searchParams.forEach((value, key) => entries.push(`${key}=${value}`)); + log.push(entries.join('&')); + }); + }); + + flushSync(() => { + url.href = 'https://svelte.dev/?bar=2'; + }); + + assert.deepEqual(log, ['foo=1', 'bar=2']); + + cleanup(); +});