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

Author: dummdidumm

.changeset/selfish-trainers-kiss.md

@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: ensure inner script tags are properly removed

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();
 		}
 	}
 }

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);
 	}
 });

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>