sveltejs / svelte

Cybernetically enhanced web apps
https://svelte.dev
MIT License
78.04k stars 4.08k forks source link

Svelte 5: decide on behaviour of `$state.frozen` #12434

Closed Rich-Harris closed 1 week ago

Rich-Harris commented 1 month ago

Describe the problem

The discussion in #12413 revealed some differences of opinion around $state.frozen. As of that PR, it will error if passed existing reactive state, which eliminates one source of confusion. But others remain:

Describe the proposed solution

Together, these observations suggest (to me at least) we should change the name and the behaviour. Taking a cue from MobX, perhaps $state.ref would be a better name. $state.raw is another option. Doubtless there are others.

This would have similar semantics to $state.frozen today — in other words only reassigning (not mutating) will cause updates (or in implementation terms: it creates a source but not a proxy) — but without the dev-time Object.freeze.

Of course, by doing this we lose the opportunity to tell people that they're incorrectly mutating an object. If we want to keep that but also have consistency between dev and prod, then the alternative is to always use Object.freeze, perf implications be damned.

Importance

nice to have

brunnerh commented 1 month ago

The Object.freeze call struck me as a bit odd from the start. If someone really wants their objects frozen, that can always be done manually. Types can also be an avenue for indicating that things are read-only.

ref sounds a bit unspecific and could maybe be misinterpreted as creating a { value } box.

RyanYANG52 commented 1 month ago

I would suggest keep this api as close as Svelte 3/4's behaviour (no proxy only source), and not treat dev and prod differently (as @brunnerh suggested leave freeze to user land). It's a perf enhance / advanced api, user who pick this over $state() normally would read the doc and know what they were doing. Plus, for user who used Svelte 3/4 would be easy to understand and easy to migrate their existing code;

In my current codebase, I use a lot of object as a complex immutable value type, for instance {text: string, value/action: T, class?: string} as options which would never change separately

dummdidumm commented 1 month ago

My thoughts:

trueadm commented 1 month ago

