Skip to content

Commit 9627426

Browse files
adigubadummdidumm
andauthored
fix: ensure single script and scripts in loops execute properly (#13386)
- 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]>
1 parent b73052f commit 9627426

File tree

4 files changed

+55
-49
lines changed

4 files changed

+55
-49
lines changed

packages/svelte/src/compiler/phases/3-transform/utils.js

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -270,6 +270,19 @@ export function clean_nodes(
270270

271271
var first = trimmed[0];
272272

273+
// Special case: Add a comment if this is a lone script tag. This ensures that our run_scripts logic in template.js
274+
// will always be able to call node.replaceWith() on the script tag in order to make it run. If we don't add this
275+
// and would still call node.replaceWith() on the script tag, it would be a no-op because the script tag has no parent.
276+
if (trimmed.length === 1 && first.type === 'RegularElement' && first.name === 'script') {
277+
trimmed.push({
278+
type: 'Comment',
279+
data: '',
280+
parent: first.parent,
281+
start: -1,
282+
end: -1
283+
});
284+
}
285+
273286
return {
274287
hoisted,
275288
trimmed,

packages/svelte/src/internal/client/dom/template.js

Lines changed: 13 additions & 49 deletions
Original file line numberDiff line numberDiff line change
@@ -72,21 +72,8 @@ export function template(content, flags) {
7272
*/
7373
/*#__NO_SIDE_EFFECTS__*/
7474
export function template_with_script(content, flags) {
75-
var first = true;
7675
var fn = template(content, flags);
77-
78-
return () => {
79-
if (hydrating) return fn();
80-
81-
var node = /** @type {Element | DocumentFragment} */ (fn());
82-
83-
if (first) {
84-
first = false;
85-
run_scripts(node);
86-
}
87-
88-
return node;
89-
};
76+
return () => run_scripts(/** @type {Element | DocumentFragment} */ (fn()));
9077
}
9178

9279
/**
@@ -151,21 +138,8 @@ export function ns_template(content, flags, ns = 'svg') {
151138
*/
152139
/*#__NO_SIDE_EFFECTS__*/
153140
export function svg_template_with_script(content, flags) {
154-
var first = true;
155141
var fn = ns_template(content, flags);
156-
157-
return () => {
158-
if (hydrating) return fn();
159-
160-
var node = /** @type {Element | DocumentFragment} */ (fn());
161-
162-
if (first) {
163-
first = false;
164-
run_scripts(node);
165-
}
166-
167-
return node;
168-
};
142+
return () => run_scripts(/** @type {Element | DocumentFragment} */ (fn()));
169143
}
170144

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

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

203178
clone.textContent = script.textContent;
204179

205-
const replace = () => {
206-
// The script has changed - if it's at the edges, the effect now points at dead nodes
207-
if (is_fragment ? node.firstChild === script : node === script) {
208-
effect.nodes_start = clone;
209-
}
210-
if (is_fragment ? node.lastChild === script : node === script) {
211-
effect.nodes_end = clone;
212-
}
213-
214-
script.replaceWith(clone);
215-
};
216-
217-
// If node === script tag, replaceWith will do nothing because there's no parent yet,
218-
// waiting until that's the case using an effect solves this.
219-
// Don't do it in other circumstances or we could accidentally execute scripts
220-
// in an adjacent @html tag that was instantiated in the meantime.
221-
if (script === node) {
222-
queue_micro_task(replace);
223-
} else {
224-
replace();
180+
// The script has changed - if it's at the edges, the effect now points at dead nodes
181+
if (is_fragment ? node.firstChild === script : node === script) {
182+
effect.nodes_start = clone;
183+
}
184+
if (is_fragment ? node.lastChild === script : node === script) {
185+
effect.nodes_end = clone;
225186
}
187+
188+
script.replaceWith(clone);
226189
}
190+
return node;
227191
}
228192

229193
/**
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
import { test } from '../../assert';
2+
3+
export default test({
4+
mode: ['client'],
5+
async test({ assert, window }) {
6+
// wait the script to load (maybe there a better way)
7+
await new Promise((resolve) => setTimeout(resolve, 1));
8+
assert.htmlEqual(
9+
window.document.body.innerHTML,
10+
`<main><b id="r1">1</b><b id="r2">2</b><b id="r3">3</b></main>`
11+
);
12+
}
13+
});
Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,16 @@
1+
<script>
2+
function jssrc(src) {
3+
return 'data:text/javascript;base64, ' + btoa(src);
4+
}
5+
6+
const scriptSrcs = [ 1, 2, 3 ]
7+
.map(n => jssrc(`document.getElementById('r${n}').innerText = '${n}';`));
8+
</script>
9+
10+
<svelte:head>
11+
{#each scriptSrcs as src}
12+
<script src={src} async defer></script>
13+
{/each}
14+
</svelte:head>
15+
16+
<b id="r1">?</b><b id="r2">?</b><b id="r3">?</b>

0 commit comments

Comments
 (0)