Replies: 40 comments 83 replies
-
@babichjacob it's inconsiderate to close and lock an issue without any explanation we're trying to contribute+become part of the svelte community -- so getting auto-silenced feels like an invalidating and unwelcoming start |
Beta Was this translation helpful? Give feedback.
-
You could wrap store usage using Svelte's context API. That way they are scoped to a component and its children, therefore scoped per user: // store.js
import { setContext, getContext } from 'svelte';
export function createMyCustomStore() {
setContext('someUniqueName', writable('some content'));
}
export function getMyCustomStore() {
getContext('someUniqueName');
}
// src/routes/__layout.svelte
<script>
...
createMyCustomStore();
</script>
// src/routes/somewhereelse.svelte
<script>
const store = getMyCustomStore();
...
</script> Note that this means that you need to call these get/set methods on component initialization. |
Beta Was this translation helpful? Give feedback.
-
Thanks for taking the time and chiming in @dummdidumm
We believe the docs are neither clear nor explicit, for these reasons:
At this point, we still feel that svelte stores should house our global state (just like a redux store), and to achieve the desired result on SSR we're using // src/routes/__layout.svelte
<script context="module">
export async function load({ fetch }) {
const res = await fetch("api.blink.xyz/conversations")
const conversations = await res.json()
return {
stuff: { conversations }
}
}
</script>
<script>
import conversationStore from "$lib/stores/conversations"
import { page } from "$app/stores"
conversationStore.update($page.stuff.data)
</script>
// src/routes/somewhereelse.svelte
<script>
import conversationStore from "$lib/stores/conversations"
</script>
<ul>
{#each $conversationStore[1].messages as message}
<li>{message.sender}: {message.content}</li>
{/each}
</ul> |
Beta Was this translation helpful? Give feedback.
-
I was just about to create a post for this same issue. Since Svelte stores are generally written as files where you have to initialize it like |
Beta Was this translation helpful? Give feedback.
-
Since there hasn't been any new activity on this thread, I have been experimenting on this a little bit, and I have found that using So, my solution is basically not have global stores. My Store file looks like: fooStore.js
Notice that we return a function which instantiates the store. This is important to ensure we are not using global stores around on server. Now in __layout.svelte
Now you can use your fooStore instance without any worry from the One caveat: Sveltekit removes |
Beta Was this translation helpful? Give feedback.
-
I kinda agree with this, there should be a JS sandbox being created per request. I understand the overhead, but i think it's critical for security, many devs are probably unaware of this caveat, despite a small note about it in the docs |
Beta Was this translation helpful? Give feedback.
-
Edit: Please see the following proposal |
Beta Was this translation helpful? Give feedback.
-
thanks for pushing this forward @AlbertMarashi |
Beta Was this translation helpful? Give feedback.
-
Eventually we have also come accross this problem. This seems like one of hte biggest issues of Sveltekit. All solutions proposed seems not cool considering the ease-of-use of Sveltekit. What is the disadvantage of restricting the stores on the request context? Additionally, currently tried the below approach (upon populating store on SSR). We get an empty store on SSR. But this also does not help because since it gives an empty store on each server import, it is meaningless. export const hello = writable<string | null>(null)
export function getHelloStore() {
if (!browser) {
return writable<string | null>(null)
} else {
return hello
}
} In the end I am really really confused, as far as I understand either you set a store on @Rich-Harris, what is really interesting is that there have been 2 issues about this problem, 1 is closed and the other (this one) is converted into a discussion. For me and my team this is the number 1 issue currently in Sveltekit. Can somebody from Sveltekit please explain what is the ultimate correct way to use the stores considering the SSR and security? |
Beta Was this translation helpful? Give feedback.
-
We ran into the same issue with our app. Data was leaking between requests, and we also used server-side authentication, so this posed a significant security risk. I ended up writing a custom store that could be used in the same way as Svelte's default // sandboxedStore.js
//
import { browser } from '$app/environment'
import { getStores } from '$app/stores'
import { v4 as uuidv4 } from 'uuid'
import { get, writable as svelteWritable } from 'svelte/store'
const storesKey = `sandbox_${uuidv4()}`
/**
* Creates a façade for a writable store that is a sandboxed store that may be used as a
* global variable. It must be initialized during component initialization.
*
* This store is contextual since it is added to the context of the root component. This means that
* it is unique to each request on the server rather than shared across multiple requests handled
* by the same server at the same time, allowing it to be used as a global variable while also
* holding user-specific data.
*
* We must subscribe to this store during component initialization before we can use it. It is
* utilized in the same way as [SvletKit's app stores](https://kit.svelte.dev/docs/modules#$app-stores).
*
* _NB: In async methods, set the store before any await, otherwise, the context will be lost once
* the promise is fulfilled._
*
* @param {any} initialValue An initial value to set the store to
* @param {string} [key] An optional key name for the store
*/
export function writable(initialValue, key) {
key = key ? `${key}_${uuidv4()}` : uuidv4()
function setStore(value) {
try {
const { page: sessionStore } = getStores()
const session = get(sessionStore)
const store = session?.[storesKey]?.[key]
const currentValue = store ? store.value : initialValue
if (!store || value !== currentValue) {
if (!session[storesKey]) session[storesKey] = {}
session[storesKey][key] = {
value,
subscribers: store?.subscribers || [],
}
// alert subscribers
if (store) {
store.subscribers.forEach(fn => {
fn(value)
})
// return the updated value
return value
}
}
} catch (error) {
// if we reached this exception, it meant that the store had not yet been initialized
return value
}
}
const sandboxedWritable = {
set: setStore,
subscribe: fn => {
try {
const { page: sessionStore } = getStores()
const session = get(sessionStore)
const store = session?.[storesKey]?.[key]
const currentValue = store ? store.value : initialValue
// call the subscription function with the current value
fn(currentValue)
// register the subscriber
if (!session[storesKey]) session[storesKey] = {}
session[storesKey][key] = {
value: store?.value || initialValue,
subscribers: [...(store?.subscribers || []), fn],
}
} catch (error) {
// if we reached this exception, it meant that the store had not yet been initialized
// call the subscription function with the initial value
fn(initialValue)
}
// return the unsubscribe function
return function unsubscribe() {
try {
const { page: sessionStore } = getStores()
const session = get(sessionStore)
// unregister the subscriber
const { subscribers } = session[storesKey][key]
subscribers.splice(subscribers.indexOf(fn), 1)
} catch (error) {
// if we reached this exception, it meant that the store had not yet been initialized
// ignore
}
}
},
update: fn => {
try {
const { page: sessionStore } = getStores()
const session = get(sessionStore)
const store = session?.[storesKey]?.[key]
const currentValue = store ? store?.value : initialValue
setStore(fn(currentValue))
} catch (error) {
// if we reached this exception, it meant that the store had not yet been initialized
setStore(fn(initialValue))
}
},
}
return browser ? svelteWritable(initialValue) : sandboxedWritable
} EDIT: I updated the code, to remove const { page: sessionStore } = getStores()
- const { stuff: session } = get(sessionStore)
+ const session = get(sessionStore) You can use this writable store in the same way as the default one, with the exception that it will not leak data on the server between requests. You can also specify a value to initialize the store with, which will be set only after the context is available: import { writable } from 'sandboxedStore'
const foo = writable()
const bar = writable('initial value') |
Beta Was this translation helpful? Give feedback.
-
I absolutely agree that this behavior is non-obvious and as such should be mentioned and explained much more clearly in the docs. Declaring a store in a separate file and then importing it in components and so on is how you do global stores in vanilla Svelte, and so many people are probably assuming they can do the same thing in SvelteKit, while not actually realizing that it could be problematic. |
Beta Was this translation helpful? Give feedback.
-
I would really prefer in build solution for the store in SSR with sensitive user data. Maybe leaving session store behind (removal of the session store) was too hasty. For the use cases discussed here the old session was really great and easy to use tool/. Even tools like Supabase SvelteKit Auth Helpers were build upon and embraced this feature. |
Beta Was this translation helpful? Give feedback.
-
I have been hitting this issue as I mentioned here #8614 (comment). Is @raduab's solution above still what most people here are using? Has any progress been made else where that I'm missing? Thanks! |
Beta Was this translation helpful? Give feedback.
-
Hi! Thanks for bringing up this info. Imo, this is quite unexpected behavior, and if a developer of an API needs people to read very carefully, then their users are not going to read (hi, user here). (All in my newbie opinion:) In Svelte (i.e. just client-side), stores don't have cross-session behavior (i.e. User1 will never see User2's info). Otherwise, I would personally just turn off SSR if I'm using stores, unless I were using a workaround like those in this discussion. |
Beta Was this translation helpful? Give feedback.
-
Unfortunately, turning off SSR with SvelteKit, and contrarily to what the documentation says, you're loosing all SEO, not some of it. From the experiments I've done, Google was unable/unwilling to fetch more than a few of the dozens of javascript files necessary to render a page, and I wasn't able to find how to have SK generate a unique js bundle either… |
Beta Was this translation helpful? Give feedback.
-
This breaks the Principle of Least Astonishment so much: https://en.wikipedia.org/wiki/Principle_of_least_astonishment I am truly deeply astonished. |
Beta Was this translation helpful? Give feedback.
-
To be honest, I don't think this requires a technical solution. Sticking to good programming practices is sufficient. Devs should just avoid using globals. Really, this has been taught for decades, long before web dev was a thing. If you need per-request stores, put them in context and/or locals and pass them explicitly to the functions that require them. A pure function that does not reply on implicit global state is much less trouble. I know putting stores in global variables has been common practice for many Svelte devs pre SevelteKit. In SPA scenarios people got away with it, but now these practices backfire. That's not an inherit issue with Svelte, just bad practice. So, IMO, this problem is not a technical one. What's needed is better training (don't use globals unless it's for true singletons; write functions as pure as possible). The docs could be more explicit about this, I guess, but it's clearly mentioned. I don't want to sound condescending. I just don't believe this is a bug that needs fixing. |
Beta Was this translation helpful? Give feedback.
-
I'm trying to wrap my head around all of this so hopefully someone can confirm if my understanding is correct. I get if I make a store inside What I'm struggling to wrap my head around is if a store is created using a Eg Skeleton has a warning around this for some of their utilities, as behind the scenes they use stores which work via exporting the store as a const Is this only a danger if it's used within a load function (both |
Beta Was this translation helpful? Give feedback.
-
Could someone tell me if svelte 5 runes address this problem somehow? |
Beta Was this translation helpful? Give feedback.
-
Holy moly why is this not stated in big red font in the docs. |
Beta Was this translation helpful? Give feedback.
-
Glad I stumbled across this. Is this issue only in regards to leaking sensitive data? Or would it also cause issues in another way? I.e. Values being changed or getting the wrong value unintentionally. I'd like to determine if all of my stores usage need updating or just the ones with sensitive data. Thanks! |
Beta Was this translation helpful? Give feedback.
-
Who changed the title from: “Dangerous Store behavior with SSR” to: “Sharing a global variable across multiple requests is unsafe in SSR” When I first created this bug as a github issue, you closed as invalid, and then moved into this discussion. I disagreed with your actions. Now almost two years later, the name is changed in what appears to be more effort at minimizing the significance of the issue. It’s a bad look for sveltekit, which is a shame because there’s so much to like about the framework and the ecosystem. |
Beta Was this translation helpful? Give feedback.
-
Yea, oof |
Beta Was this translation helpful? Give feedback.
-
We have enhanced the docs to more clearly explain why shared state is a problem in SSR and what to do instead: https://kit.svelte.dev/docs/state-management . You may also use clever workarounds like this library which makes it possible to use seemingly global Note that the underlying problem - using global variables on the server - is not a problem related specifically to SvelteKit, this is a general problem/gotcha related to server environments. You could create a Next.js app, use a global variable across different components and run into the same problem. But in the case of Next.js/React you run into this much less because you have to use hooks to manage state in a reactive way which implicitly is scoped to a component tree, thereby avoiding this problem. In Svelte it's more apparent/common because of how easy it is to reuse global state across components and is taught that way in many docs. Historically this wasn't a problem because of the SPA-first world we lived in for so long. With SvelteKit now encouraging to use SSR, this comes back to bite us, as we now have to be more aware of these gotchas. We know that we have to make this more clear in the docs/tutorials and are looking for ways to improve this. |
Beta Was this translation helpful? Give feedback.
-
Especially considering that Svelte and Sveltekit are often used interchangeably. The line is often blurred where Svelte starts and SvelteKit begins - that was kinda the whole point of these frameworks in the first place (imo). |
Beta Was this translation helpful? Give feedback.
-
I'd like to share my perspective as someone who just run into this issue while trying to add some server side rendering to a previously client-only app. I'm keeping most of my shared application state in stores that are exported from js files, from where it is imported in the svelte files that need it. From what I can tell, this pattern is common in Svelte, and even used in the official tutorial. In my opinion there are two issues with the current approach:
I don't know how most of the other frameworks handle this problem, but in Vue for example, the official solution for handling complex shared state (Pinia) is "SSR-safe" by default, for good reason. Svelte stores are of course not the same, but comparable in that they are the framework's recommended solution for shared state. Is there any reason why they should NOT be SSR-safe, aside from some added complexity / impact on performance? I can't come up with any use case that would rely on the current behaviour, whereas obviously there are issues that come with it, as evident by this lengthy discussion. |
Beta Was this translation helpful? Give feedback.
-
Please see the following proposal: Would love to get your feedback and finally solve this important issue |
Beta Was this translation helpful? Give feedback.
-
Huntabyte did a video on this discussion and proposes to leverage a class implementation when using Svelte 5. Here's a link to the video if anyone's interested: Youtube: Global Stores Are Dangerous |
Beta Was this translation helpful? Give feedback.
-
Describe the bug
In adopting Svelte for our chat+communities app, we are trying to utilize the Store for our complex state needs. In this process, we came across semantic differences between stores on the client vs the server that are very concerning -- described in detail here: #2213 Using a Svelte store in the load function causes weird behavior
We are coming from
react+redux
, which also has the concept of a store where you would keep complex global state. In our app, we use it to store the authenticated user'sprofile
and privateconversations+messages
. If we did this in a Svelte Store where it's treated as an in-memory singleton, there's the significant potential to leak personal/sensitive data to others using the app. Furthermore, SvelteKit documentation does a good job of making the case that Stores are where complex state should happen, but makes no mention of the implicit risks and impedance mismatch when SSR is used in conjunction with Stores.What is the recommended approach for handling complex, user-specific (private) global reactive state in SvelteKit? Are there any plans to address the implications of SSR Stores more explicitly in the documentation?
Reproduction
See #2213 for reproduction
Logs
No response
System Info
`@sveltejs/kit 1.0.0-next.282`
Severity
serious, but I can work around it
Additional Information
No response
Beta Was this translation helpful? Give feedback.
All reactions