Support overwriting the default template loader via a handle hook #2773

atuttle · 2021-11-10T21:13:59Z

Here's a brief video explaining why I can't accomplish the same thing without this change. (Apologies for the dog snoring in the background! 🐶 💤 )

Basically, our app is multi-tenant and we allow our customers to provide their own skins for different sections. But most importantly: the skin content is not within our control. All customers run on the same cluster of servers, and at runtime we inspect the hostname/url and look up the appropriate skin to wrap around the app.

As discussed in #2762, I tried to make this work with {@html before}<slot />{@html after} in a layout, and that works as long as the skin doesn't have ANY tags wrapping the app content (must be a direct descendant of <body>). Since I'm splitting the skin content on a token, any wrapping tags become unclosed, and then {@html ...} goes haywire.

This change allows me to overwrite the default template({ head, body}) method called during SSR by specifying a custom per-request template method in a handle() hook:

// src/hooks.js
import loadSkin from '...';
export async function handle({ request, resolve }) {
	const skinHTML = await loadSkin(request.host, request.path);
	request.locals.template = ({ head, body }) => {
		return skinHTML
			.replace('</head>', `${head}</head>`)
			.replace('{{app}}', `<div id="svelte">${body}</div>`);
	};

	return resolve(request);
}

Before submitting the PR, please make sure you do the following

It's really useful if your PR references an issue where it is discussed ahead of time. In many cases, features are absent for a reason. For large changes, please create an RFC: https://github.com/sveltejs/rfcs
This message body should clearly illustrate what problems it solves.
Ideally, include a test that fails without this PR but passes with it.

I'm not sure how I could write a test for this functionality, but if someone wants to offer some advice, I can give it a try.

Tests

Run the tests with pnpm test and lint the project with pnpm lint and pnpm check

I also am not able to get the tests to pass locally; both before and after my change they fail at the same spot:

Changesets

If your PR makes a change that should be noted in one or more packages' changelogs, generate a changeset by running pnpx changeset and following the prompts. All changesets should be patch until SvelteKit 1.0

…sveltejs#2762

changeset-bot · 2021-11-10T21:14:02Z

🦋 Changeset detected

Latest commit: 7971df6

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 1 package

Name	Type
@sveltejs/kit	Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

rmunn · 2021-11-11T15:21:46Z

Not being part of the core team, I can't approve PRs. But I can give you some feedback. I suspect that request.locals is not going to be the right place to put this, as Svelte-Kit tried very hard not to touch the locals object or require any particular properties to be in it. Otherwise this feature will interfere with someone's code at some point, where they want to set a template property in their request.locals for some reason of their own, and suddenly Svelte-Kit has reserved a property name in the object that is supposed to be 100% for user data.

Now, using something other than request.locals will mean that you'll have to change the data type of the request in some way, or perhaps add an optional second parameter to the resolve function passed to handle (where that second parameter would be your ({head, body}) => "template" function). So obviously it's tempting to use request.locals. But I'm pretty sure the core team won't approve a PR that makes Svelte-Kit rely on request.locals having any specific structure.

atuttle · 2021-11-11T21:02:48Z

Appreciate the feedback @rmunn. I'm happy to make whatever the necessary changes are, once I'm given some ("blessed") direction.

benmccann · 2021-11-11T23:48:05Z

Not being part of the core team, I can't approve PRs. But I can give you some feedback

@rmunn thanks so much for all your contributions! We'd love to keep you involved. Are you on Discord by chance? I sent you an email, but just wanted to mention it here as well to make sure I had the right address 😄

Rich-Harris · 2021-11-12T12:56:48Z

This is a fun problem to solve. I'm wondering if you could do this sort of thing...

<!-- src/app.html -->
<!DOCTYPE html>
<html lang="en">
  <head>
    <meta charset="utf-8" />
    <link rel="icon" href="/favicon.png" />
    <meta name="viewport" content="width=device-width, initial-scale=1" />

    %svelte.head%
  </head>
  <body>
