Skip to content

Commit b093ad6

Browse files
committed
fix: ensure inner script tags are properly removed
We have a `run_scripts` function which is invoked in case a template block contains a script tag, and that function replaces the script tags (so that they actually run). If such a script tag is first or last in the template, the replacement is not picked up by our `node.first_node/last_node` logic anymore, and so it contains a stale tag. That means on cleanup the remove logic fails. In the case of the referenced issue, it just runs past the script tag till the very end of the head, removing the just added style tag from the new page. Fixes #13086
1 parent 53a90fb commit b093ad6

File tree

4 files changed

+39
-8
lines changed

4 files changed

+39
-8
lines changed

.changeset/selfish-trainers-kiss.md

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,5 @@
1+
---
2+
'svelte': patch
3+
---
4+
5+
fix: ensure inner script tags are properly removed

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

Lines changed: 18 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -187,26 +187,41 @@ function run_scripts(node) {
187187
// scripts were SSR'd, in which case they will run
188188
if (hydrating) return;
189189

190+
const is_fragment = node.nodeType === 11;
190191
const scripts =
191192
/** @type {HTMLElement} */ (node).tagName === 'SCRIPT'
192193
? [/** @type {HTMLScriptElement} */ (node)]
193194
: node.querySelectorAll('script');
195+
const effect = /** @type {Effect} */ (current_effect);
196+
194197
for (const script of scripts) {
195-
var clone = document.createElement('script');
198+
const clone = document.createElement('script');
196199
for (var attribute of script.attributes) {
197200
clone.setAttribute(attribute.name, attribute.value);
198201
}
199202

200203
clone.textContent = script.textContent;
201204

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+
202217
// If node === script tag, replaceWith will do nothing because there's no parent yet,
203218
// waiting until that's the case using an effect solves this.
204219
// Don't do it in other circumstances or we could accidentally execute scripts
205220
// in an adjacent @html tag that was instantiated in the meantime.
206221
if (script === node) {
207-
queue_micro_task(() => script.replaceWith(clone));
222+
queue_micro_task(replace);
208223
} else {
209-
script.replaceWith(clone);
224+
replace();
210225
}
211226
}
212227
}

packages/svelte/tests/runtime-browser/samples/sole-script-tag/_config.js

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -4,8 +4,12 @@ export default test({
44
// Test that template with sole script tag does execute when instantiated in the client.
55
// Needs to be in this test suite because JSDOM does not quite get this right.
66
mode: ['client'],
7-
test({ window, assert }) {
7+
async test({ target, assert }) {
88
// In here to give effects etc time to execute
9-
assert.htmlEqual(window.document.body.innerHTML, 'this should be executed');
9+
assert.htmlEqual(target.querySelector('div')?.innerHTML || '', 'this should be executed');
10+
// Check that the script tag is properly removed
11+
target.querySelector('button')?.click();
12+
await new Promise((r) => setTimeout(r, 100));
13+
assert.equal(target.querySelector('div'), null);
1014
}
1115
});
Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,11 @@
1-
<div></div>
2-
{#if true}
3-
<script>document.body.innerHTML = 'this should be executed'</script>
1+
<script lang="ts">
2+
let visible = $state(true);
3+
</script>
4+
5+
<button onclick={() => (visible = false)}>hide</button>
6+
{#if visible}
7+
<script>
8+
document.body.querySelector('.after').innerHTML = 'this should be executed';
9+
</script>
410
{/if}
11+
<div class="after">after</div>

0 commit comments

Comments
 (0)