Skip to content

The data prop and subsequent $derived not mutable by default #13626

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
henrykrinkle01 opened this issue Mar 22, 2025 · 14 comments
Open

The data prop and subsequent $derived not mutable by default #13626

henrykrinkle01 opened this issue Mar 22, 2025 · 14 comments
Labels

Comments

@henrykrinkle01
Copy link
Contributor

Describe the bug

Now that $derived has been made writable. This is intended to make optimistic UI updates possible without having to use the $state in $derived.by dance. However, it still doesn't entirely solve the problem because data is a $state.raw, which means mutations to data or its deriveds won't update the UI. To make it work like Svelte 4 did, the trick is still needed, which is kinda ugly and not beginner friendly.

I believe making data a full $state instead of $state.raw would solve this problem. Any reasons not to do so?

Reproduction

https://svelte.dev/playground/a69b38c2e041412989ca3edb509a0138?version=5.25.2

Logs

System Info

Svelte 5.25.2
SvelteKit 2.20.2

Severity

annoyance

Additional Information

No response

@gterras
Copy link
Contributor

gterras commented Mar 23, 2025

IMO state would be a good default but switching to raw when needed should be as simple as possible, although this may mean making data less magical and more explicit.

@elliott-with-the-longest-name-on-github
Copy link
Contributor

I think this is intentional (though Rich might hop in here and tell me I'm wrong). What you can do (and what we prefer) is to make it explicit that you intend to optimistically update something:

const { data } = $props();
const likesCheeseburgers = $derived(data.likesCheeseburgers);

likesCheeseburgers = true;

@henrykrinkle01
Copy link
Contributor Author

Right, but that only works if you reassign likesCheeseburgers as a whole, which makes sense if it's a primitive like boolean.
If it's an object and we want to mutate the object instead of reassigning it, it doesn't update the UI.

@oxisto
Copy link

oxisto commented Mar 24, 2025

Right, but that only works if you reassign likesCheeseburgers as a whole, which makes sense if it's a primitive like boolean. If it's an object and we want to mutate the object instead of reassigning it, it doesn't update the UI.

Exactly! The examples given by the $derived / data example are just too simplified. Usually data contains a complex structure, like for example some business data, e.g., a customer that you return as data and that you want to edit as part of the page and then sent back to the server (either directly or using server actions).

@dummdidumm
Copy link
Member

Most of the time - and the way the APIs work push you in that direction - it's better to not mutate, instead reassign the data. In other words, instead of data.foo = 'bar' you do data = { ...data, foo: 'bar' }. That way the update is truly optimistic since it's local to your component, and doesn't have unintended consequences in other parts of your app.

@oxisto
Copy link

oxisto commented Mar 31, 2025

Most of the time - and the way the APIs work push you in that direction - it's better to not mutate, instead reassign the data. In other words, instead of data.foo = 'bar' you do data = { ...data, foo: 'bar' }. That way the update is truly optimistic since it's local to your component, and doesn't have unintended consequences in other parts of your app.

Then what's the point of having the rune system in the first place then?

@dummdidumm
Copy link
Member

Universal reactivity, easier to read code, more predictable behavior, being able to deprecate (and at some point remove) APIs like $$slots/$$props/$$restProps, and much more

@henrykrinkle01
Copy link
Contributor Author

Most of the time - and the way the APIs work push you in that direction - it's better to not mutate, instead reassign the data. In other words, instead of data.foo = 'bar' you do data = { ...data, foo: 'bar' }. That way the update is truly optimistic since it's local to your component, and doesn't have unintended consequences in other parts of your app.

It's easy to say when foo is only 'bar'. It's really unhandy when data contains deeply nested objects and arrays. I don't understand the hypothetical consequences of doing this:

data.deeply.nested.object.property= 'new value'

vs this:

data = {...data, deeply: {...data.deeply, nested: {...data.deeply.nested, object: {...data.deeply.nested.object, property: 'new value'}}}};

It wasn't like this in Svelte 4.

I feel like this is a purist ideology similar to '$derived is classically considered read-only' that should be reconsidered.

@brunnerh
Copy link
Member

I also think this might be worth considering but if this is enabled by adding a state proxy to data (and maybe form), this would also be a breaking change...

@henrykrinkle01
Copy link
Contributor Author

I can't think of anything this change would 'break', but perhaps I'm ignorant. I think this is more an enhancement rather than a breaking change. Like moving from read-only $derived to writable is also technically a breaking change. If the change does break something existing and/or have unintended consequence, at least make it easy to opt-in and keep the current default.

@sacrosanctic
Copy link
Contributor

sacrosanctic commented Mar 31, 2025

It currently doesn't affect data in other components.

I presume wrapping data in a state proxy changes that.

@henrykrinkle01
Copy link
Contributor Author

If you're talking about page.data then right. However currently nobody is actually doing page.data mutation, at least without a hacky workaround.
If data is a fully proxified $state and you want to make changes local to the current component only, it's easy to do:

let localData = $derived($state.snapshot(data));

If a dev wants to mutate data (and the global page.data) directly, let them deal with the possible consequences. Make it explicit with $bindable and warn with invalid binding mutation, which is in line with how Svelte works in other components

@brunnerh
Copy link
Member

brunnerh commented Mar 31, 2025

The change to $derived was not breaking because prior to the change, reassignment was prevented by the compiler, code doing it could not exist.

Adding a proxy is a breaking change because certain APIs just do not work with them (e.g. structuredClone), this code would then throw errors.

@henrykrinkle01
Copy link
Contributor Author

So it seems the least invasive solution is proxifying data in a $derived.
It would've been nice if this syntax was allowed as an alternative to the current $state in $derived.by trick:

let localData= $derived($state(data));

It's simple and straight-forward for anyone new to Svelte(Kit) to understand

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

8 participants