From 18db0cab86cf15198be059c60c044b66512e573e Mon Sep 17 00:00:00 2001 From: Mathias Picker <48158184+MathiasWP@users.noreply.github.com> Date: Fri, 27 Feb 2026 20:26:53 +0100 Subject: [PATCH] fix: SvelteMap incorrectly handles keys with `undefined` values (#17826) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Summary `SvelteMap` had two bugs related to how it checked for key existence internally: ### 1. `has()` and `get()` returned wrong results for keys with `undefined` values Both methods used `super.get(key) !== undefined` to determine if a key existed before creating a per-key reactive source. This fails for keys whose value is legitimately `undefined`, causing: - `has(key)` to return `false` for existing keys with `undefined` values - `get(key)` to skip creating a per-key source and fall back to tracking `version`, resulting in over-notification **Fix:** Replace `super.get(key) !== undefined` with `super.has(key)` in both `has()` and `get()`, matching the pattern already used in `SvelteSet`. ### 2. `delete()` skipped reactive updates when a key had no per-key source The `size` and `version` reactive updates were inside the `if (s !== undefined)` block, meaning they only fired when a per-key source existed (i.e., someone had previously called `has()` or `get()` on that specific key). If a key was added via the constructor or `set()` but never individually read, deleting it would not trigger reactive updates for effects depending on `size` or iterators. **Fix:** Move `set(this.#size, super.size)` and `increment(this.#version)` to a separate `if (res)` block so they fire whenever a key is actually deleted, regardless of whether a per-key source existed. ### Before fix ```js const map = new SvelteMap([['foo', undefined]]); map.has('foo'); // false (should be true) map.get('foo'); // undefined but tracks version instead of per-key source ``` ### After fix ```js const map = new SvelteMap([['foo', undefined]]); map.has('foo'); // true map.get('foo'); // undefined with correct per-key tracking ``` ## Test plan Tests are in `packages/svelte/src/reactivity/map.test.ts`: - `map.has()` returns `true` for constructor-initialized keys with `undefined` values - `map.get()` returns `undefined` with proper per-key reactive tracking - `map.delete()` triggers `has()`/`get()` reactivity for undefined-valued keys - `map.set(key, undefined)` followed by `has()`/`get()` works correctly - `map.delete()` triggers `size` reactivity for keys that were never individually read (no per-key source) - All existing tests pass unchanged 🤖 Generated with [Claude Code](https://claude.com/claude-code) --------- Co-authored-by: Claude Opus 4.6 --- .changeset/fix-svelte-map-undefined.md | 5 ++ packages/svelte/src/reactivity/map.js | 11 ++-- packages/svelte/src/reactivity/map.test.ts | 69 ++++++++++++++++++++++ 3 files changed, 80 insertions(+), 5 deletions(-) create mode 100644 .changeset/fix-svelte-map-undefined.md diff --git a/.changeset/fix-svelte-map-undefined.md b/.changeset/fix-svelte-map-undefined.md new file mode 100644 index 0000000000..fa981c46f8 --- /dev/null +++ b/.changeset/fix-svelte-map-undefined.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: `SvelteMap` incorrectly handles keys with `undefined` values diff --git a/packages/svelte/src/reactivity/map.js b/packages/svelte/src/reactivity/map.js index 014b5e7c7c..48d06a05a7 100644 --- a/packages/svelte/src/reactivity/map.js +++ b/packages/svelte/src/reactivity/map.js @@ -98,8 +98,7 @@ export class SvelteMap extends Map { var s = sources.get(key); if (s === undefined) { - var ret = super.get(key); - if (ret !== undefined) { + if (super.has(key)) { s = this.#source(0); if (DEV) { @@ -134,8 +133,7 @@ export class SvelteMap extends Map { var s = sources.get(key); if (s === undefined) { - var ret = super.get(key); - if (ret !== undefined) { + if (super.has(key)) { s = this.#source(0); if (DEV) { @@ -202,8 +200,11 @@ export class SvelteMap extends Map { if (s !== undefined) { sources.delete(key); - set(this.#size, super.size); set(s, -1); + } + + if (res) { + set(this.#size, super.size); increment(this.#version); } diff --git a/packages/svelte/src/reactivity/map.test.ts b/packages/svelte/src/reactivity/map.test.ts index 2f9f064b42..8bb6f72f7b 100644 --- a/packages/svelte/src/reactivity/map.test.ts +++ b/packages/svelte/src/reactivity/map.test.ts @@ -207,6 +207,75 @@ test('map handling of undefined values', () => { cleanup(); }); +test('map.has() and map.get() with undefined values', () => { + const map = new SvelteMap([['foo', undefined]]); + + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push('has', map.has('foo')); + }); + + render_effect(() => { + log.push('get', map.get('foo')); + }); + + flushSync(() => { + map.delete('foo'); + }); + + flushSync(() => { + map.set('bar', undefined); + }); + }); + + assert.deepEqual(log, [ + 'has', + true, + 'get', + undefined, + 'has', + false, + 'get', + undefined, + // set('bar') bumps version, causing has('foo')/get('foo') effects to re-run + 'has', + false, + 'get', + undefined + ]); + + assert.equal(map.has('bar'), true); + assert.equal(map.get('bar'), undefined); + + cleanup(); +}); + +test('map.delete() triggers size reactivity for keys without per-key sources', () => { + const map = new SvelteMap([ + [1, 'a'], + [2, 'b'] + ]); + + const log: any = []; + + const cleanup = effect_root(() => { + render_effect(() => { + log.push(map.size); + }); + + // delete key 2 which was never individually read (no per-key source) + flushSync(() => { + map.delete(2); + }); + }); + + assert.deepEqual(log, [2, 1]); + + cleanup(); +}); + test('not invoking reactivity when value is not in the map after changes', () => { const map = new SvelteMap([[1, 1]]);