Merge branch 'main' into opaque-rune

trueadm · trueadm · commit 0166bc485040 · 2024-12-10T15:53:21.000Z
diff --git a/.changeset/chatty-windows-own.md b/.changeset/chatty-windows-own.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: ensure snippet hoisting works in the correct scope
diff --git a/.changeset/cyan-ducks-train.md b/.changeset/cyan-ducks-train.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: restore input binding selection position
diff --git a/.changeset/red-worms-suffer.md b/.changeset/red-worms-suffer.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: transform everything that is not a selector inside `:global`
diff --git a/.changeset/thick-dryers-kiss.md b/.changeset/thick-dryers-kiss.md
@@ -0,0 +1,5 @@
+---
+'svelte': patch
+---
+
+fix: don't emit assignment warnings for bindings
diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/SnippetBlock.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/SnippetBlock.js
@@ -99,8 +99,9 @@ function can_hoist_snippet(scope, scopes, visited = new Set()) {
 		if (binding.initial?.type === 'SnippetBlock') {
 			if (visited.has(binding)) continue;
 			visited.add(binding);
+			const snippet_scope = /** @type {Scope} */ (scopes.get(binding.initial));
 
-			if (can_hoist_snippet(binding.scope, scopes, visited)) {
+			if (can_hoist_snippet(snippet_scope, scopes, visited)) {
 				continue;
 			}
 		}
diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/AssignmentExpression.js
@@ -169,6 +169,17 @@ function build_assignment(operator, left, right, context) {
 		}
 	}
 
+	// special case — ignore `bind:prop={getter, (v) => (...)}` / `bind:value={x.y}`
+	if (
+		path.at(-1) === 'Component' ||
+		path.at(-1) === 'SvelteComponent' ||
+		(path.at(-1) === 'ArrowFunctionExpression' &&
+			path.at(-2) === 'SequenceExpression' &&
+			(path.at(-3) === 'Component' || path.at(-3) === 'SvelteComponent'))
+	) {
+		should_transform = false;
+	}
+
 	if (left.type === 'MemberExpression' && should_transform) {
 		const callee = callees[operator];
 
diff --git a/packages/svelte/src/compiler/phases/3-transform/css/index.js b/packages/svelte/src/compiler/phases/3-transform/css/index.js
@@ -78,7 +78,7 @@ const visitors = {
 		context.state.code.addSourcemapLocation(node.end);
 		context.next();
 	},
-	Atrule(node, { state, next }) {
+	Atrule(node, { state, next, path }) {
 		if (is_keyframes_node(node)) {
 			let start = node.start + node.name.length + 1;
 			while (state.code.original[start] === ' ') start += 1;
@@ -87,7 +87,7 @@ const visitors = {
 
 			if (node.prelude.startsWith('-global-')) {
 				state.code.remove(start, start + 8);
-			} else {
+			} else if (!is_in_global_block(path)) {
 				state.code.prependRight(start, `${state.hash}-`);
 			}
 
@@ -134,7 +134,7 @@ const visitors = {
 			}
 		}
 	},
-	Rule(node, { state, next, visit }) {
+	Rule(node, { state, next, visit, path }) {
 		if (state.minify) {
 			remove_preceding_whitespace(node.start, state);
 			remove_preceding_whitespace(node.block.end - 1, state);
@@ -154,7 +154,7 @@ const visitors = {
 			return;
 		}
 
-		if (!is_used(node)) {
+		if (!is_used(node) && !is_in_global_block(path)) {
 			if (state.minify) {
 				state.code.remove(node.start, node.end);
 			} else {
@@ -182,20 +182,20 @@ const visitors = {
 					state.code.appendLeft(node.block.end, '*/');
 				}
 
-				// don't recurse into selector or body
+				// don't recurse into selectors but visit the body
+				visit(node.block);
 				return;
 			}
-
-			// don't recurse into body
-			visit(node.prelude);
-			return;
 		}
 
 		next();
 	},
 	SelectorList(node, { state, next, path }) {
-		// Only add comments if we're not inside a complex selector that itself is unused
-		if (!path.find((n) => n.type === 'ComplexSelector' && !n.metadata.used)) {
+		// Only add comments if we're not inside a complex selector that itself is unused or a global block
+		if (
+			!is_in_global_block(path) &&
+			!path.find((n) => n.type === 'ComplexSelector' && !n.metadata.used)
+		) {
 			const children = node.children;
 			let pruning = false;
 			let prune_start = children[0].start;
@@ -359,6 +359,14 @@ const visitors = {
 	}
 };
 
+/**
+ *
+ * @param {Array<Css.Node>} path
+ */
+function is_in_global_block(path) {
+	return path.some((node) => node.type === 'Rule' && node.metadata.is_global_block);
+}
+
 /**
  * @param {Css.PseudoClassSelector} selector
  * @param {Css.Combinator | null} combinator
diff --git a/packages/svelte/src/compiler/phases/scope.js b/packages/svelte/src/compiler/phases/scope.js
@@ -628,12 +628,8 @@ export function create_scopes(ast, root, allow_reactive_declarations, parent) {
 
 		SnippetBlock(node, context) {
 			const state = context.state;
-			// Special-case for root-level snippets: they become part of the instance scope
-			const is_top_level = !context.path.at(-2);
 			let scope = state.scope;
-			if (is_top_level) {
-				scope = /** @type {Scope} */ (parent);
-			}
+
 			scope.declare(node.expression, 'normal', 'function', node);
 
 			const child_scope = state.scope.child();
diff --git a/packages/svelte/src/internal/client/dev/assign.js b/packages/svelte/src/internal/client/dev/assign.js
@@ -1,3 +1,4 @@
+import { untrack } from '../runtime.js';
 import * as w from '../warnings.js';
 import { sanitize_location } from './location.js';
 
@@ -23,7 +24,12 @@ function compare(a, b, property, location) {
  * @param {string} location
  */
 export function assign(object, property, value, location) {
-	return compare((object[property] = value), object[property], property, location);
+	return compare(
+		(object[property] = value),
+		untrack(() => object[property]),
+		property,
+		location
+	);
 }
 
 /**
@@ -33,7 +39,12 @@ export function assign(object, property, value, location) {
  * @param {string} location
  */
 export function assign_and(object, property, value, location) {
-	return compare((object[property] &&= value), object[property], property, location);
+	return compare(
+		(object[property] &&= value),
+		untrack(() => object[property]),
+		property,
+		location
+	);
 }
 
 /**
@@ -43,7 +54,12 @@ export function assign_and(object, property, value, location) {
  * @param {string} location
  */
 export function assign_or(object, property, value, location) {
-	return compare((object[property] ||= value), object[property], property, location);
+	return compare(
+		(object[property] ||= value),
+		untrack(() => object[property]),
+		property,
+		location
+	);
 }
 
 /**
@@ -53,5 +69,10 @@ export function assign_or(object, property, value, location) {
  * @param {string} location
  */
 export function assign_nullish(object, property, value, location) {
-	return compare((object[property] ??= value), object[property], property, location);
+	return compare(
+		(object[property] ??= value),
+		untrack(() => object[property]),
+		property,
+		location
+	);
 }
diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/input.js b/packages/svelte/src/internal/client/dom/elements/bindings/input.js
@@ -30,8 +30,17 @@ export function bind_value(input, get, set = get) {
 		// In runes mode, respect any validation in accessors (doesn't apply in legacy mode,
 		// because we use mutable state which ensures the render effect always runs)
 		if (runes && value !== (value = get())) {
+			var start = input.selectionStart;
+			var end = input.selectionEnd;
+
 			// the value is coerced on assignment
 			input.value = value ?? '';
+
+			// Restore selection
+			if (end !== null) {
+				input.selectionStart = start;
+				input.selectionEnd = Math.min(end, input.value.length);
+			}
 		}
 	});
 
diff --git a/packages/svelte/tests/css/samples/global-block/expected.css b/packages/svelte/tests/css/samples/global-block/expected.css
@@ -69,3 +69,20 @@
 			color: red;
 		}
 	}*/
+	/* :global{*/
+		.x{
+			animation: svelte-xyz-test 1s;
+		}
+
+		@keyframes test-in{
+			to{
+				opacity: 1;
+			}
+		}
+	/*}*/
+
+	@keyframes svelte-xyz-test{
+		to{
+			opacity: 1;
+		}
+	}
diff --git a/packages/svelte/tests/css/samples/global-block/input.svelte b/packages/svelte/tests/css/samples/global-block/input.svelte
@@ -71,4 +71,21 @@
 			color: red;
 		}
 	}
+	:global{
+		.x{
+			animation: test 1s;
+		}
+
+		@keyframes test-in{
+			to{
+				opacity: 1;
+			}
+		}
+	}
+
+	@keyframes test{
+		to{
+			opacity: 1;
+		}
+	}
 </style>
diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-coercive-assignment-warning/Test.svelte b/packages/svelte/tests/runtime-runes/samples/proxy-coercive-assignment-warning/Test.svelte
@@ -0,0 +1,11 @@
+<script>
+	let { x = $bindable() } = $props();
+
+	$effect(() => {
+		x = {};
+	});
+
+	export function soThatTestReturnsAnObject() {
+		return x;
+	}
+</script>
diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-coercive-assignment-warning/_config.js b/packages/svelte/tests/runtime-runes/samples/proxy-coercive-assignment-warning/_config.js
@@ -6,19 +6,19 @@ export default test({
 		dev: true
 	},
 
-	html: `<button>items: null</button>`,
+	html: `<button>items: null</button> <div>x</div>`,
 
 	test({ assert, target, warnings }) {
 		const btn = target.querySelector('button');
 
 		flushSync(() => btn?.click());
-		assert.htmlEqual(target.innerHTML, `<button>items: []</button>`);
+		assert.htmlEqual(target.innerHTML, `<button>items: []</button> <div>x</div>`);
 
 		flushSync(() => btn?.click());
-		assert.htmlEqual(target.innerHTML, `<button>items: [0]</button>`);
+		assert.htmlEqual(target.innerHTML, `<button>items: [0]</button> <div>x</div>`);
 
 		assert.deepEqual(warnings, [
-			'Assignment to `items` property (main.svelte:5:24) will evaluate to the right-hand side, not the value of `items` following the assignment. This may result in unexpected behaviour.'
+			'Assignment to `items` property (main.svelte:8:24) will evaluate to the right-hand side, not the value of `items` following the assignment. This may result in unexpected behaviour.'
 		]);
 	}
 });
diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-coercive-assignment-warning/main.svelte b/packages/svelte/tests/runtime-runes/samples/proxy-coercive-assignment-warning/main.svelte
@@ -0,0 +1,16 @@
+ <script>
+	import Test from './Test.svelte';
+
+	let entries = $state([]);
+	let object = $state({ items: null });
+</script>
+
+<button onclick={() => (object.items ??= []).push(object.items.length)}>
+	items: {JSON.stringify(object.items)}
+</button>
+
+<!-- these should not emit warnings -->
+<div bind:this={entries[0]}>x</div>
+<Test bind:this={entries[1]}></Test>
+<Test bind:this={() => entries[2], (v) => (entries[2] = v)}></Test>
+<Test bind:x={entries[3]}></Test>
diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-coercive-assignment/_config.js b/packages/svelte/tests/runtime-runes/samples/proxy-coercive-assignment/_config.js
@@ -5,7 +5,7 @@ export default test({
 	html: `<button>items: null</button>`,
 
 	test({ assert, target }) {
-		const [btn1, btn2] = target.querySelectorAll('button');
+		const [btn1] = target.querySelectorAll('button');
 
 		flushSync(() => btn1.click());
 		assert.htmlEqual(target.innerHTML, `<button>items: [0]</button>`);
diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-coercive-assignment/main.svelte b/packages/svelte/tests/runtime-runes/samples/proxy-coercive-assignment/main.svelte
diff --git a/packages/svelte/tests/runtime-runes/samples/proxy-nullish-coalescing-assignment-warning/main.svelte b/packages/svelte/tests/runtime-runes/samples/proxy-nullish-coalescing-assignment-warning/main.svelte
diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-hoisting-2/_config.js b/packages/svelte/tests/runtime-runes/samples/snippet-hoisting-2/_config.js
@@ -0,0 +1,5 @@
+import { test } from '../../test';
+
+export default test({
+	html: 'a'
+});
diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-hoisting-2/main.svelte b/packages/svelte/tests/runtime-runes/samples/snippet-hoisting-2/main.svelte
@@ -0,0 +1,13 @@
+<script>
+	let abc = 'a'
+</script>
+
+{@render b()}
+
+{#snippet a()}
+	{abc}
+{/snippet}
+
+{#snippet b()}
+	{@render a()}
+{/snippet}
diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-hoisting-3/_config.js b/packages/svelte/tests/runtime-runes/samples/snippet-hoisting-3/_config.js
@@ -0,0 +1,5 @@
+import { test } from '../../test';
+
+export default test({
+	html: '<h1>Hello world!</h1>'
+});
diff --git a/packages/svelte/tests/runtime-runes/samples/snippet-hoisting-3/main.svelte b/packages/svelte/tests/runtime-runes/samples/snippet-hoisting-3/main.svelte
@@ -0,0 +1,13 @@
+<script module>
+	export { foo }
+</script>
+
+<script>
+	let name = 'world';
+</script>
+
+<h1>Hello {name}!</h1>
+
+{#snippet foo()}
+	oo
+{/snippet}

-Original file line number
+Diff line change
@@ @@ -0,0 +1,5 @@ @@
 +---
 +'svelte': patch
 +---
++
 +fix: ensure snippet hoisting works in the correct scope
Original file line number	Diff line number	Diff line change
`@@ -99,8 +99,9 @@ function can_hoist_snippet(scope, scopes, visited = new Set()) {`
`99`	`99`	`if (binding.initial?.type === 'SnippetBlock') {`
`100`	`100`	`if (visited.has(binding)) continue;`
`101`	`101`	`visited.add(binding);`
	`102`	`+ const snippet_scope = /** @type {Scope} */ (scopes.get(binding.initial));`
`102`	`103`
`103`		`- if (can_hoist_snippet(binding.scope, scopes, visited)) {`
	`104`	`+ if (can_hoist_snippet(snippet_scope, scopes, visited)) {`
`104`	`105`	`continue;`
`105`	`106`	`}`
`106`	`107`	`}`