kit icon indicating copy to clipboard operation
kit copied to clipboard

Setup each page - init hook, `export const blocking`, etc.

Open stalkerg opened this issue 1 year ago • 34 comments

Describe the problem

For i18n or similar subsystems, we should make initialization before any language things are needed, and for such initialization, we usually use a session (user language) and information from the HTTP request. Before the new routing system was introduced, we were using the root __layout.svelte to do it, it's working well for SSR and CSR time. I should mention that what i18n should be inited on the server side and init on the client side as well (or state should be send as is).

After migration to new routing, we lose one "transitional" place to init such thing because +layout.js can be called after +page.js, and you must do await parent() in each page to avoid such race condition.

For server-side only init, we have a good place it's handle in hooks.js with event.locals it's an excellent way to init server only things like DB connection, but for things what should be a transition from SSR to CSR we have no place.

I also have a similar issue for i18n implementation project what I use https://github.com/cibernox/svelte-intl-precompile/issues/55

Describe the proposed solution

Maybe it's possible to do it more straightforwardly, but at least we need a global hook for what will be called on SSR and on CSR, after +layout.server.js but before +layout.js and +page.js.

Alternatives considered

No response

Importance

would make my life easier

Additional Information

No response

stalkerg avatar Aug 23 '22 09:08 stalkerg

If I understand you correctly, it could work today for you, too, but you'd have to do await parent() in each layout/page, which is cumbersome. I had this concern, too. My idea was to introduce export const blocks = <boolean> to +layout.js (and possibly to layout.server.js) so you have to write it in one place only for situations like this.

dummdidumm avatar Aug 23 '22 10:08 dummdidumm

@dummdidumm right, but honestly, I suppose +layout.js it's not a good place, and for me, it just should be before +layout.js and +page.js (doesn't care who first) it's not directly tied with the layout. Also, because we can have a named layout, I don't want to duplicate this logic of how I should do it now.

stalkerg avatar Aug 23 '22 11:08 stalkerg

@dummdidumm right, but honestly, I suppose +layout.js it's not a good place, and for me, it just should be before +layout.js and +page.js (doesn't care who first) it's not directly tied with the layout. Also, because we can have a named layout, I don't want to duplicate this logic of how I should do it now.

This would be solved by the new Layouts proposal as well, since the root layout would always exist.

tcc-sejohnson avatar Aug 23 '22 15:08 tcc-sejohnson

@tcc-sejohnson I see, in that case, it can work, but from a semantic point of view, it's not good. It's not a layout anymore if you understand what I mean.

stalkerg avatar Aug 23 '22 16:08 stalkerg

@tcc-sejohnson I see, in that case, it can work, but from a semantic point of view, it's not good. It's not a layout anymore if you understand what I mean.

We've had this discussion before and pretty much just decided "Layout doesn't mean literally just laying things out on the page. You can lay data out for your app as well." No one's been able to come up with a better term, so I'd just adjust your understanding of the word.

