Skip to content
This repository was archived by the owner on Jan 11, 2023. It is now read-only.

Conversation

vilarfg
Copy link

@vilarfg vilarfg commented Dec 27, 2019

Fixes #1036

@DominikGuzei
Copy link

Thanks @vilarfg, works great for SSR rendering of JSS styles into the template 👍

.replace('%sapper.head%', () => `<noscript id='sapper-head-start'></noscript>${head}<noscript id='sapper-head-end'></noscript>`)
.replace('%sapper.styles%', () => styles);
const body = replace(template(), {
...res.replacers,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I suggest moving this below defaults, such that you can override them. Overriding defaults could be a flexible way to solve #1034, #904, #657 and similar issues. For example: including polyfills for certain user-agents, excluding scripts on others, using absolute urls in <base>, etc.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same here as my other comment, actually.

) {
for (const key in replacers) {
if (replacers.hasOwnProperty(key)) {
template = template.replace(new RegExp(`%sapper.${key}%`, 'g'), replacers[key] as any);
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Have replacers less coupled to the %sapper.key% pattern? It would be flexible to let replacers define their own patterns to replace.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I didn't realize that this PR was around when I started my work, but I've done something similar over at #1642, which happens to address your concern here, @arve0

@benmccann
Copy link
Member

Thanks for putting together this PR! Sapper never had a good method of handling configuration, which made it difficult to know how to integrate this feature. However, that's been addressed in SvelteKit by adding a svelte.config.cjs file and adapters. I'm going to go ahead and close this PR since we won't be adding the feature to Sapper. You might like to check out SvelteKit instead. Someone's sent a PR for this feature already actually sveltejs/kit#641

@benmccann benmccann closed this Mar 24, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

ability to replace text on template through middleware

6 participants