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

Concurrent requests can cause LL store to change locale mid-flight during SSR (SvelteKit, adapter-node) #159

Closed
aloker opened this issue Jan 26, 2022 · 5 comments · Fixed by #162
Labels
bug Something isn't working

Comments

@aloker
Copy link

aloker commented Jan 26, 2022

Version

2.60.1

Describe the bug

In SvelteKit with adapter-node, currently typesafe-i18n creates only a single LL store in memory that is used for all requests. Multiple concurrent requests may change the current locale of that store. This can cause locales to switch within one requests if another request with another locale runs concurrently, leading to completely or partially wrong translations.

Reproduction

The code I used is based on the sveltekit example, including the hook, the __layout.svelte and the [lang] dir.

Consider this load function of a route:

let nextRequestId = 1;

export const load: Load = async () => {
  const requestId = nextRequestId++;
  const sample = () => get(LL).hello();

  console.log(`Request ${requestId}, before API call:`, sample());

  await someExpensiveRequest();

  console.log(`Request ${requestId}, after API call :`, sample());

  return {
    props: {
      val: sample()
    }
  };
};

Let's say we have to visistors, one at http://localhost:3000/en, the other at http://localhost:3000/de

When request come in sequentially, the server side output would be something like:

Request 1, before API call: Hello in English
Request 1, after API call : Hello in English
Request 2, before API call: Hallo in Deutsch
Request 2, after API call : Hallo in Deutsch

But if http://localhost:3000/de is visisted before "someExpensiveRequest" resolves, we'd see this:

Request 3, before API call: Hello in English
Request 4, before API call: Hallo in Deutsch
Request 3, after API call : Hallo in Deutsch
Request 4, after API call : Hallo in Deutsch

That is, request 4 changes the locale and that's the locale that is then used by the older request 3 (which started in English).

Logs

See above

Config

{
   "adapter": "svelte",
   "$schema": "https://unpkg.com/typesafe-i18n@2.60.1/schema/typesafe-i18n.json"
}

Additional information

The issue is obviously caused by the fact that there's a single store in memory that can be changed concurrently by multiple in-flight requests. It would probably an idea to have one store in memory for each locale and somehow "attach" the correct one to each request.

I've done something in the past using i18next where the correct store is selected in the root __layout and then passed to the components via context (https://svelte.dev/docs#run-time-svelte-setcontext) and to other load functions via "stuff" (https://kit.svelte.dev/docs#loading-output-stuff). It wasn't very beautiful tbh, but it worked. Maybe you can come up with something more elegant.

@aloker aloker added the bug Something isn't working label Jan 26, 2022
@ivanhofer
Copy link
Owner

Oh, right 🙈 I actually thought that SvelteKit handles concurrent users.
This is something I need to check in detail. Thanks for pointing this out.

@ivanhofer
Copy link
Owner

@aloker I now have tried a few things and the following solution works for me.
It's in a WIP state so it's not so elegant right now:

  1. Replace the contents of src/i18n/i18n.svelte.ts with the following code:
import { getI18nSvelteStore } from 'typesafe-i18n/adapters/adapter-svelte';
import type { Locales, Translation, TranslationFunctions, Formatters } from './i18n-types'
import { baseLocale, getTranslationForLocale } from './i18n-util'
import { initFormatters } from './formatters'
import { derived, get, writable } from 'svelte/store';

const { initI18n: init, setLocale, isLoadingLocale, locale: _locale, LL: _LL } = getI18nSvelteStore<Locales, Translation, TranslationFunctions, Formatters>()

const cache = {} as Record<Locales, { LL: TranslationFunctions, locale: Locales }>

const initI18n = async (locale: Locales = baseLocale) => {
   await init(locale, getTranslationForLocale, initFormatters)
   cache[locale] = { LL: get(_LL), locale: get(_locale) }
}

export const restoreI18n = (l: Locales) => {
   const x = cache[l]
   __LL.set(x.LL)
   __locale.set(x.locale)
}

const __LL = writable(get(_LL))
_LL.subscribe(v => __LL.set(v))

const __locale = writable(get(_locale))
_locale.subscribe(v => __locale.set(v))

const LL = derived([__LL], (v) => v)
const locale = derived([__locale], (v) => v)

export { initI18n, setLocale, isLoadingLocale, locale, LL }

export default LL
  1. Temporarely set the generateonlytypes option to true, so the generator doesn't override your modified i18n-svelte.ts file

  2. Then call fixI18n inside the "normal" script-tag (not in context="module") of your root __layout component.

This should block the correct store until everthing is rendered for that request.

If you are accessing translations inside the load function, you would need to make sure that you also call fixI18n before accessing the translations. Or to make your life easier, make a get(LL) inside the root __layout page and pass it into stuff and use the LL from the stuff inside load functions.

Let me know if this works for you or you need some help.

@ivanhofer
Copy link
Owner

I will refactor how typesafe-i18n handles loading of locales in the next major release.
It was always a bit of a pain-point that loading the locale and using the translations is so deeply coupled.
By splitting both parts, we can get rid of some limitations and will have a more elegant solution to the problem described by you.
This would also make the implementation of #113 a easier.

@ivanhofer
Copy link
Owner

@aloker I now have refactored how typesafe-i18n loads locales.

You can check the latest commit I have made to the svelte-kit example to see what has changed.
Most of the changes are beeing made when installing version 3 and running the generator.
You then just would need to manually change these files:

Please visit the release post for more information.

@aloker
Copy link
Author

aloker commented Jan 29, 2022

Thanks for your fast response! I didn't have the opportunity to test it, but I'll check it out ASAP. Keep up the great work!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants