Skip to content
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

page.data mutable in legacy mode #13178

Open
henrykrinkle01 opened this issue Dec 16, 2024 · 5 comments
Open

page.data mutable in legacy mode #13178

henrykrinkle01 opened this issue Dec 16, 2024 · 5 comments
Labels

Comments

@henrykrinkle01
Copy link
Contributor

henrykrinkle01 commented Dec 16, 2024

Describe the bug

I found that the new page.data is mutable in legacy mode but not in rune mode.

Reproduction

Can't import from the new '$app/state` module in Sveltelab for unknown reasons (Kit updated). So here is the code:

//+page.server.ts
export async function load() {
    return { random: Math.random() }
}
<script lang="ts">
	import { page } from '$app/state';
</script>

{page.data.random}
<button onclick={() => page.data.random++}> Increment </button>

Logs

No response

System Info

N/A

Severity

annoyance

Additional Information

No response

@eltigerchino
Copy link
Member

eltigerchino commented Dec 17, 2024

If anything, page.data should be immutable in both modes with clearer warnings. Although, that conflicts with how it currently works with the page store...

SvelteKit makes three readonly state objects available via the $app/state module — page, navigating and updated.

@dummdidumm
Copy link
Member

This isn't related to the page state specifically, it's related to how Svelte 4 (and therefore legacy mode) behaves when imports are mutated.

In this playground example, you can see that when clicking the button, incrementing the value causes reactivity: The $: statement reruns, and the attribute is updated - only the text is not updated for some reason.

So while we could adjust Svelte 5 to not update the text when mutations to an import happen, we cannot and should not adjust the behavior here, as this has nothing to do with $app/state specifically.

Re immutability warnings: I'm not sure what we can really do here without wrapping it in a proxy, and that will cause problems itself. I think it's fine as is.

@henrykrinkle01
Copy link
Contributor Author

Why can't page.data be mutable by default?
We found a way to make it mutable anyway

@dummdidumm
Copy link
Member

Because it's a recipe for chaos - better to have it be a reflection of what your actual data is and change it through specific actions

@henrykrinkle01
Copy link
Contributor Author

I just wanted something like a mutable data prop, which is currently immutable. Questions related to this problem pop up almost everyday in the Discord server. Therefore the first thing I tested when the PR came out was whether page.data is mutable. I was kinda surprised that it is but later found out that it isn't in rune mode.
Do you have any update on PR #11809?

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

3 participants