remove the symbol and any checks. In other words, nothing is done to the object (but you're also responsible for not accidentally mixing it with proxied state)

I don't think we can do that. If you remember that's what original $state.raw did in my PR that I never landed – because the chances of accidentally proxying it are huge. For example you create some frozen state and then push it into an array, or assigning a property on some object to be the value.

trueadm commented 1 month ago

Personally, I'm in favour of keeping $state.frozen as is and separately introducing $state.raw and seeing how that goes.

Rich-Harris commented 1 month ago

That feels like the worst of all options honestly — extra surface area, and it would be very hard to articulate when to use which.

To the best of my understand/recollection (which may not be fully accurate) the reason we added the symbol was to deal with this scenario:

let object = $state.frozen({ x: 1 });
let array = $state([object]); // oops, `array[0]` is now a proxy, we didn't want that

Svelte doesn't proxify objects with the symbol, meaning $state.frozen avoids that issue. But maybe we were thinking about this backwards. Nothing about object itself changes when array[0] is proxified; that array[0].x is reactive is a fact about array, not a fact about object. So we should adjust the array declaration accordingly.

Currently though, we don't have a way to express 'this thing is reactive — we can do array.push(...) and so on — but its children shouldn't inherit that reactiveness'. That's the missing piece that led us down this path. So here's what I propose:

let object = $state.raw({ x: 1 }); // reassigning (but not mutating) `object` will cause updates
let array = $state.shallow([object]); // we can push/pop/etc, but the members of the array are untouched

// or...
let array = $state.shallow([{ x: 1 }]);

array[0].x += 1; // triggers no updates (should probably yield a warning, on a best-effort basis)
ryanatkn commented 1 month ago

This is maybe offtopic and I'm not requesting the rationale, just offering a thought expressed by others in various forms - is deep reactivity the best default? With limited experience I think I'd prefer $state() proxying nothing as the default raw form, and then also having $state.shallow() and $state.deep(), if it makes sense.

It would feel like less of a surprise/risk if .deep() is visible. The caveats around deep state, including the boundary at reactive builtin collections, make deep state uncommon in my current patterns, but I'm still adapting to thinking in those terms. (I was using immutable: true)

There's also the idea switching to the less elegant SvelteArray and object/record that could be returned by runes to make accidental mutation mistakes less of a problem via types.

dummdidumm commented 1 month ago

$state.shallow sounds a bit like scenario-solving to me - I imagine it only really being useful for arrays, because you don't want to lose the "push/pop/etc triggers reactivity" niceness. For objects it's less useful, and it's also very inflexible because what if I want reactivity two levels deep instead?

If it's really "only" about arrays, there are other solutions I think would be better:

peterreeves commented 1 month ago

...what if I want reactivity two levels deep instead?

Wouldn't that just be this?

let position = { x: 1, y: 1 };
let transform = $state.shallow({ position });
let obj = $state.shallow({ transform });
Rich-Harris commented 1 month ago

Gah, GitHub didn't show me more recent replies. Editing:

is deep reactivity the best default?

There's definitely room for debate on this topic, but so far when we've had that discussion we've landed on deep reactivity being the right default — the idea being that by using the $state rune you're already declaring your intent. It's a more robust version of how Svelte 4 already works (foo.bar += 1 invalidating foo), and mirrors the choice made by other systems (Vue's ref, MobX's observable).

I suspect that for many people, the question arises precisely because we don't have an opt-out mechanism at present. It's clearly a gap in the API (and one that the aforementioned systems cover with shallowRef and observable.shallow respectively).

Wouldn't that just be this?

It's definitely solvable, yeah. But I think 'I want reactivity two levels deep' is a real edge case anyway

peterreeves commented 1 month ago

Thinking about it more, do you need a $state.shallow(...) rune? Could you just get away with only a $state.raw(...) and prevent it's contents from being proxied if it was included inside another $state? e.g.

let obj = $state({
    transform: {
        position: $state.raw({
            x: 0,
        })
    },
    material: {
        colour: {
            r: 1,
        },
    }
});

// ...elsewhere
obj.transform.position.x = 2; // Does NOT trigger reactivity, as reactivity "stops" at `position`
obj.material.colour.r = 0.5; // Does trigger reactivity

If so that seems like a pretty clear way to carve up which bits of an object should be reactive, and which bits shouldn't. Just wrap things you don't want to be reactive in $state.raw(...). If you want it to be immutable, use $state.raw(Object.freeze(...)).

trueadm commented 1 month ago

I'm more leaning towards $state.frozen to be deeply frozen in DEV too. I feel one of the nice things about $state being deeply reactive is that it means that Svelte can track the values of all the nested properties. This enables us to enable future work where we might suspend or fork the current state tree and do nice things with immutability for free. If $state.frozen was deep, then you'd also get this nice guarantee too. However, when I think of $state.raw, it seems that the benefit is that you can now mutate state without it applying to the UI – and this also means that we no longer track these values, meaning the above benefits now are no longer viable to us.

Rich-Harris commented 1 month ago

Could you just get away with only a $state.raw(...) and prevent it's contents from being proxied if it was included inside another $state?

I don't think so. For one thing, you'd need to add a symbol to the position object to create a boundary, which isn't a dealbreaker but is something that would be nice to avoid.

But the larger issue is that you would need to use it on every assignment...

obj.transform = {
  position: $state.raw({
    x: 1
  })
};

...otherwise position would be proxified at that point. To me that feels like an abstraction leak. For what it's worth you can essentially do that today in userland, by virtue of the fact that classes are treated as 'boundaries':

class Raw {
  constructor(properties) {
    Object.assign(this, properties);
  }
}

let obj = $state({
  transform: {
    position: new Raw({
      x: 0,
    })
  },
  material: {
    colour: {
      r: 1,
    },
  }
});

It's also much more annoying to do this...

const swatch = $state([
  $state.raw(createColour('red')),
  $state.raw(createColour('orange')),
  $state.raw(createColour('yellow')),
  $state.raw(createColour('green')),
  $state.raw(createColour('blue')),
  $state.raw(createColour('indigo')),
  $state.raw(createColour('violet'))
]);

...than this:

const swatch = $state.shallow([
  createColour('red'),
  createColour('orange'),
  createColour('yellow'),
  createColour('green'),
  createColour('blue'),
  createColour('indigo'),
  createColour('violet')
]);

I'm more leaning towards $state.frozen to be deeply frozen in DEV too

I thought we were leaning towards getting rid of the dev/prod discrepancy? The main reason to use this thing — whatever form it takes — is that you have a large blob of data that you don't plan to mutate, and so you're opting out of deep anything.

when I think of $state.raw, it seems that the benefit is that you can now mutate state without it applying to the UI

I don't think that's right. You're not declaring something with $state.raw so that you're free to mutate it, you're declaring something with $state.raw because you don't intend to mutate it.

trueadm commented 1 month ago

I thought we were leaning towards getting rid of the dev/prod discrepancy? The main reason to use this thing — whatever form it takes — is that you have a large blob of data that you don't plan to mutate, and so you're opting out of deep anything.

Almost every immutable library I know makes use of Object.freeze and they all avoid it in prod too, because it's expensive. That's also why we removed it from prod. It's frustrating that we add the Symbol, I agree. However, to keep things consistent is was the only way.

I don't think that's right. You're not declaring something with $state.raw so that you're free to mutate it, you're declaring something with $state.raw because you don't intend to mutate it.

Up until this point, every use-case has been for it being mutable by "third party" things. There's half a dozen issues related to this, and so I feel like people will just use it for that intention.

I've got strong mixed feelings on $state.raw really. I don't think it's really providing any real benefits over what we have already today without introducing its own set of drawbacks.

Rich-Harris commented 1 month ago

Up until this point, every use-case has been for it being mutable by "third party" things

Really? I feel like the primary use case is more like this:

let data = $state.raw();

poll('/some-endpoint', 5000, (v) => {
  if (data === undefined || v.timestamp > data.timestamp) {
    data = v; // big blob of data that will never be mutated
  }
});

If I was still doing my previous job at the NYT, building visualisations of big hairy datasets, then that's exactly what I'd want to do (and I'd be pissed if Svelte took it upon itself to call Object.freeze recursively, even if only in dev).

trueadm commented 1 month ago

I just see it being used to wrongly solve https://github.com/sveltejs/svelte/issues/12364

dummdidumm commented 1 month ago

I see it like this:

So yes, it can be used to do things which are not forkable. But the people writing code in such a way probably are not the people who care about a world in which they can do forking, and/or cannot make use of it anyway, because they want to / have to integrate with third party libraries which have different requirements (#12364 actually is a good example of this, as it's not practical to require people to not use certain patterns if that's the only choice they have to integrate with something).

trueadm commented 1 month ago

I see it like this:

  • it's a "leave me alone I know what I'm doing" mode
  • it's a "don't interfere with this outside stuff I'm doing" mode

So yes, it can be used to do things which are not forkable. But the people writing code in such a way probably are not the people who care about a world in which they can do forking, and/or cannot make use of it anyway, because they want to / have to integrate with third party libraries which have different requirements (#12364 actually is a good example of this, as it's not practical to require people to not use certain patterns if that's the only choice they have to integrate with something).

You just described classes though. Which are a superior primitive in Svelte 5 for this purpose. If people are using object literals and arrays in state, they will make mistakes, regardless of interference from the framework. I know this so well first-hand working on React.

I stand by what I said before, strongly too. $state.raw as it stands isn’t compelling enough to convince me otherwise.

dummdidumm commented 1 month ago

But it's not practical to force classes upon people, again they might even be able to use them because the integration requires them to pass in POJOs, or gives POJOs back out which you then would need to class-ify somehow.

trueadm commented 1 month ago

But it's not practical to force classes upon people, again they might even be able to use them because the integration requires them to pass in POJOs, or gives POJOs back out which you then would need to class-ify somehow.

I’ve not seen any convincing cases of that outside of examples where they can easily be adapted. I think it’s more of an educational thing.

dummdidumm commented 1 month ago

I don't think you can easily adapt (if at all) all use cases. The third party library might have certain requirements about the state passed to it (like it not being frozen or proxified), or (more common) the third party integration will give you a POJO and it would be super annoying/cumbersome to convert those to classes just so that they are not proxified. Take Rich's example:

poll('/some-endpoint', 5000, (v) => {
  if (data === undefined || v.timestamp > data.timestamp) {
    data = v; // big blob of data that will never be mutated
  }
});

If I had to convert the whole thing to classes just so that they are left alone would be imperformant and very annoying to do - maybe even impossible if you don't know the shape up front.

Also, if your solution is to use classes to mutate things, then that still means things can be mutated and therefore not reversible from a fork anyway, so nothing is won. The reality of software development is that not everything can be forced into the way we want it to be, and $state.raw is a good compromise in that direction.

trueadm commented 1 month ago

I don't think you can easily adapt (if at all) all use cases. The third party library might have certain requirements about the state passed to it (like it not being frozen or proxified), or (more common) the third party integration will give you a POJO and it would be super annoying/cumbersome to convert those to classes just so that they are not proxified. Take Rich's example:

poll('/some-endpoint', 5000, (v) => {
  if (data === undefined || v.timestamp > data.timestamp) {
    data = v; // big blob of data that will never be mutated
  }
});

If I had to convert the whole thing to classes just so that they are left alone would be imperformant and very annoying to do - maybe even impossible if you don't know the shape up front.

Also, if your solution is to use classes to mutate things, then that still means things can be mutated and therefore not reversible from a fork anyway, so nothing is won. The reality of software development is that not everything can be forced into the way we want it to be, and $state.raw is a good compromise in that direction.

That compromise isn’t what Rich is saying above though. He’s not mutating the object.

dummdidumm commented 1 month ago

Yes, but he also doesn't want it to be frozen or have a symbol attached or anything - it should be left alone. And and others wouldn't want to mutate the object either, unless they have to because their third party integration does require it.

Rich-Harris commented 1 month ago

He’s not mutating the object.

Are you saying that the 'correct' solution is to do this?

poll('/some-endpoint', 5000, (v) => {
  if (data === undefined || v.timestamp > data.timestamp) {
-    data = v;
+    data = new Raw(v);
  }
});

Because that is absurd! Classes are absolutely not the right tool for this job, and nor is freezing. Other systems have an equivalent API, there's no reason we shouldn't.

trueadm commented 1 month ago

Are you saying that the 'correct' solution is to do this?

No, I'm not.

Other systems have an equivalent API, there's no reason we shouldn't.

So from my point of view, we should aim to have the guarantees of immutability as that enables some powerful future provisioning around features. So that means we either have mutable pseudo immutability with signals ($state) or full immutability with frozen. That way we can safely carry out work in forks and throw away work without it causing side-effects, which is important for things like Suspense and async forking/transitions.

Rich-Harris commented 1 month ago

No, I'm not.

What then?

we should aim to have the guarantees of immutability

The use of $state.raw is the guarantee of immutability. If someone uses a feature that is documented as being for that exact purpose, we can treat the data as though it's immutable, and if it breaks because the user willfully used it incorrectly that's on them. Once again:

You're not declaring something with $state.raw so that you're free to mutate it, you're declaring something with $state.raw because you don't intend to mutate it.

Forcing Object.freeze down everyone's throats because you don't trust users to read the documentation is overly paternalistic.

trueadm commented 1 month ago

The use of $state.raw is the guarantee of immutability. If someone uses a feature that is documented as being for that exact purpose, we can treat the data as though it's immutable, and if it breaks because the user willfully used it incorrectly that's on them.

Again, that just doesn't fly with me. Not unless we add a DEV time warning that it was incorrectly used. The whole "it's on them" has never scaled in frontend frameworks, it just leads to people avoiding future upgrades because the level of effort required to change over likely isn't worth it to them. I'm being stubborn with this because it's a topic that is close to my heart and I have a great deal of experience trying to tackle migrations that have suffered from this.

For example, if you suggested that in DEV we deeply put each object literal/array into a WeakMap, and then wherever we have an object mutation the compiler adds in a check to see if that object was in the WeakMap – then you'd likely capture these cases without the need to use proxies.

Rich-Harris commented 1 month ago

The sorts of cases I'm describing are graphics like this election results page, which is powered by (inter alia) this 6.5MB blob of JSON.

Assuming that we don't want to use Object.freeze, because we don't want the dev/prod discrepancy it introduces, that leaves us with the WeakSet approach. For the JSON above we're talking about adding more than 38,000 objects to the set. Which is fast enough to do that performance probably isn't a dealbreaker, but we can only do the mutation validation inside .svelte and .svelte.js files. Are we okay with that caveat?

There's a much bigger problem though:

<script>
  let element = $state.raw();

  function update_document_title() {
    document.title = 'updated'; // oops! element.ownerDocument is frozen
  }
</script>

<div bind:this={element}>...</div>

If we have to say to people '$state.raw creates a source but not a proxy, but please don't use it in this probably non-exhaustive list of scenarios in which it will cause extremely hard-to-debug breakage' then we may as well tell them to use a different framework.

Maybe there are some tricks we can use to avoid those cases — ignoring accessors, or only freezing POJOs — but it feels like a game of whac-a-mole that will end up with a convoluted set of rules that no-one really understands. There's a difference between guiding users towards successful outcomes and treating them like children, and this feels like the latter to be honest.

dummdidumm commented 1 month ago

We gotta meet people where they are - we can't force everything through the Svelte-way of doing things. That would be very much the opposite of what Svelte 3/4 did. Also in other places we say "if you want to mutate things, use a class", so we're already saying that there ways around this. And this is the reality: There will always be ways around this. JS as a language isn't very good at immutability, and so we can't design future work under the premise that everything is. What we can do is design new APIs that would under certain circumstances not work quite correct because you can't revert things because you didn't play by the rules, but that's on you. But what we absolutely cannot do is design APIs that people are forced to use that also force them to use immutability.

trueadm commented 1 month ago

But what we absolutely cannot do is design APIs that people are forced to use that also force them to use immutability.

As a side note, you most definitely can. It's clear that you and Rich don't want to do that though, so I'm happy to leave that conversation.

However, I still feel we need to do more here than just invent $state.raw and have it be just a plain object. I do think we need to consider it's interaction with other APIs, such as $state and also we need to make people aware that they shouldn't mutate the object passed to $state.raw. Many people might not know that they should, so friendly warnings are beneficial. As to how far we want to go with those warnings is up to us. However, I don't think we should do nothing. That seems unacceptable, given we warn on so many other things. For example mutating state passed as a prop has had so much work put into helping people avoid doing it to non-bound props – and that's good. I just feel like we need some level or protection here – because most mutations might be innocent or accidental, regardless of how well someone knows Svelte.

FoHoOV commented 1 month ago

The use of $state.raw is the guarantee of immutability. If someone uses a feature that is documented as being for that exact purpose, we can treat the data as though it's immutable, and if it breaks because the user willfully used it incorrectly that's on them. Once again:

You're not declaring something with $state.raw so that you're free to mutate it, you're declaring something with $state.raw because you don't intend to mutate it.

@Rich-Harris Can I ask what is the reasoning that something like this warrants a warning (also there are lots of other missuses that produce warnings and saved me bunch of times, I really love svlete cuz of these DX things), but this doesn't (which is a LOT more prone to bugs IMHO) and should be resorted to docs only? I hope that I do understand your point, if I see a variable named like SOME_SUTFF I get that this is a constant and even if I can change the data, I would not. This is a naming convention whilest $state.raw() isn't and I personally can't use the same logic here. If it's technically possible to error or warn on mutations then why not?

Rich-Harris commented 1 month ago

Because per https://github.com/sveltejs/svelte/issues/12434#issuecomment-2239038471 it doesn't seem like it is technically possible

peterkogo commented 3 weeks ago

I agree with the notion of providing more flexibility with $state.raw instead of correct immutability, as this would make my life easier.

possibly/maybe: make foo = foo always trigger an update, not just when the values aren't equal. Satisfies demands of people who want to wrap some third party stuff and want to trigger reruns by doing this, and also reflects the "your on your own"-style of this feature

This would be the cherry on top.

Svelte 5s ergonomics already excel in most use cases but implementing more complicated shenanigans is hard/impossible. This proposal would open up paths for more controlled optimizations in user land, especially for libraries.

trueadm commented 3 weeks ago

I agree with the notion of providing more flexibility with $state.raw instead of correct immutability, as this would make my life easier.

possibly/maybe: make foo = foo always trigger an update, not just when the values aren't equal. Satisfies demands of people who want to wrap some third party stuff and want to trigger reruns by doing this, and also reflects the "your on your own"-style of this feature

This would be the cherry on top.

Svelte 5s ergonomics already excel in most use cases but implementing more complicated shenanigans is hard/impossible. This proposal would open up paths for more controlled optimizations in user land, especially for libraries.

We want to keep equality as it is in runes mode for multiple reasons including performance and future features such as error boundaries and async state.

arackaf commented 2 weeks ago

I know I'm not in the maintainer's guild for Svelte 🙂, but as an enthusiastic user, and an engineer with decent experience working on decently large systems, I'm very excited by

let object = $state.raw({ x: 1 }); // reassigning (but not mutating) `object` will cause updates
let array = $state.shallow([object]); // we can push/pop/etc, but the members of the array are untouched

landing. Those are features I've already been looking for in Svelte 5, and have cobbled together workarounds for. I actually have a blog post pending on FrontendMasters about these very things, and I hate the complexity I had to show to achieve those basic features (which exist in other solutions like MobX).

Using something like $state.shallow() for a reactive array of non-reactive objects is an incredibly common use case. It's wasteful to set up deeply reactive plumbing for objects you know will not be changing. But here's the thing: I would not be looking for strong guarantees of immutability. I merely want reactivity to NOT be set up for all those individual properties.

Most importantly, if I DO want strong guarantees of immutability, I'm free to just pull an immutable js library off the shelf, and stick a truly immutable array right inside of $state.shallow (or actually, I believe I'd use $state.raw if I understand Rich's proposal correctly).

chaiyo4096 commented 2 weeks ago

 ส่งจากแท็บเล็ต Huawei ของฉัน-------- ข้อความต้นฉบับ --------จาก: Adam Rackis @.>วันที่: จ. 5 ส.ค. 2024 03:13ถึง: sveltejs/svelte @.>สำเนา: Subscribed @.***>เรื่อง: Re: [sveltejs/svelte] Svelte 5: decide on behaviour of $state.frozen (Issue #12434) I know I'm not in the maintainer's guild for Svelte 🙂, but as an enthusiastic user, and an engineer with decent experience working on decently large systems, I'm very excited by let object = $state.raw({ x: 1 }); // reassigning (but not mutating) object will cause updates let array = $state.shallow([object]); // we can push/pop/etc, but the members of the array are untouched landing. Those are features I've already been looking for in Svelte 5, and have cobbled together workarounds for. I actually have a blog post pending on FrontendMasters about these very things, and I hate the complexity I had to show to achieve those basic features (which exist in other solutions like MobX). Using something like $state.shallow() for a reactive array of non-reactive objects is an incredibly common use case. It's wasteful to set up deeply reactive plumbing for objects you know will not be changing. But here's the thing: I would not be looking for strong guarantees of immutability. I merely want reactivity to NOT be set up for all those individual properties. Most importantly, if I DO want strong guarantees of immutability, I'm free to just pull an immutable js library off the shelf, and stick a truly immutable array right inside of $state.shallow (or actually, I believe I'd use $state.raw if I understand Rich's proposal correctly).

—Reply to this email directly, view it on GitHub, or unsubscribe.You are receiving this because you are subscribed to this thread.Message ID: @.***>

trueadm commented 2 weeks ago

I know I'm not in the maintainer's guild for Svelte 🙂, but as an enthusiastic user, and an engineer with decent experience working on decently large systems, I'm very excited by

let object = $state.raw({ x: 1 }); // reassigning (but not mutating) `object` will cause updates
let array = $state.shallow([object]); // we can push/pop/etc, but the members of the array are untouched

landing. Those are features I've already been looking for in Svelte 5, and have cobbled together workarounds for. I actually have a blog post pending on FrontendMasters about these very things, and I hate the complexity I had to show to achieve those basic features (which exist in other solutions like MobX).

Using something like $state.shallow() for a reactive array of non-reactive objects is an incredibly common use case. It's wasteful to set up deeply reactive plumbing for objects you know will not be changing. But here's the thing: I would not be looking for strong guarantees of immutability. I merely want reactivity to NOT be set up for all those individual properties.

Most importantly, if I DO want strong guarantees of immutability, I'm free to just pull an immutable js library off the shelf, and stick a truly immutable array right inside of $state.shallow (or actually, I believe I'd use $state.raw if I understand Rich's proposal correctly).

If you are mutating the raw object, are you happy for the UI to not update?

arackaf commented 2 weeks ago

If you are mutating the raw object, are you happy for the UI to not update?

Yes. I subscribed to that when I chose to use $state.shallow or $state.raw. That's the contract. That's the api. The binding itself is reactive, but the underlying values are not.

If I choose to mutate them, that's on me. If you all want to go the extra mile and rig up some warnings for when I do choose to edit properties that are non-reactive that would of course be nice to have, but certainly not required imo.

arackaf commented 2 weeks ago

$state.shallow sounds a bit like scenario-solving to me - I imagine it only really being useful for arrays, because you don't want to lose the "push/pop/etc triggers reactivity" niceness. For objects it's less useful, and it's also very inflexible because what if I want reactivity two levels deep instead?

If it's really "only" about arrays, there are other solutions I think would be better:

  • $state.raw gets some tiny additional code that's like "if this is an array, patch the mutating methods to trigger reactivity"
  • import { SvelteArray } from 'svelte/reactivity'

@dummdidumm I may have misunderstood Rich, but I think it's explicitly designed (only) for arrays (as opposed to "only really being useful for arrays"). It's a common enough use case that MobX has an identically named api for it

Special casing $state.raw to behave differently when passed an array feels like a bit of a code smell. Avoiding different behaviors based on the argument passed to $state (or a helper) feels like the reasoning behind disallowing return $state({}) in this thread.

Lastly, import { SvelteArray } from 'svelte/reactivity' feels like a huge departure from the rest of the Svelte api. Everything else would go through $state or $state.xyz, except for (shallowly) reactive arrays, which would go through the SvelteArray import.