Shared singleton store instance on server, handling dynamic user locales on SSR

[OwsOwies]

Hello. I've noticed that this library creates a single $locale store shared in sveltekit server instance.
As per sveltekit documentation

Mutating any shared state on the server will affect all clients, not just the current one.

As I understand, in init as per

<script>
  init({
    fallbackLocale: 'en',
    initialLocale: getLocaleFromNavigator($session.acceptedLanguage)
  });	
</script>

you set current user's request locale in this single store instance.
I don't think this is desired behavior. This could probably cause racing conditions between multiple SSR requests.
Store should be created per request on SSR, probably in the highest level component, or am I missing something?

[cibernox]

Thanks for reporting. I have to investigate this, I was assuming that every request would have its own store instances, but perhaps I'm not entirely familiar of how sveltekit handles requests.

[robertadamsonsmith]

I was concerned about this issue as well, but I think that in practice it isn't a problem. It is probably worth describing in the documentation how best to avoid SSR race conditions.

This is how I understand it - hopefully I've not misunderstood anything.

In sveltekit, the load() that you can implement is asynchronous, which means that multiple loads may be carried out on the server at the same time for SSR. This means that you cannot reliably set the language in your load() function (global stores, which this library uses, are shared between all ongoing SSR requests).

(For this example, I assume that the language is determined from a params value)

import { addMessages, init, t } from 'svelte-intl-precompile'
import en from '$locales/en';
import es from '$locales/es';	
addMessages('en', en);
addMessages('es', es);

<script context="module">
  export async function load({ params, fetch }) {
    const response = await fetch(`/a-file.json`);
    init({ initialLocale: params.lang }); //NOT RELIABLE!
    return {};
  }
</script>

<h1>{$t('title')}</h1>

Once the load() has resolved though, the rest of the code to render the SSR your page is synchronous, and so setting the language in your regular (not context=module) script instead is reliable.

import { addMessages, init, t } from 'svelte-intl-precompile'
import en from '$locales/en';
import es from '$locales/es';	
addMessages('en', en);
addMessages('es', es);

<script context="module">
  export async function load({ params, fetch }) {
    const response = await fetch(`/a-file.json`);
    return {
      props: {
        lang: params.lang
      }
    };
  }
</script>

<script>
  export let lang;  
  init({ initialLocale: lang }); //RELIABLE!
</script>

<h1>{$t('title')}</h1>

If you want to be able to dynamically import the language, which is necessarily an asynchronous operation, then you have to load the language from the load() function, and then set it again from the normal script block. It isn't as neat, but it works:

import { waitLocale, init, t } from 'svelte-intl-precompile'
register('en', () => import('$locales/en'));
register('es', () => import('$locales/es'));

<script context="module">
  export async function load({ params, fetch }) {
    const response = await fetch(`/a-file.json`);
    init({initialLocale: params.lang})
    await waitLocale(); //wait to make sure that the language we need has been loaded
    return {
      props: {
        lang: params.lang
      }
    };
  }
</script>

<script>
  export let lang;  
  init({ initialLocale: lang }); //make it the current language now
</script>

<h1>{$t('title')}</h1>

As something of an aside to all this, it would be somewhat neater if init() set a context store which the format() function picked up on to use the correct language.

This would mean that global stores weren't needed, trying to set the language from context="module" script block would fail (instead of being allowed, and subject to the SSR race condition issue), and more than one language could be used on the same page in a predictable way. It is probably simple enough to implement that on top of this library though, since quite a lot of the externals are exported.

[cibernox]

Thanks for this deep dive, it makes sense.

I am of the general opinion that for as much as it is possible, libraries should not give users the opportunity of shooting themselves in the foot in such a sneaky way as a race condition like this one. I think we should try to fix it.
One reason I kept the store approach is because it's the one used in ember-i18n, from which I copied 95% of the public API,
both because I liked it and because it made switching to this library a 20 minutes task.

My understanding is that setting the locale using setContext means we don't need helpers like $t or $date to be stores anymore (assuming we also use setContext for storing the dictionary of keys/translations). If we keep those functions as stores, would updating the context be reactive? Right now changing the selected locale updates every bit of text throughout the app.

[robertadamsonsmith]

I like that the format function works as a store, because it means I can change the language and everything updates reactively. I want to be able to limit the scope of that reactivity though, so that changing the language in once place will only effect a specific component and its descendants.

Changes to setContext are not inherently reactive, so where you want reactivity you typically use setContext to pass a store to descendants.

For a project I'm working on, I've written a small wrapper which uses context to do the sort of thing I need it to do, but I need to think through it a bit more to see if it is appropriate for more general usage.

For your top level component (or in any component that you want to have a different language), instead of this:

import { init, _, locale } from "svelte-intl-precompile";

init({
	initialLocale: 'es',
	fallbackLocale: 'en'
})

You need to have something like this (using my modified library, so the naming is a bit different):

import { setLocale } from "svelte-intl-precompile-custom";