+    <!-- BEFORE -->
    <div id="svelte">%svelte.body%</div>
+    <!-- AFTER -->
  </body>
</html>

...and then, in src/hooks.js...

export function handle({ request, resolve }) {
  const response = await resolve(request);

  if (response.headers['content-type'] === 'text/html') {
    const skin = await loadSkin(request.host, request.path);
    const [before, after] = skin.split('{{app}}');
    response.body = response.body
      .replace('<!-- BEFORE -->', before)
      .replace('<!-- AFTER -->', after || '');
  }

  return response;
}

...and have users provide a skin that only includes <body> rather than <head> content?

(One minor detail that shouldn't affect anything, but is worth mentioning just in case — I'd like to remove the id="svelte" stuff in the near future: #2221)

@rmunn is right that request.locals is a non-starter, unfortunately — we'd basically need to change the resolve API to allow additional arguments — which is why I'm wondering if this solution would get you where you need to go.

atuttle · 2021-11-12T14:04:17Z

@Rich-Harris excellent thinking! And that may not even require any SK mods. I'll give it a go and get back to you. (I was looking forward to having been a contributor tho! 😞 Guess I'll have to find something else I can help out with...)

Congrats on the new job, by the by.

atuttle · 2021-11-12T15:52:43Z

As expected, that worked fantastically. I guess I have to withdraw this PR, then.

benmccann · 2021-11-12T17:58:19Z

I'll go ahead and close this then since it sounds like you're no longer pursuing it

7antra · 2022-04-17T14:26:31Z

Hey team !

Sorry, I don't want to reopen this, but how far is difficult to have a dynamic template ? for exemple if you have one template for main app : app.html and an other for newsletter for exemple newsletter.html with tags %svelte.head% and %svelte.body% inside.

Is it very difficult for load the one we want depending URL.pathname ?

(Sorry for my English btw)

atuttle · 2022-09-26T19:06:22Z

@7antra I documented the exact steps I ended up taking, which sounds like it can do what you want: https://adamtuttle.codes/blog/2021/sveltekit-customizing-app-html-at-runtime/

atuttle added 2 commits November 10, 2021 14:41

Support overwriting the default template loader via a handle hook. ref …

29b45bb

…sveltejs#2762

add changeset

7971df6

atuttle mentioned this pull request Nov 11, 2021

Support dynamic content of app.html #2762

Closed

benmccann closed this Nov 12, 2021

atuttle deleted the feature/dynamic-template-loader branch November 17, 2021 14:22

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Support overwriting the default template loader via a handle hook #2773

Support overwriting the default template loader via a handle hook #2773

atuttle commented Nov 10, 2021 •

edited

Loading

changeset-bot bot commented Nov 10, 2021

rmunn commented Nov 11, 2021

atuttle commented Nov 11, 2021

benmccann commented Nov 11, 2021

Rich-Harris commented Nov 12, 2021

atuttle commented Nov 12, 2021

atuttle commented Nov 12, 2021

benmccann commented Nov 12, 2021

7antra commented Apr 17, 2022

atuttle commented Sep 26, 2022

Support overwriting the default template loader via a handle hook #2773

Support overwriting the default template loader via a handle hook #2773

Conversation

atuttle commented Nov 10, 2021 • edited Loading

Before submitting the PR, please make sure you do the following

Tests

Changesets

changeset-bot bot commented Nov 10, 2021

🦋 Changeset detected

rmunn commented Nov 11, 2021

atuttle commented Nov 11, 2021

benmccann commented Nov 11, 2021

Rich-Harris commented Nov 12, 2021

atuttle commented Nov 12, 2021

atuttle commented Nov 12, 2021

benmccann commented Nov 12, 2021

7antra commented Apr 17, 2022

atuttle commented Sep 26, 2022

atuttle commented Nov 10, 2021 •

edited

Loading