svelte
svelte copied to clipboard
Svelte5: Deep reactivity is unintuitive with reactive Maps
Describe the bug
Intuitively, I would expect this to be reactive:
import { Map } from 'svelte/reactivity';
const s = $state(new Map([[0, { nested: 0 }]]));
// somewhere:
s.get(0).nested++; // should be reactive
However, reactive Maps (and Sets) don't proxy their values by default. This breaks the intuition that values passed to the $state
rune are made deeply reactive. I think even if this is out of scope for the reactivity helpers, it should be clearly documented.
Reproduction
https://svelte-5-preview.vercel.app/#H4sIAAAAAAAACm2SzW6DMAyAX8WKKgEqgq7SLkCZdtyhe4GxQ0bNlAlClLidqijvvvDTrXQ95GDnsz_LiWWNaNGw7M0yyTtkGXtWisWMzmoIzAlbQh-b_qjrIVOYWgtFZSUrEp3qNYGFPVfgoNF9B8FUkmrkNYmToHOQD-xwWiToP76wpgfYwcoQJwwtdFxlYB24KF9S23-UxO9BFkYz7PEVNo1nQ5_blWBHF82WxBcl3HexcOLtETPYgMuvke2IGKQw4EG84C6CiupeGgLh-3jwRRJqTy2FN8pkbLNe58vbyfY52aIbyMXwuNlEc6SRjlrCJKlb5PpXLCZmXlcli_TvTWShylc0hId5h36zi8meJqsrUnXh9-P-7014DftP0PUH0Qg8sIy0z767H5Uo1qc_AgAA
Logs
No response
System Info
n/a
Severity
annoyance
I had the same thought but I think they want to make it explicit. However you don't need to pass new Map
to state, you can just create the new Map
and if you want some object to be reactive inside it you should create it with $state
As mentioned writing $state(new Map())
does not make sense. $state
intentionally does not make things like Map
reactive. That's what svelte/reactivity
is for.
I would, however, expect it to work without wrapping it in a $state
:
const map = new Map([['foo', { deep: 0 }]]);
map.set('bar', { deep: 0 });
// I would expect both of these to be reactive:
map.get('foo').deep++;
map.get('bar').deep++;
Though technically this is possible:
const entries = $state([['foo', { deep: 0 }]]);
const map = new Map(entries);
let bar = $state({ deep: 0 });
map.set('bar', bar);
// Both of these are reactive:
map.get('foo').deep++;
map.get('bar').deep++;
That feels like a painful amount of boilerplate to achieve the same, and it seems prone to error as you could end up with some values being reactive and some not reactive:
const map = new Map();
let foo = $state({ deep: 0 });
map.set('foo', foo);
let bar = { deep: 0 };
map.set('bar', bar);
map.get('foo').deep++;
map.get('bar').deep++; // Oops, I did a dumb dumb, and now this is not reactive. Happy debugging!
All this makes me wonder if this works as designed, or if it could/should be improved.
As mentioned writing
$state(new Map())
does not make sense.$state
intentionally does not make things likeMap
reactive. That's whatsvelte/reactivity
is for.I would, however, expect it to work without wrapping it in a
$state
:const map = new Map([['foo', { deep: 0 }]]); map.set('bar', { deep: 0 }); // I would expect both of these to be reactive: map.get('foo').deep++; map.get('bar').deep++;
Though technically this is possible:
const entries = $state([['foo', { deep: 0 }]]); const map = new Map(entries); let bar = $state({ deep: 0 }); map.set('bar', bar); // Both of these are reactive: map.get('foo').deep++; map.get('bar').deep++;
That feels like a painful amount of boilerplate to achieve the same, and it seems prone to error as you could end up with some values being reactive and some not reactive:
const map = new Map(); let foo = $state({ deep: 0 }); map.set('foo', foo); let bar = { deep: 0 }; map.set('bar', bar); map.get('foo').deep++; map.get('bar').deep++; // Oops, I did a dumb dumb, and now this is not reactive. Happy debugging!
All this makes me wonder if this works as designed, or if it could/should be improved.
They were originally designed with deep reactivity and changed back to be explicit because Dominik thought not having a $state
anywhere was too magic.
I kinda agree with you that it would be nice to have it.
Ah, I see, that makes sense. I do understand the reasoning, and I can certainly agree with it to some extend. It's how I feel about the entirety of svelte/reactivity
actually. Personally I think it's weird to have primitives for $state
and $derived
, and having to import a magical Date
/Map
classes from svelte/reactivity
(though it's nice to get tree-shakeable implementations of the primitives and such!), but maybe that's another discussion.
Back on topic, I fear we'll end up seeing a lot of this in the wild:
import { Map } from 'svelte/reactivity';
class ReactiveMap extends Map {
constructor(entries) {
let e = $state(entries);
super(e);
}
set(key, value) {
let v = $state(value);
super.set(key, v);
}
}
export { ReactiveMap as Map };
It does the trick, but it leans toward writing JavaScript pre 2015, where everyone was writing the same utility functions because the base library was lacking.
Do you have a reference to any discussions about deep reactivity in maps/sets? I'd love to read about the reasoning, and I was looking in https://github.com/sveltejs/svelte/issues/10263, but some quick searches left me empty handed.
They were originally designed with deep reactivity and changed back to be explicit because Dominik thought not having a $state anywhere was too magic.
Would it be possible and desirable to enable deep reactivity if the Map
is wrapped in a $state
rune? Or even to automatically replace normal Map
s, Set
s, Date
s, etc. with their reactive versions if they're put inside a $state
rune?
Would it be possible and desirable to enable deep reactivity if the
Map
is wrapped in a$state
rune?
It may be possible, but would break the current logical semantics of how classes work, since when you try doing it with SomeOtherClass
, the behavior is basically the opposite.
Or even to automatically replace normal
Map
s,Set
s,Date
s, etc. with their reactive versions if they're put inside a$state
rune?
No, https://github.com/sveltejs/svelte/issues/10263#issuecomment-1910900295.
It may be possible, but would break the current logical semantics of how classes work, since when you try doing it with SomeOtherClass, the behavior is basically the opposite.
eh, I disagree - you're explicitly importing Map
from svelte/reactivity
here, my mental model views it as a builtin versus a class
@ottomated I believe that the point is that if you would do this:
class MyClass {
#something = {}
setSomething(value) {
this.#something = value;
}
getSomething() {
return this.#something;
}
}
const myClass = $state(new MyClass());
It would not result in a "reactive class", which would cause a lot of confusion among developers if $state(new Map())
does result in a reactive class.
This is the subtle difference between magic and voodoo.
Suppose we were to change things so that values were automatically replaced with reactive proxies — what would a good opt-out mechanism look like?
Suppose we were to change things so that values were automatically replaced with reactive proxies — what would a good opt-out mechanism look like?
$state.frozen(new Map())
is my intuition, but then my intuition was also that $state(new Map())
would be required.
After using svelte 5 for a few projects, I've built the habit of wrapping anything that needs to be reactive in a rune of some kind. It feels a bit more magic than magical that svelte/reactivity
hides that away. Maybe the syntax could be
import { $map } from 'svelte/reactivity';
const map = $map([['key', 'value']]);
// ^? Map<string, string>
// opt-out
const shallowMap = $map.frozen();
But then you would need to do $state({ map: $map() })
which would definitely need at least some lint rules to feel right.
An opt-out mechanism could not be something like new ReactiveMap([], { deep: false})
though. I don't think you can deviate from the base class's public API at all.
Yeah that doesn't seem like an improvement frankly. For one thing what is a frozen map? It implies you can't set
or delete
etc.
let bar = $state({ deep: 0 });
map.set('bar', bar);
This doesn't seem like a painful amount of boilerplate to me, it seems like a reasonable way to opt in to the desired behaviour.
I don't think you can deviate from the base class's public API at all.
And this is basically what makes me uncomfortable about the proposal to make values reactive by default. This...
let foo = { deep: 0 };
map.set('foo', foo);
console.log(map.get('foo') === foo); // false!
...violates my expectations a bit too much.
For one thing what is a frozen map? It implies you can't
set
ordelete
etc.
Sure, frozen is a bad word for it - could be $map.shallow
, or default to shallow and add $map.deep
. More important than the semantics is the underlying idea of using rune-like names for svelte/reactivity
exports, would like to hear your thoughts on that or if the API is already too set in stone.
This doesn't seem like a painful amount of boilerplate to me, it seems like a reasonable way to opt in to the desired behaviour.
I don't think it's a boilerplate issue, I think it's an expected behavior issue. $state
being proxied by default makes this really hard.
// this works
const obj = $state({ });
obj.foo = { deep: 0 };
// this doesn't - but it feels the same!
const map = $state(new Map());
map.set('foo', { deep: 0 });
And this is basically what makes me uncomfortable about the proposal to make values reactive by default.
I would call that a different thing than changing the public API. I would also argue that this is much weirder:
const map = $state({});
let foo = { deep: 0 };
map.foo = foo;
console.log(map.foo === foo); // false!
More important than the semantics is the underlying idea of using rune-like names for
svelte/reactivity
exports, would like to hear your thoughts on that or if the API is already too set in stone.
Names beginning with $
are prohibited, because if you can do this...
import { $map } from 'somewhere';
...then you can also do this, which is just a recipe for silliness:
import { $state } from 'somewhere';
We could say 'you can have names beginning with $
except for existing runes', but then we'd be forgoing the ability to add new runes without breaking changes, which would be a grave error.
The alternative is to make $map
and $set
etc actual runes, but that would also be a big mistake. For one thing, it dilutes the 'rune' concept to mean gestures vaguely towards 'reactivity' rather than 'an instruction to the compiler'. For another, it sucks from a day-to-day usability standpoint if you start typing $s
or $d
and the first autocomplete suggestions are $set
and $date
rather than the far more commonly-used $state
and $derived
.
Finally, we want to avoid the impression that there's something special about these classes — Map
and Set
and Date
and URL
are things that you could build in userland. We're making that unnecessary, because we're a batteries-included framework, but the point is that you can use Svelte's primitives to make your own reactive classes. Using runic notation would undermine that message.
I would also argue that this is much weirder
It's not weirder. At most, it's equivalently weird, but I'd argue that big-blob-of-reactive-state is a more appropriate place for that sort of weirdness than something that, aside from causing effects to rerun on updates, is designed to exactly replicate built-in APIs.
Names beginning with
$
are prohibited
This makes sense.
The alternative is to make
$map
and$set
etc actual runes, but that would also be a big mistake.
This was my thought too.
Finally, we want to avoid the impression that there's something special about these classes —
Map
andSet
andDate
andURL
are things that you could build in userland. We're making that unnecessary, because we're a batteries-included framework, but the point is that you can use Svelte's primitives to make your own reactive classes. Using runic notation would undermine that message.
I'm trying to reconcile my thoughts this, because at heart I agree with this as well. Here's the difference, I think:
A. Some people using Svelte 5 will only use deep reactive state, because that's what they're used to in Svelte 4. When those people see svelte/reactivity
, they expect it to be deeply reactive by default. They don't care that they can make their own reactive classes, and probably aren't seeing Map
, Set
, etc. as classes, but rather as builtins. In fact, default javascript Map
s are "deeply reactive" in Svelte 4 if you use map = map
.
B. Once people build an intuition for $state
in custom classes, this behavior makes perfect sense. I would even put myself in group A when I submitted this issue, and reading the responses has converted me to group B. However, even if the documentation on this is expanded, I expect many svelte 5 adopters to be in group A.
Edit: another note on this - I looked at the source code several times while trying to understand this at first and it doesn't avoid that impression - it uses internal methods rather than runes.
It's not weird_er_. At most, it's equivalently weird, but I'd argue that big-blob-of-reactive-state is a more appropriate place for that sort of weirdness than something that, aside from causing effects to rerun on updates, is designed to exactly replicate built-in APIs.
In the hierarchy of potential unintuitiveness, I think getters and setters on an object you defined yourself is higher than method calls on a class you imported, if that makes sense?
Side note: I wonder if it would be possible to patch the AST i.e. s.foo === foo
-> $state.snapshot(s.foo) === foo
and maintain equality this way.
I looked at the source code several times while trying to understand this at first and it doesn't avoid that impression - it uses internal methods rather than runes
It does, just because it's one less moving part if we don't need to run the Svelte compiler on its own source code in order to run tests etc. It would be slightly more cumbersome to implement in userland since we don't expose source
, but still totally doable.
Side note: I wonder if it would be possible to patch the AST
The thought has definitely occurred! It would likely help in cases like #11403. Reasons not to do it:
- performance overhead, unless it's a dev-only thing that says 'you probably meant to use
snapshot
' - uncanny valley, if
===
works differently in.svelte.js
vs.js
(e.g. you're passing values to a library function that isn't aware of this) - to preserve the illusion you'd need to patch things like
[].indexOf
and[].includes
, and likely many other things. it'd be a moving target and there'd always be ways to break it
Because of those, it's almost certainly better to just educate Svelte developers that foo.bar = bar; foo.bar === bar
won't necessarily evaluate to true
It does, just because it's one less moving part if we don't need to run the Svelte compiler on its own source code in order to run tests etc.
Yeah, I understand why, just a note.
Because of those, it's almost certainly better to just educate Svelte developers that
foo.bar = bar; foo.bar === bar
won't necessarily evaluate totrue
Yes, and that knowledge would extend to svelte/reactivity
automatically.
I honestly think the best solution to all of this is something like this.
import { map, set, url } from 'svelte/reactivity';
const m = map.deep();
const shallow_set = set();
const u = url('http://example.com');
It solves the opt-out / opt-in issue cleanly, and it also solves a different footgun I ran into: if you refactor a chunk of code into a new component that includes a ReactiveMap constructor but forget to import ReactiveMap, you'll silently lose reactivity.
In addition (I assume this is already on the roadmap), the documentation should include more of these examples, there should be more lint warnings around this, etc.
Closing since we don't want to change this. Even something like map.deep()
is more overhead compared to educating people about $state()
+ map.set(..)
- less concept to learn for the latter, and better reusable between use cases.
Agree that documentation should touch on this, but that's tracked more generally elsehwere.