Skip to content

Commit

Permalink
fix: ensure single script and scripts in loops execute properly (#13386)
Browse files Browse the repository at this point in the history
- add a comment to lone script tags to ensure there's always a parent so we can synchronously call the replace function
- always call the replace function, not just on the first call

fixes #13378

---------

Co-authored-by: Simon Holthausen <[email protected]>
  • Loading branch information
adiguba and dummdidumm authored Sep 26, 2024
1 parent b73052f commit 9627426
Show file tree
Hide file tree
Showing 4 changed files with 55 additions and 49 deletions.
13 changes: 13 additions & 0 deletions packages/svelte/src/compiler/phases/3-transform/utils.js
Original file line number Diff line number Diff line change
Expand Up @@ -270,6 +270,19 @@ export function clean_nodes(

var first = trimmed[0];

// Special case: Add a comment if this is a lone script tag. This ensures that our run_scripts logic in template.js
// will always be able to call node.replaceWith() on the script tag in order to make it run. If we don't add this
// and would still call node.replaceWith() on the script tag, it would be a no-op because the script tag has no parent.
if (trimmed.length === 1 && first.type === 'RegularElement' && first.name === 'script') {
trimmed.push({
type: 'Comment',
data: '',
parent: first.parent,
start: -1,
end: -1
});
}

return {
hoisted,
trimmed,
Expand Down
62 changes: 13 additions & 49 deletions packages/svelte/src/internal/client/dom/template.js
Original file line number Diff line number Diff line change
Expand Up @@ -72,21 +72,8 @@ export function template(content, flags) {
*/
/*#__NO_SIDE_EFFECTS__*/
export function template_with_script(content, flags) {
var first = true;
var fn = template(content, flags);

return () => {
if (hydrating) return fn();

var node = /** @type {Element | DocumentFragment} */ (fn());

if (first) {
first = false;
run_scripts(node);
}

return node;
};
return () => run_scripts(/** @type {Element | DocumentFragment} */ (fn()));
}

/**
Expand Down Expand Up @@ -151,21 +138,8 @@ export function ns_template(content, flags, ns = 'svg') {
*/
/*#__NO_SIDE_EFFECTS__*/
export function svg_template_with_script(content, flags) {
var first = true;
var fn = ns_template(content, flags);

return () => {
if (hydrating) return fn();

var node = /** @type {Element | DocumentFragment} */ (fn());

if (first) {
first = false;
run_scripts(node);
}

return node;
};
return () => run_scripts(/** @type {Element | DocumentFragment} */ (fn()));
}

/**
Expand All @@ -182,10 +156,11 @@ export function mathml_template(content, flags) {
* Creating a document fragment from HTML that contains script tags will not execute
* the scripts. We need to replace the script tags with new ones so that they are executed.
* @param {Element | DocumentFragment} node
* @returns {Node | Node[]}
*/
function run_scripts(node) {
// scripts were SSR'd, in which case they will run
if (hydrating) return;
if (hydrating) return node;

const is_fragment = node.nodeType === 11;
const scripts =
Expand All @@ -202,28 +177,17 @@ function run_scripts(node) {

clone.textContent = script.textContent;

const replace = () => {
// The script has changed - if it's at the edges, the effect now points at dead nodes
if (is_fragment ? node.firstChild === script : node === script) {
effect.nodes_start = clone;
}
if (is_fragment ? node.lastChild === script : node === script) {
effect.nodes_end = clone;
}

script.replaceWith(clone);
};

// If node === script tag, replaceWith will do nothing because there's no parent yet,
// waiting until that's the case using an effect solves this.
// Don't do it in other circumstances or we could accidentally execute scripts
// in an adjacent @html tag that was instantiated in the meantime.
if (script === node) {
queue_micro_task(replace);
} else {
replace();
// The script has changed - if it's at the edges, the effect now points at dead nodes
if (is_fragment ? node.firstChild === script : node === script) {
effect.nodes_start = clone;
}
if (is_fragment ? node.lastChild === script : node === script) {
effect.nodes_end = clone;
}

script.replaceWith(clone);
}
return node;
}

/**
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
import { test } from '../../assert';

export default test({
mode: ['client'],
async test({ assert, window }) {
// wait the script to load (maybe there a better way)
await new Promise((resolve) => setTimeout(resolve, 1));
assert.htmlEqual(
window.document.body.innerHTML,
`<main><b id="r1">1</b><b id="r2">2</b><b id="r3">3</b></main>`
);
}
});
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
<script>
function jssrc(src) {
return 'data:text/javascript;base64, ' + btoa(src);
}
const scriptSrcs = [ 1, 2, 3 ]
.map(n => jssrc(`document.getElementById('r${n}').innerText = '${n}';`));
</script>

<svelte:head>
{#each scriptSrcs as src}
<script src={src} async defer></script>
{/each}
</svelte:head>

<b id="r1">?</b><b id="r2">?</b><b id="r3">?</b>

0 comments on commit 9627426

Please sign in to comment.