Skip to content
This repository was 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
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
26 changes: 20 additions & 6 deletions runtime/src/server/middleware/get_page_handler.ts
Original file line number Diff line number Diff line change
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