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

Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
b011f68
feat(template): support for arbitrary replacements
Dec 27, 2019
a87102f
feat(template): allow multiple substitutions
Dec 27, 2019
b745513
Merge branch 'master' of https://github.com/sveltejs/sapper into temp…
Feb 18, 2020
729c885
Merge branch 'master' of https://github.com/sveltejs/sapper into temp…
Apr 11, 2020
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
26 changes: 20 additions & 6 deletions runtime/src/server/middleware/get_page_handler.ts
View file
Open in desktop
Expand Up @@ -320,12 +320,14 @@ export function get_page_handler(
// users can set a CSP nonce using res.locals.nonce
const nonce_attr = (res.locals && res.locals.nonce) ? ` nonce="${res.locals.nonce}"` : '';

const body = template()
.replace('%sapper.base%', () => `<base href="${req.baseUrl}/">`)
.replace('%sapper.scripts%', () => `<script${nonce_attr}>${script}</script>`)
.replace('%sapper.html%', () => html)
.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.

base: () => `<base href="${req.baseUrl}/">`,
scripts: () => `<script${nonce_attr}>${script}</script>`,
html: () => html,
head: () => `<noscript id='sapper-head-start'></noscript>${head}<noscript id='sapper-head-end'></noscript>`,
styles: () => styles
});

res.statusCode = status;
res.end(body);
Expand Down Expand Up @@ -356,6 +358,18 @@ export function get_page_handler(
};
}

function replace(
template: string,
replacers: Record<string, string | CallableFunction>
) {
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

}
}
return template;
}

function read_template(dir = build_dir) {
return fs.readFileSync(`${dir}/template.html`, 'utf-8');
}
Expand Down