const { _, locale } = setLocale({
	initial: 'es',
	fallback: 'en'
})

And then in each descendant component instead of this:

import { _, locale } from "svelte-intl-precompile";

You need to do something like this:

import { getLocale } from "svelte-intl-precompile-custom";

const { _, locale } = getLocale();

I've also modified waitLocale() to require a locale name, so that I'm not reliant on global stores/state, and I use the same options structure for waitLocale as I do for setLocale, to reduce redundancy when you need to do waitLocale(opts) in a sveltekit load() function, and then a setLocale(opts) in your script.

The code to make this work is pretty trivial, but I need to use it a bit more to see if there are any gotchas.

It's also not clear to me if modifying the usage pattern of the library is something generally wanted, or if it can/should harmonise better with the existing api - it would be a shame to break with the ember-i18n API without a sufficiently good reason.

[OwsOwies]

Once the load() has resolved though, the rest of the code to render the SSR your page is synchronous, and so setting the language in your regular (not context=module) script instead is reliable.

Well, I wouldn't depend on it. It could be easily broken the more mature svelte becomes.

My understanding is that setting the locale using setContext means we don't need helpers like $t or $date to be stores anymore (assuming we also use setContext for storing the dictionary of keys/translations). If we keep those functions as stores, would updating the context be reactive? Right now changing the selected locale updates every bit of text throughout the app.

As @robertadamsonsmith said, it is better to keep them as stores because it let's you switch locale in runtime and reactively update the view.

As something of an aside to all this, it would be somewhat neater if init() set a context store which the format() function picked up on to use the correct language.

It's also not clear to me if modifying the usage pattern of the library is something generally wanted, or if it can/should harmonise better with the existing api - it would be a shame to break with the ember-i18n API without a sufficiently good reason.

I have not inspected implementation thoroughly but as this library is stricte svelte wouldn't it be possible to include getContext call inside implementation, just as you mentioned?
Everything would stay as is and calls like this one:
$t('some.key')
would call getContext('localeStore') under the hood. To go even further you could check if context locale store is available and fallback to default one (aka global one, like current implementation one) if it is not existing or if you call outside .svelte file. Not allowing to initialize in context="module" as you suggested is also an option but there is potential issue if you would like to use library outside .svelte file for example create simple derived([locale]) store.

[robertadamsonsmith]

Well, I wouldn't depend on it. It could be easily broken the more mature svelte becomes.

If svelte changed the way that SSR worked so that the rendering step wasn't fully synchronous, I think it would break a lot of things. Anyway, I do think that it is best to avoid global stores for anything that requires SSR, so I wouldn't like to depend on it either.

I have not inspected implementation thoroughly but as this library is stricte svelte wouldn't it be possible to include getContext call inside implementation, just as you mentioned?

Unfortunately, getContext is a lifecycle function, and can only be called during component initialisation.

I'm currently using a bit of wrapper/initialisation code that looks like this:

//lang.js
import {
	waitLocale,
	addMessages,
	register as register2,
	formatMessage,
} from "svelte-intl-precompile";
import { getContext, setContext, hasContext } from "svelte";
import { writable, derived } from "svelte/store";

const key = Symbol();

export async function register(lang, dictionary) {
	if (dictionary instanceof Function) {
		register2(lang, dictionary);
	} else {
		addMessages(lang, dictionary);
	}
}

export async function wait(opts) {
	const { lang, fallback = null } = opts;
	await waitLocale(lang);
	if (fallback) await waitLocale(fallback);
	return opts;
}

function createContext(lang, opts) {
	let locale = writable(lang);
	let _ = derived(
		locale,
		(locale) => (val, opts2) =>
			formatMessage(val, { ...opts, locale, ...opts2 })
	);
	return { locale, _ };
}

export function create(lang, opts = {}) {
	let context = createContext(lang, opts);
	setContext(key, context);
	return context;
}

export function get() {
	return getContext(key);
}

And I use it like this in my top-level component (I could also do this from more than one component, if I want, say, the left side of the app to be in one language, and the right side in another):

<script>
//app.svelte
import { create, register, _ } from "./lang";
import en from "../locales/en";
import he from "../locales/he";

//register locales
register("en", en);
register("he", he);

//create locale context
const { locale } = create('en');
</script>

And I use it in a descendant component like this:

<script>
//child
import { get } from "./lang";
const { _, locale } = get();
</script>

<h2>{$_("test")}</h2>

<button on:click={()=>$locale = "he"} >Change Language</button>

With doing things this way, I can have different locale contexts in different parts of my app (I just call create() from more than one location). There are still some globally stored settings that would ideally be per context instead, but it does what I need it to do for now. I expect I'll keep adapting it to my needs for now.

Now, I did want to see if I could avoid needing to call get(), to make using it a bit cleaner, and it does sort of work if I add this function to my lang.js:

export const _ = {
	subscribe: (...args) => {
		const { _ } = get();
		return _.subscribe(...args);
	},
};

And then I can just do this:

