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

feat(template): support for arbitrary replacements #1037

Closed
wants to merge 4 commits into from
Closed

feat(template): support for arbitrary replacements #1037

wants to merge 4 commits into from

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