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

Add support a new SvelteKit routing system #55

Open
stalkerg opened this issue Aug 23, 2022 · 18 comments
Open

Add support a new SvelteKit routing system #55

stalkerg opened this issue Aug 23, 2022 · 18 comments

Comments

@stalkerg
Copy link

We have no load function in layout.svelte anymore, and an alternative like +layout.js is not working automatically because SvelteKit can call it after +page.js.
We can still use it if we will call directly await parent(); in each +page.js but looks like a hack, add extra boilerplate and reduce performance.

I suppose we should find a better place to init i18n and it shouldn't depend on the order of +layout.js and +page.js.

@cibernox
Copy link
Owner

cibernox commented Aug 23, 2022

Interesting. I was going to migrate the documentation page to the new routing API. I didn't know they reversed the order of execution from being top -> down to going from the pages to the layouts.

tl;dr; the code should be run ASAP in the app's boot process, before any locale-dependent code runs.

@stalkerg
Copy link
Author

stalkerg commented Aug 23, 2022

tl;dr; the code should be run ASAP in the app's boot process, before any locale-dependent code runs.

yep, I thought also about lazy singleton, but I still have only +layout.js to do it because I get user session with locale name from +layout.server.js (maybe it's possible in hook.js as before, but it's tricky)
sveltejs/kit#5883 here is more context about the session.
This sveltejs/kit#5883 (reply in thread) is the main issue for us.

@stalkerg
Copy link
Author

@cibernox if you have time you can provide more context to sveltejs/kit#6183

@benmccann
Copy link
Contributor

I didn't know they reversed the order of execution from being top -> down to going from the pages to the layouts.

Just FYI, the order was not reversed, but the load functions now run in parallel by default - unless you call await parent() in which case it awaits the result of the parent load function.

@cibernox
Copy link
Owner

cibernox commented Aug 24, 2022

@benmccann that makes more sense.

tl;dr; Is there in sveltekit an idiomatic place to put code that you want to run before any other code in the app? Setting up global configuration (such as the user's locale, which is often critical to render anything else on the page) is a good example.
In Ember (sorry, long time Ember developer here) there was the concept of initializers and instance-initializers, where you could put code that must run before the app is ready to boot.

@benmccann
Copy link
Contributor

Not at the moment. Tracked in sveltejs/kit#6183

@stalkerg
Copy link
Author

stalkerg commented Oct 2, 2022

@cibernox I found a much more worse issue - on the server side, t and locale shared between ALL requests, and we have race conditions during setup/set layout.

For the browser, it's working fine because we load locale individually for each browser and once, but on the server side, we shared such states between sessions.

@cibernox
Copy link
Owner

I wish there was some wait of setting up a reproduction that allows to consistently reproduce this locally so it can be fixed.

@stalkerg
Copy link
Author

stalkerg commented Dec 26, 2022

Okay, I can reproduce race conditions constantly on SSR.
Basically, all global storage is shared between all sessions, and because between running load functions and rendering template, we have async invoke, we constantly mixed languages between sessions. :(

Basically, between load in +layout.js and .svelte render can be any amount of load requests. (and we have at least from +page.js)

@cibernox
Copy link
Owner

As bad as it sounds, it’s good that you can reproduce it consistently. I got the current approach from svelte-i18n but I’ll have to deviate from it to make sure ever request gets it’s own t and other stores with no shared state.

@stalkerg
Copy link
Author

Yes, now I can play with it but unfortunately, I can't find a good way to push down any local (per request) params into .svelte except load in +page.js file. It seems like we should init i18n in +layout.server.js, and manually push down through +layout.js and +page.js.
We have event.local but it helps only to notify handler about the current language, probably we can use $page to share the request local language function.

@stalkerg
Copy link
Author

stalkerg commented Dec 26, 2022

seems like I can get, a stable per-request locale name in the template by $page.data, is it ok to use formatMessage function directly to avoid the global await locale.set(myLocale)?

UPDATE:

  1. Passing any functions into $page.data is not possible because it should be serialized.
  2. Using directly formatMessage works, but I am not sure about async load localization and etc.

@stalkerg
Copy link
Author

Okay, it seems like I solved it, but not so beautiful.
Now I have my own

export const _ = derived(
  [locale],
  () => (id, options = {}) => formatMessage(
    browser ? get(locale) : get(page).data.localeName,
    id,
    options,
  ),
);

for the browser, I am using locale from our locale storage and for SSR I am getting such value from $page.data (I put it here in +layout.js).

I have no dynamic dictionary changes that mean I shouldn't be derived from. Also, I never do await locale.set(name) on the server side now, and preload all locales at once for SSR.

@multipliedtwice
Copy link

@stalkerg could you please provide a little more context on how to implement it?

@pasqui23
Copy link

pasqui23 commented Jan 8, 2023

I've solved it by using hooks.server.ts

// lang.ts
import type { Handle, RequestEvent } from '@sveltejs/kit';
import {
  init,
  waitLocale,
  getLocaleFromAcceptLanguageHeader,
  getLocaleFromNavigator
} from 'svelte-intl-precompile';
import { registerAll, availableLocales } from '$locales';

registerAll();



const DEFAULT_LOCALE = 'en';

// add this hook to your hooks.server.ts sequence
// and update app.html to use `<html lang="%lang%">`
export const setLocale: Handle = async ({ event, resolve }) => {
  const lang = await loadLocale(event);
  return resolve(event, {
    transformPageChunk: ({ html }) => html.replace('%lang%', lang)
  });
};

// call this function with await in hooks.client.ts

export async function loadLocale(event: RequestEvent | undefined) {
  const locale = event ? getSSRLocale(event) : getClientLocale();
  console.log(locale);
  init({ initialLocale:locale, fallbackLocale: DEFAULT_LOCALE });
  await waitLocale(locale);
  return locale;
}

function getSSRLocale(event: RequestEvent) {
  // prefer stored user locale, fall back to accept header and default
  return (
    event.locals.user?.locale ||
    getLocaleFromAcceptLanguageHeader(event.request.headers.get('Accept-Language')) ||
    DEFAULT_LOCALE
  );
}

function getClientLocale() {
  // html lang attr is set by SSR hook, so we just reuse that
  // otherwise fall back to navigator or default
  return document?.documentElement.lang || getLocaleFromNavigator() || DEFAULT_LOCALE;
}

// hooks.server.ts
import type { Handle } from '@sveltejs/kit';
import { sequence } from '@sveltejs/kit/hooks';
import { setLocale } from './lang';
export const handle: Handle = sequence(setLocale);

// hooks.client.ts
import { loadLocale } from './lang';
await loadLocale();

Bonus ponts,it is even compatible with prerendering.

I'm not 100% sure that hooks.client.ts is needed however.

@pasqui23
Copy link

pasqui23 commented Jan 8, 2023

In general hooks are always ran before any layout and page code and they were made so exactly to support this use case.

@yadoga
Copy link

yadoga commented Jul 2, 2023

@stalkerg could you please provide a little more context on how to implement it?

seconding @stalkerg

@stalkerg
Copy link
Author

stalkerg commented Aug 4, 2023

@yadoga, what exactly do you want to see? I will try to check how it's now works and put it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

6 participants