From c372dd864db30b4f81c105cd365cb9be5a280688 Mon Sep 17 00:00:00 2001
From: Simon H <5968653+dummdidumm@users.noreply.github.com>
Date: Fri, 6 Sep 2024 21:28:27 +0200
Subject: [PATCH] fix: ensure inner script tags are properly removed (#13152)

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
---
 .changeset/selfish-trainers-kiss.md           |  5 +++++
 .../src/internal/client/dom/template.js       | 21 ++++++++++++++++---
 .../samples/sole-script-tag/_config.js        |  8 +++++--
 .../samples/sole-script-tag/main.svelte       | 13 +++++++++---
 4 files changed, 39 insertions(+), 8 deletions(-)
 create mode 100644 .changeset/selfish-trainers-kiss.md

diff --git a/.changeset/selfish-trainers-kiss.md b/.changeset/selfish-trainers-kiss.md
new file mode 100644
index 0000000000..40e781e20f
--- /dev/null
+++ b/.changeset/selfish-trainers-kiss.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: ensure inner script tags are properly removed
diff --git a/packages/svelte/src/internal/client/dom/template.js b/packages/svelte/src/internal/client/dom/template.js
index 767bb5bdd0..0b927071d0 100644
--- a/packages/svelte/src/internal/client/dom/template.js
+++ b/packages/svelte/src/internal/client/dom/template.js
@@ -187,26 +187,41 @@ function run_scripts(node) {
 	// scripts were SSR'd, in which case they will run
 	if (hydrating) return;
 
+	const is_fragment = node.nodeType === 11;
 	const scripts =
 		/** @type {HTMLElement} */ (node).tagName === 'SCRIPT'
 			? [/** @type {HTMLScriptElement} */ (node)]
 			: node.querySelectorAll('script');
+	const effect = /** @type {Effect} */ (current_effect);
+
 	for (const script of scripts) {
-		var clone = document.createElement('script');
+		const clone = document.createElement('script');
 		for (var attribute of script.attributes) {
 			clone.setAttribute(attribute.name, attribute.value);
 		}
 
 		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(() => script.replaceWith(clone));
+			queue_micro_task(replace);
 		} else {
-			script.replaceWith(clone);
+			replace();
 		}
 	}
 }
diff --git a/packages/svelte/tests/runtime-browser/samples/sole-script-tag/_config.js b/packages/svelte/tests/runtime-browser/samples/sole-script-tag/_config.js
index 650e9241ea..ee1e810d30 100644
--- a/packages/svelte/tests/runtime-browser/samples/sole-script-tag/_config.js
+++ b/packages/svelte/tests/runtime-browser/samples/sole-script-tag/_config.js
@@ -4,8 +4,12 @@ export default test({
 	// Test that template with sole script tag does execute when instantiated in the client.
 	// Needs to be in this test suite because JSDOM does not quite get this right.
 	mode: ['client'],
-	test({ window, assert }) {
+	async test({ target, assert }) {
 		// In here to give effects etc time to execute
-		assert.htmlEqual(window.document.body.innerHTML, 'this should be executed');
+		assert.htmlEqual(target.querySelector('div')?.innerHTML || '', 'this should be executed');
+		// Check that the script tag is properly removed
+		target.querySelector('button')?.click();
+		await new Promise((r) => setTimeout(r, 100));
+		assert.equal(target.querySelector('script'), null);
 	}
 });
diff --git a/packages/svelte/tests/runtime-browser/samples/sole-script-tag/main.svelte b/packages/svelte/tests/runtime-browser/samples/sole-script-tag/main.svelte
index 509b36028d..eda4cee629 100644
--- a/packages/svelte/tests/runtime-browser/samples/sole-script-tag/main.svelte
+++ b/packages/svelte/tests/runtime-browser/samples/sole-script-tag/main.svelte
@@ -1,4 +1,11 @@
-<div></div>
-{#if true}
-    <script>document.body.innerHTML = 'this should be executed'</script>
+<script lang="ts">
+	let visible = $state(true);
+</script>
+
+<button onclick={() => (visible = false)}>hide</button>
+{#if visible}
+	<script>
+		document.body.querySelector('.after').innerHTML = 'this should be executed';
+	</script>
 {/if}
+<div class="after">after</div>