Skip to content

Commit 6c5d119

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 6c5d119

File tree

4 files changed

+36
-8
lines changed

4 files changed

+36
-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: 15 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -191,22 +191,34 @@ function run_scripts(node) {
191191
/** @type {HTMLElement} */ (node).tagName === 'SCRIPT'
192192
? [/** @type {HTMLScriptElement} */ (node)]
193193
: node.querySelectorAll('script');
194+
const effect = /** @type {Effect} */ (current_effect);
194195
for (const script of scripts) {
195-
var clone = document.createElement('script');
196+
const clone = document.createElement('script');
196197
for (var attribute of script.attributes) {
197198
clone.setAttribute(attribute.name, attribute.value);
198199
}
199200

200201
clone.textContent = script.textContent;
201202

203+
const replace = () => {
204+
// The script has changed - if it's at the edges, the effect now points at dead nodes
205+
if (node.firstChild === script) {
206+
effect.nodes_start = clone;
207+
} else if (node.lastChild === script) {
208+
effect.nodes_end = clone;
209+
}
210+
211+
script.replaceWith(clone);
212+
};
213+
202214
// If node === script tag, replaceWith will do nothing because there's no parent yet,
203215
// waiting until that's the case using an effect solves this.
204216
// Don't do it in other circumstances or we could accidentally execute scripts
205217
// in an adjacent @html tag that was instantiated in the meantime.
206218
if (script === node) {
207-
queue_micro_task(() => script.replaceWith(clone));
219+
queue_micro_task(replace);
208220
} else {
209-
script.replaceWith(clone);
221+
replace();
210222
}
211223
}
212224
}

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)