<script>
import { _ } from "./lang.js";
</script>
<h2>{$_("test")}</h2>

That works, because auto subscription is done during component initialisation, and so it can safely call getContext in the wrapped store subscription function. I don't like it though, because I don't think there are any guarantees in which order store subscriptions occur in relation to other initialisation code, and so it can break in some situations (such as trying to use it in the same component from which you created the context with create(), or when you don't want to use auto-subscription for some reason ).

[OwsOwies]

Well, it seems yours is the only possible solution then. For the time being at least.
I think it's ok, calling const { _, locale } = get(); is not that big of a deal. It is also kinda verbose in a good way, it explicitly says that _ is somewhat bound to current context instead of global meaning of import { t } from 'svelte-intl-precompile.

That works, because auto subscription is done during component initialisation, and so it can safely call getContext in the wrapped store subscription function. I don't like it though, because I don't think there are any guarantees in which order store subscriptions occur in relation to other initialisation code

I 100% agree with this.

[steffenstolze]

Since this is still open and almost 3 months old I assume that it's not safe to use it like this with SvelteKit and Vercel?

As far as I understood this could result in race conditions where people get a different locale than they've selected?

//__layout.svelte
<script context="module">
	import { addMessages, init, getLocaleFromPathname } from 'svelte-intl-precompile';
	import de from '../locales/de.json';
	import en from '../locales/en.json';
	addMessages('de', de);
	addMessages('en', en);
	init({
		fallbackLocale: 'de',
		initialLocale: getLocaleFromPathname(/^\/(.*?)\//) || 'de' //RegEx to detect "/en/" or "/de/" in URL
	});
</script>
...

[cibernox]

The race condition seems possible in theory but I haven't been able to reproduce it. If this repo is affected by this, i believe the svelte-i18n should too as our approach to setting the current locale is the same, and I don't think anyone has reported race conditions.

I have the hunch that while possible in theory, the single threaded nature of javascript makes it not happen in practice.

[OwsOwies]

I don't agree with your reasoning here. No one reported race-conditions on some different repository, how is that an argument.

Here is an official discussion on svelte kit where someone mentions data leaking:
sveltejs/kit#4339 (comment)
Still, it's anecdotal, just like you saying that you don't think anyone has reported race conditions. It might be possible, it might be not.
Please note that it might be hard to determine that such issue even exists in your production application if there is no one reporting such a mismatch, if you don't have dedicated analytics for this or you don't encounter it yourself. That is the nature of such issues.
I don't have any evidence to confirm the issue exists or not as my app is not in a production yet and I can't perform any experiment myself. However I do think that if svelte documentation mentions explicitly not to keep shared state like this evidence is not really necessary, it's just something you should not do period. If you do you might potentially encounter issues, which is the case here.

Here is official svelte kit paragraph saying explicitly that those stores should not be global:

In many server environments, a single instance of your app will serve multiple users. For that reason, per-request state must not be stored in shared variables outside your load functions.

https://kit.svelte.dev/docs/load#shared-state

Here is similar issue to this one on the library you mentioned:
kaisermann/svelte-i18n#165
As per this issue, the owner admits that he doesn't really know if the library is compatibile with sveltekit. It's still open:
kaisermann/svelte-i18n#166

If the goal of this library is to be compatible with svelte kit, this issue should be resolved.

[jacob-8]

The race condition seems possible in theory but I haven't been able to reproduce it.

The race condition is a very real problem. I've used svelte-i18n on a tool that's available in 11 languages. As a dev, the race condition is hard to hit and it would hit me about once every 4 months causing me to scratch my head and not really understand. It was particularly noticeable with a Hebrew<->English race condition as the page flips from RTL to LTR on client hydrate. But our user base is growing and I'm sure it's becoming more common as we have had a few isolated reports pop up from users. Now I'm rolling out component screenshot testing to my components (just 5 languages to get rolling but soon all 11) which fires off a lot of different language requests at the same or almost the same time. It makes the screenshot regression results very noisy as I may have 2 actual changes, but 40 false changes due to this language race condition alone.

[cibernox]

Thank you to provide real world experience. I need to figure out a way of ensuring that the current locale stays consistent for the entirety of the lifetime of a request.
maybe storing that info on the request object itself could be the solution but I have to investigate how to access it.

[jacob-8]

I need to figure out a way of ensuring that the current locale stays consistent for the entirety of the lifetime of a request.

@cibernox, I decided to just write a very minimal, framework-agnostic i18n solution to solve this race condition problem. I don't use much formatting stuff, 99% is just string translation so after digging in I'm quite happy with my very lightweight solution that also gives me type-safety for free without a build process. Read through my set up instructions and especially note the page on using with SvelteKit. If you can update your library to work the same way as I'm doing by getting a translate function per request in my +layout.ts file:

export const load: LayoutLoad = async ({ params: { locale }) => {
  const t = await getTranslator({ locale })
  return { locale, t }
}

...then I think you'll be able to easily provide a solution for use in SvelteKit that avoids race conditions.