(If you look at the dictionary definition, "the way in which the parts of something are arranged or laid out", it actually does apply to data as well. You're "arranging your app", not just "arranging the visual layout of a page".)

tcc-sejohnson avatar Aug 23 '22 16:08 tcc-sejohnson

You're "arranging your app", not just "arranging the visual layout of a page".)

It's very confusing in the frontend context but I got it. One more comment if you permit: initialization of i18n is not data. Anyway, where can I check this proposal or PR?

stalkerg avatar Aug 23 '22 17:08 stalkerg

@stalkerg

https://github.com/sveltejs/kit/discussions/6124

tcc-sejohnson avatar Aug 23 '22 17:08 tcc-sejohnson

@tcc-sejohnson I do not fully understand how a new proposal can solve this issue because. Root layout exists even now (+layout.js) in my case, but it's run after +page.js when I need access to i18n features.

stalkerg avatar Aug 24 '22 05:08 stalkerg

I'm going to drop here some background on prior art on how other frameworks, in this case Ember.js, approached this problem of having code that needs to run before the continues the boot process and that code may or may not be blocking.

In Ember such code was put in /initializers and /instance-initializers. The functions there ran before the app booted and were typically used to setup critical initial configuration (i18n is an obvious example of setup that should run as early as possible because it will affect how everything else is rendered. Configuring user credentials if there's some kind of API wrapper singleton could be another one). If those functions returned a promise, the app boot's process would block until that promise resolves. If no promise is resolved the boot process continues without delay.

The shade of meaning between initializers and instance-initializers was that initializers ran once in the entire lifetime of the app, even in SSR, while instance-initializers ran once in the browser but once per request in SSR.

Initializers also had names and accepted before/after: otherInitializersName in case there were many and the execution order was important, much like rollup plugins.

I just mention this because IMO Ember got this reasonably right. Sveltekit may decide to come up with a different abstraction, but it's good to know how others approached similar problems in the past.

cibernox avatar Aug 24 '22 16:08 cibernox

export const blocking would work - though it'd also mean that you need an extra route group in most cases. E.g. a layout that just does the i18n setup discussed here in a blocking fashion, then a layout that loads the user in a parallel fashion and displays the layout UI. I'm not sure how annoying this would be in practice, but probably at least some

benmccann avatar Aug 24 '22 16:08 benmccann

Another thing coming up from a discussion: When doing this in the root layout and the setup depends on things that are causing the load function to rerun, the setup code might be rerun when you don't want that. #6294 could also help with that.

dummdidumm avatar Aug 30 '22 14:08 dummdidumm

Having a hook like hooks/initialize.server.js and hooks/initialize.client.js would be extremely useful for injecting code to run "on boot". Many ORMs like TypeORM and MicroORM depend on you being able to run code on startup and those hooks would be a great way of adding this in a non-breaking way.

EDIT: Ruby on Rails even has a whole initializers folder containing many files that run on boot.

sphinxc0re avatar Sep 12 '22 09:09 sphinxc0re

@sphinxc0re

No need for this. Back a while ago, we had #6179. This causes hooks.{js|ts} to be run whenever the app "starts" (this is a bit of an amorphous term when considering serverless stuff, but basically it's guaranteed to be run prior to responding to any requests). This means that any initialization logic can be run in the root of hooks and it'll run on startup.

tcc-sejohnson avatar Sep 12 '22 18:09 tcc-sejohnson

@tcc-sejohnson that is very interesting. Do you know if this is intended behaviour and expected to stay or it just happens to be that way because of an implementation detail?

If that's guaranteed I'll go ahead and update the documentation on my library, but I wouldn't like the rug to be pulled from under me.

cibernox avatar Sep 12 '22 19:09 cibernox

@cibernox No it's intended -- I purposefully made that PR because of the same use-cases as @sphinxc0re. It was created in response to #6147.

tcc-sejohnson avatar Sep 12 '22 19:09 tcc-sejohnson

@tcc-sejohnson then you made my day! Thanks for the heads up!

cibernox avatar Sep 12 '22 19:09 cibernox

So this should be documented then, right?

sphinxc0re avatar Sep 12 '22 19:09 sphinxc0re

So this should be documented then, right?

Hah, that would close my issue from a looong time ago: #1753

tcc-sejohnson avatar Sep 12 '22 21:09 tcc-sejohnson

@tcc-sejohnson thanks! I will try it, at least it sounds good.

stalkerg avatar Sep 14 '22 04:09 stalkerg

@tcc-sejohnson, unfortunately, using hooks.client.js is not working for me because I need access to request (/page/?lang=en ) to get the language GET argument and init i18n. Yes, as you said, here https://github.com/sveltejs/kit/pull/6756#issuecomment-1244533496, it's only for the server. Basically, it's already described in the issue. @cibernox it's not helping us :(

stalkerg avatar Sep 14 '22 08:09 stalkerg

@stalkerg there is a section about i18n in the docs: https://kit.svelte.dev/docs/accessibility#the-lang-attribute 🤷🏼‍♂️ maybe it helps

sphinxc0re avatar Sep 15 '22 07:09 sphinxc0re

Here's what i've been experimenting with today:

lang.ts

import type { Handle, RequestEvent } from '@sveltejs/kit';
import {
	init,
	register,
	waitLocale,
	getLocaleFromAcceptLanguageHeader,
	getLocaleFromNavigator
} from 'svelte-intl-precompile';
register('en', () => import('$locales/en'));
register('de', () => import('$locales/de'));

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?) {
	let locale = event ? getSSRLocale(event) : getClientLocale();
	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.client.ts

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

hooks.server.ts

import { setLocale } from './lang';
export const handle: Handle = sequence(/*auth etc */, setLocale);

Is this a valid approach?

dominikg avatar Sep 16 '22 13:09 dominikg

unfortunate side effect: you have to update your build target to es2022 for support of TLA

dominikg avatar Sep 16 '22 14:09 dominikg

Good solution @dominikg

unfortunate side effect: you have to update your build target to es2022 for support of TLA

If you don't want to use es2022, you could do this

hooks.client.ts

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

in the root +layout.js like this:

import { browser } from '$app/environment';
import { loadLocale } from '../lang';

export const load = async () => {
  if (browser) {
    await loadLocale();
  }
};

This should work.

PatrickG avatar Sep 16 '22 16:09 PatrickG

@PatrickG, as I mentioned in the issue, it will work only if you add await parent() for each page load function.

stalkerg avatar Sep 18 '22 01:09 stalkerg

@dominikg thank you for the solution. But sorry, I disagree with @PatrickG ; it's a very complicated and sphagety-like solution. In the SveltKit we mustn't do it.

stalkerg avatar Sep 18 '22 01:09 stalkerg

@PatrickG, as I mentioned in the issue, it will work only if you add await parent() for each page load function.

I was only talking about the solution @dominikg provided.

Edit: nevermind, I forgot about that

After migration to new routing, we lose one "transitional" place to init such thing because +layout.js can be called after +page.js, and you must do await parent() in each page to avoid such race condition.

PatrickG avatar Sep 18 '22 01:09 PatrickG

@sphinxc0re thanks for the advice, it does not solve this issue, but help to more accurate handle language even now.

stalkerg avatar Sep 18 '22 03:09 stalkerg

My proposal would be an exported init or initialize (can't think of a better name) function in hooks.client.js that gets called (and awaited) right after the init function in https://github.com/sveltejs/kit/blob/177a5a9f8219f3f9633d8f8dc879f8472f74d6a2/packages/kit/src/runtime/client/start.js#L28

Maybe the returned value could be the equivalent of locals in the client-side load functions or merged with data.

PatrickG avatar Sep 18 '22 03:09 PatrickG

@dominikg That is beautiful code. Thanks for providing that ! Going to try that out.

ZetiMente avatar Sep 19 '22 15:09 ZetiMente