Prevent unsafe server-side store updates

[Rich-Harris]

• closes SSR rendered page retains old store value in dev mode #8614

This aliases svelte/store to a virtual module that intercepts calls to writable and overwrites the set and update methods to throw an error if you try to use them on the server. This means that something like this...

// src/lib/stores.js
import { writable } from 'svelte/store';

export const user = writable(null);

// src/routes/+layout.server.js
import { user } from '$lib/stores.js';

export async function load(event) {
  user.set(await get_user(event));
}

...would throw an error in development.

We don't want this to apply for stores that were created during load or render, so we track whether that's currently happening and only apply the override when it's not. (Due to the async nature of data loading there's a very small chance of harmless false negatives.)

Needs tests

Please don't delete this checklist! Before submitting the PR, please make sure you do the following:

• It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
• This message body should clearly illustrate what problems it solves.
• Ideally, include a test that fails without this PR but passes with it.

Tests

• Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

Changesets

• If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpm changeset and following the prompts. Changesets that add features should be minor and those that fix bugs should be patch. Please prefix changeset messages with feat:, fix:, or chore:.

[dummdidumm]

Damn, that's a smart way to implement this. I think it needs to be configurable/opt-in and/or a warning though.

1. Some existing apps might suddenly crash, leaving people scratching their heads what's going on
2. For some apps this might be intentional. Imagine a Node environment where some persistent state across users (multiplayer game state for example) is handled through writables - something like that should still be possible

[Rich-Harris]

Downgrading it to a warning could work.

There's a more immediate practical concern though — using __sveltekit/store makes a module impossible to unit test. Not sure how best to solve that without replacing uvu with vitest

[Rich-Harris]

Also, the virtual modules are getting out of hand, I'm going to add a dedent utility in a separate PR. We have trim(...) already but i think it can be improved

[Conduitry]

Doesn't import-meta-resolve need to be a prod dependency of Kit? It's used in packages/kit/src/exports/vite/index.js which isn't getting bundled AFAIK.

[itssumitrai]

Hi, this would start breaking our production application. We have a custom wrapper on top of svelte store to make it not accidentally share state on server for different requests, but this change throwing out error is basically going to force us and probably others into looking at other state management libraries instead of using Svelte stores. I think a configurable warning is much better alternative.

[Rich-Harris]

In that case — and since this somewhat complex addition only prevents one specific kind of unwanted side-effect, namely those that happen to use writable instead of any other method of sharing state (including normal variables) — I'm inclined to close this, and rely on the docs to help people avoid shooting themselves in the foot. Any objections?

[dummdidumm]

Sounds good to me 👍

[205g0]

Would it be a good idea to put a dev && console.log('writable <name> created') after every Writable creation in my app? If one of these console.logs pops up in my server log, I'd know something is wrong.

After reading all related threads, I found the hardest part, how to test / detect server-side store updates. There's somehow no feedback loop telling instantly, hey, this is wrong (or is there?). Hence, the aforementioned idea.

Moreover, Svelte stores are one of the best state management systems where you just code complex state without mental drainage, I just wonder if I can keep this flow also with ssr = true.