Do not expose default slot let bindings to named slots (#6049)

* should not extend scope for across slots

* disallow named slots inheriting let: scope from default slots

* fix tests

* fix test

* fix

* add runtime tests

* rename test since it doesn't inherit anymore

* fix lint

* remove warnings

* add compile script

* document script

* improve warning

* fix test

* handle renames

* fix lint

* gather names from all parents instead of just the nearest

* remove unused import

* add reminder

---------

Co-authored-by: gtmnayan <[email protected]>

Author: tanhauhau

scripts/compile-test.js

@@ -0,0 +1,40 @@
+// Compile all Svelte files in a directory to JS and CSS files
+// Usage: node scripts/compile-test.js <directory>
+
+import { mkdirSync, readFileSync, writeFileSync } from 'fs';
+import path from 'path';
+import glob from 'tiny-glob/sync.js';
+import { compile } from '../src/compiler/index.js';
+
+const cwd = path.resolve(process.argv[2]);
+
+const options = [
+	['normal', {}],
+	['hydrate', { hydratable: true }],
+	['ssr', { generate: 'ssr' }]
+];
+for (const file of glob('**/*.svelte', { cwd })) {
+	const contents = readFileSync(`${cwd}/${file}`, 'utf-8').replace(/\r/g, '');
+	let w;
+	for (const [name, opts] of options) {
+		const dir = `${cwd}/_output/${name}`;
+
+		const { js, css, warnings } = compile(contents, {
+			...opts,
+			filename: file
+		});
+
+		if (warnings.length) {
+			w = warnings;
+		}
+
+		mkdirSync(dir, { recursive: true });
+		js.code && writeFileSync(`${dir}/${file.replace(/\.svelte$/, '.js')}`, js.code);
+		css.code && writeFileSync(`${dir}/${file.replace(/\.svelte$/, '.css')}`, css.code);
+	}
+
+	if (w) {
+		console.log(`Warnings for ${file}:`);
+		console.log(w);
+	}
+}

src/compiler/compile/Component.js

@@ -1643,8 +1643,9 @@ export default class Component {
 	 * @param {string} name
 	 * @param {any} node
 	 * @param {import('./nodes/shared/TemplateScope.js').default} template_scope
+	 * @param {import("./nodes/shared/Node.js").default} [owner]
 	 */
-	warn_if_undefined(name, node, template_scope) {
+	warn_if_undefined(name, node, template_scope, owner) {
 		if (name[0] === '$') {
 			if (name === '$' || (name[1] === '$' && !is_reserved_keyword(name))) {
 				return this.error(node, compiler_errors.illegal_global(name));
@@ -1656,6 +1657,34 @@ export default class Component {
 		if (this.var_lookup.has(name) && !this.var_lookup.get(name).global) return;
 		if (template_scope && template_scope.names.has(name)) return;
 		if (globals.has(name) && node.type !== 'InlineComponent') return;
+
+		function has_out_of_scope_let() {
+			for (let parent = owner.parent; parent; parent = parent.parent) {
+				if (parent.type === 'InlineComponent') {
+					const { let_attributes } = parent;
+
+					for (const attr of let_attributes) {
+						if (
+							// @ts-expect-error
+							// TODO extract_names only considers patterns but let attributes return expressions
+							(attr.expression && extract_names(attr.expression).includes(name)) ||
+							attr.name === name
+						)
+							return true;
+					}
+				}
+			}
+
+			return false;
+		}
+
+		if (owner && has_out_of_scope_let()) {
+			return this.warn(node, {
+				code: 'missing-declaration',
+				message: `let:${name} declared on parent component cannot be used inside named slot`
+			});
+		}
+
 		this.warn(node, compiler_warnings.missing_declaration(name, !!this.ast.instance));
 	}
 

src/compiler/compile/nodes/InlineComponent.js

@@ -4,7 +4,6 @@ import map_children from './shared/map_children.js';
 import Binding from './Binding.js';
 import EventHandler from './EventHandler.js';
 import Expression from './shared/Expression.js';
-import Let from './Let.js';
 import compiler_errors from '../compiler_errors.js';
 import { regex_only_whitespaces } from '../../utils/patterns.js';
 
@@ -22,9 +21,6 @@ export default class InlineComponent extends Node {
 	/** @type {import('./EventHandler.js').default[]} */
 	handlers = [];
 
-	/** @type {import('./Let.js').default[]} */
-	lets = [];
-
 	/** @type {import('./Attribute.js').default[]} */
 	css_custom_properties = [];
 
@@ -37,6 +33,8 @@ export default class InlineComponent extends Node {
 	/** @type {string} */
 	namespace;
 
+	/** @type {Attribute[]} */
+	let_attributes;
 	/**
 	 * @param {import('../Component.js').default} component
 	 * @param {import('./shared/Node.js').default} parent
@@ -58,6 +56,8 @@ export default class InlineComponent extends Node {
 			this.name === 'svelte:component'
 				? new Expression(component, this, scope, info.expression)
 				: null;
+
+		const let_attributes = (this.let_attributes = []);
 		info.attributes.forEach(
 			/** @param {import('../../interfaces.js').BaseDirective | import('../../interfaces.js').Attribute | import('../../interfaces.js').SpreadAttribute} node */ (
 				node
@@ -84,7 +84,7 @@ export default class InlineComponent extends Node {
 						this.handlers.push(new EventHandler(component, this, scope, node));
 						break;
 					case 'Let':
-						this.lets.push(new Let(component, this, scope, node));
+						let_attributes.push(node);
 						break;
 					case 'Transition':
 						return component.error(node, compiler_errors.invalid_transition);
@@ -98,21 +98,9 @@ export default class InlineComponent extends Node {
 				/* eslint-enable no-fallthrough */
 			}
 		);
-		if (this.lets.length > 0) {
-			this.scope = scope.child();
-			this.lets.forEach(
-				/** @param {any} l */ (l) => {
-					const dependencies = new Set([l.name.name]);
-					l.names.forEach(
-						/** @param {any} name */ (name) => {
-							this.scope.add(name, dependencies, this);
-						}
-					);
-				}
-			);
-		} else {
-			this.scope = scope;
-		}
+
+		this.scope = scope;
+
 		this.handlers.forEach(
 			/** @param {any} handler */ (handler) => {
 				handler.modifiers.forEach(
@@ -178,6 +166,18 @@ export default class InlineComponent extends Node {
 				children: info.children
 			});
 		}
+
+		if (let_attributes.length) {
+			// copy let: attribute from <Component /> to <svelte:fragment slot="default" />
+			// as they are for `slot="default"` only
+			children.forEach((child) => {
+				const slot = child.attributes.find((attribute) => attribute.name === 'slot');
+				if (!slot || slot.value[0].data === 'default') {
+					child.attributes.push(...let_attributes);
+				}
+			});
+		}
+
 		this.children = map_children(component, this, this.scope, children);
 	}
 	get slot_template_name() {

src/compiler/compile/nodes/Slot.js

@@ -40,39 +40,7 @@ export default class Slot extends Element {
 			}
 		);
 		if (!this.slot_name) this.slot_name = 'default';
-		if (this.slot_name === 'default') {
-			// if this is the default slot, add our dependencies to any
-			// other slots (which inherit our slot values) that were
-			// previously encountered
-			component.slots.forEach(
-				/** @param {any} slot */ (slot) => {
-					this.values.forEach(
-						/**
-						 * @param {any} attribute
-						 * @param {any} name
-						 */ (attribute, name) => {
-							if (!slot.values.has(name)) {
-								slot.values.set(name, attribute);
-							}
-						}
-					);
-				}
-			);
-		} else if (component.slots.has('default')) {
-			// otherwise, go the other way — inherit values from
-			// a previously encountered default slot
-			const default_slot = component.slots.get('default');
-			default_slot.values.forEach(
-				/**
-				 * @param {any} attribute
-				 * @param {any} name
-				 */ (attribute, name) => {
-					if (!this.values.has(name)) {
-						this.values.set(name, attribute);
-					}
-				}
-			);
-		}
+
 		component.slots.set(this.slot_name, this);
 		this.cannot_use_innerhtml();
 		this.not_static_content();

src/compiler/compile/nodes/shared/Expression.js

@@ -77,6 +77,7 @@ export default class Expression {
 		this.scope_map = map;
 		const expression = this;
 		let function_expression;
+
 		// discover dependencies, but don't change the code yet
 		walk(info, {
 			/**
@@ -125,7 +126,7 @@ export default class Expression {
 							dependencies.add(name);
 						}
 						component.add_reference(node, name);
-						component.warn_if_undefined(name, nodes[0], template_scope);
+						component.warn_if_undefined(name, nodes[0], template_scope, owner);
 					}
 					this.skip();
 				}
@@ -376,8 +377,9 @@ export default class Expression {
 								node = node.parent;
 							}
 							const func_expression = func_declaration[0];
-							if (node.type === 'InlineComponent' || node.type === 'SlotTemplate') {
-								// <Comp let:data />
+
+							if (node.type === 'SlotTemplate') {
+								// <svelte:fragment let:data />
 								this.replace(func_expression);
 							} else {
 								// {#each}, {#await}
@@ -458,6 +460,7 @@ export default class Expression {
 				}
 			}
 		});
+
 		if (declarations.length > 0) {
 			block.maintain_context = true;
 			declarations.forEach(

src/compiler/compile/render_dom/wrappers/InlineComponent/index.js

@@ -1,19 +1,18 @@
-import Wrapper from '../shared/Wrapper.js';
-import BindingWrapper from '../Element/Binding.js';
-import SlotTemplateWrapper from '../SlotTemplate.js';
+import { b, p, x } from 'code-red';
+import { extract_ignores_above_node } from '../../../../utils/extract_svelte_ignore.js';
 import { sanitize } from '../../../../utils/names.js';
+import { namespaces } from '../../../../utils/namespaces.js';
+import compiler_warnings from '../../../compiler_warnings.js';
 import add_to_set from '../../../utils/add_to_set.js';
-import { b, x, p } from 'code-red';
-import is_dynamic from '../shared/is_dynamic.js';
-import bind_this from '../shared/bind_this.js';
-import EventHandler from '../Element/EventHandler.js';
-import { extract_names } from 'periscopic';
-import mark_each_block_bindings from '../shared/mark_each_block_bindings.js';
 import { string_to_member_expression } from '../../../utils/string_to_member_expression.js';
+import BindingWrapper from '../Element/Binding.js';
+import EventHandler from '../Element/EventHandler.js';
+import SlotTemplateWrapper from '../SlotTemplate.js';
+import Wrapper from '../shared/Wrapper.js';
+import bind_this from '../shared/bind_this.js';
+import is_dynamic from '../shared/is_dynamic.js';
 import { is_head } from '../shared/is_head.js';
-import compiler_warnings from '../../../compiler_warnings.js';
-import { namespaces } from '../../../../utils/namespaces.js';
-import { extract_ignores_above_node } from '../../../../utils/extract_svelte_ignore.js';
+import mark_each_block_bindings from '../shared/mark_each_block_bindings.js';
 
 const regex_invalid_variable_identifier_characters = /[^a-zA-Z_$]/g;
 
@@ -77,11 +76,6 @@ export default class InlineComponentWrapper extends Wrapper {
 			).toLowerCase()
 		};
 		if (this.node.children.length) {
-			this.node.lets.forEach((l) => {
-				extract_names(l.value || l.name).forEach((name) => {
-					renderer.add_to_context(name, true);
-				});
-			});
 			this.children = this.node.children.map(
 				(child) =>
 					new SlotTemplateWrapper(

src/compiler/compile/render_dom/wrappers/SlotTemplate.js

@@ -38,10 +38,7 @@ export default class SlotTemplateWrapper extends Wrapper {
 			type: 'slot'
 		});
 		this.renderer.blocks.push(this.block);
-		const seen = new Set(lets.map((l) => l.name.name));
-		this.parent.node.lets.forEach((l) => {
-			if (!seen.has(l.name.name)) lets.push(l);
-		});
+
 		/** @type {import('./InlineComponent/index.js').default} */ (this.parent).set_slot(
 			slot_template_name,
 			get_slot_definition(this.block, scope, lets)

src/compiler/compile/render_ssr/handlers/Slot.js

@@ -25,11 +25,6 @@ export default function (node, renderer, options) {
 			: ${result}
 	`);
 	if (slot && nearest_inline_component) {
-		const lets = node.lets;
-		const seen = new Set(lets.map((l) => l.name.name));
-		nearest_inline_component.lets.forEach((l) => {
-			if (!seen.has(l.name.name)) lets.push(l);
-		});
 		options.slot_scopes.set(slot, {
 			input: get_slot_scope(node.lets),
 			output: renderer.pop()

src/compiler/compile/render_ssr/handlers/SlotTemplate.js

@@ -20,11 +20,7 @@ export default function (node, renderer, options) {
 	);
 	renderer.push();
 	renderer.render(children, options);
-	const lets = node.lets;
-	const seen = new Set(lets.map((l) => l.name.name));
-	parent_inline_component.lets.forEach((l) => {
-		if (!seen.has(l.name.name)) lets.push(l);
-	});
+
 	const slot_fragment_content = renderer.pop();
 	if (!is_empty_template_literal(slot_fragment_content)) {
 		if (options.slot_scopes.has(node.slot_template_name)) {

test/runtime/samples/component-slot-default-in-each/Nested.svelte

@@ -0,0 +1,11 @@
+<script>
+	let promise = new Promise(resolve => resolve(10));
+</script>
+
+{#each {length: 3} as _, i}
+	<slot item={i}/>
+{/each}
+
+{#await promise then value}
+	<slot {value}/>
+{/await}

test/runtime/samples/component-slot-default-in-each/_config.js

@@ -0,0 +1,7 @@
+export default {
+	html: `
+		<div>0 - undefined</div>
+		<div>1 - undefined</div>
+		<div>2 - undefined</div>
+	`
+};

test/runtime/samples/component-slot-default-in-each/main.svelte

@@ -0,0 +1,7 @@
+<script>
+	import Nested from './Nested.svelte';
+</script>
+
+<Nested let:item let:value>
+	<div>{item} - {value}</div>
+</Nested>

test/runtime/samples/component-slot-let-scope-2/Nested.svelte

@@ -0,0 +1,3 @@
+<slot />
+<slot thing={2} name="thing"/>
+	

test/runtime/samples/component-slot-let-scope-2/_config.js

@@ -0,0 +1,3 @@
+export default {
+	html: '<span>undefined</span><span>2</span>'
+};

test/runtime/samples/component-slot-let-scope-2/main.svelte

@@ -0,0 +1,8 @@
+<script>
+	import Nested from './Nested.svelte';
+</script>
+
+<Nested let:thing>
+	<span>{thing}</span>
+	<svelte:fragment slot="thing" let:thing><span>{thing}</span></svelte:fragment>
+</Nested>

test/runtime/samples/component-slot-named-inherits-default-lets/Nested.svelte renamed to test/runtime/samples/component-slot-let-scope-3/Nested.svelte

@@ -1,6 +1,4 @@
 <script>
-	import { onDestroy } from 'svelte';
-
 	let count = 0;
 
 	function increment() {