Skip to content

fix: SvelteMap incorrectly handles keys with undefined values#17826

Merged
Rich-Harris merged 5 commits intosveltejs:mainfrom
MathiasWP:fix/svelte-map-undefined-values
Feb 27, 2026
Merged

fix: SvelteMap incorrectly handles keys with undefined values#17826
Rich-Harris merged 5 commits intosveltejs:mainfrom
MathiasWP:fix/svelte-map-undefined-values

Conversation

@MathiasWP
Copy link
Copy Markdown
Contributor

@MathiasWP MathiasWP commented Feb 27, 2026

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

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

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

`SvelteMap.has()` and `SvelteMap.get()` used `super.get(key) !== undefined`
to check if a key exists, which returns a false negative when the value is
legitimately `undefined`. Replaced with `super.has(key)`.

Also fixed `SvelteMap.delete()` to trigger reactive updates (size + version)
whenever a key is actually deleted, not only when a per-key source existed.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@changeset-bot
Copy link
Copy Markdown

changeset-bot bot commented Feb 27, 2026

🦋 Changeset detected

Latest commit: 9f7abd4

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package
Name Type
svelte Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
@7nik
Copy link
Copy Markdown
Contributor

7nik commented Feb 27, 2026

The test should be in packages/svelte/src/reactivity/map.test.ts

MathiasWP and others added 3 commits February 27, 2026 17:11
The set() optimization to skip reactivity for same-value constructor-
initialized keys isn't safe — re-setting a mutated object with the same
reference should still trigger updates. Reverted to always trigger when
no source exists. Also cleaned up test comments.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Move SvelteMap undefined values test from tests/signals/test.ts to
src/reactivity/map.test.ts per PR feedback. Add test for delete()
triggering size reactivity on keys without per-key sources.

Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>
Copy link
Copy Markdown
Member

@Rich-Harris Rich-Harris left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you!

@Rich-Harris Rich-Harris merged commit 18db0ca into sveltejs:main Feb 27, 2026
17 of 18 checks passed
@github-actions github-actions bot mentioned this pull request Feb 27, 2026
Rich-Harris pushed a commit that referenced this pull request Feb 27, 2026
This PR was opened by the [Changesets
release](https://github.com/changesets/action) GitHub action. When
you're ready to do a release, you can merge this and the packages will
be published to npm automatically. If you're not ready to do a release
yet, that's fine, whenever you add more changesets to main, this PR will
be updated.


# Releases
## svelte@5.53.6

### Patch Changes

- perf: optimize parser hot paths for faster compilation
([#17811](#17811))

- fix: `SvelteMap` incorrectly handles keys with `undefined` values
([#17826](#17826))

- fix: SvelteURL `search` setter now returns the normalized value,
matching native URL behavior
([#17828](#17828))

- fix: visit synthetic value node during ssr
([#17824](#17824))

- fix: always case insensitive event handlers during ssr
([#17822](#17822))

- chore: more efficient effect scheduling
([#17808](#17808))

- perf: optimize compiler analysis phase
([#17823](#17823))

- fix: skip redundant batch.apply
([#17816](#17816))

- chore: null out current_batch before committing branches
([#17809](#17809))

Co-authored-by: github-actions[bot] <41898282+github-actions[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants