add validation and test

Author: tanhauhau

src/compiler/compile/nodes/Element.ts

@@ -847,6 +847,11 @@ export default class Element extends Node {
 			);
 		}
 	}
+
+	get slot_template_name() {
+		// attribute.get_static_value
+		return this.attributes.find(attribute => attribute.name === 'slot').get_static_value() as string;
+	}
 }
 
 function should_have_attribute(

src/compiler/compile/nodes/SlotTemplate.ts

@@ -12,7 +12,7 @@ export default class SlotTemplate extends Node {
 	children: INode[];
 	lets: Let[] = [];
 	slot_attribute: Attribute;
-	slot_name: string = 'default';
+	slot_template_name: string = 'default';
 
 	constructor(component: Component, parent: INode, scope: TemplateScope, info: any) {
 		super(component, parent, scope, info);
@@ -52,7 +52,7 @@ export default class SlotTemplate extends Node {
 								message: `slot attribute value is missing`
 							});
 						}
-						this.slot_name = this.slot_attribute.get_static_value() as string;
+						this.slot_template_name = this.slot_attribute.get_static_value() as string;
 						break;
 					}
 					throw new Error(`Invalid attribute "${node.name}" in <svelte:slot>`);
@@ -70,7 +70,7 @@ export default class SlotTemplate extends Node {
 		if (this.parent.type !== 'InlineComponent') {
 			this.component.error(this, {
 				code: `invalid-slotted-content`,
-				message: `Element with a slot='...' attribute must be a child of a component`
+				message: `<svelte:slot> must be a child of a component`
 			});
 		}
 	}

src/compiler/compile/nodes/shared/TemplateScope.ts

@@ -3,8 +3,9 @@ import ThenBlock from '../ThenBlock';
 import CatchBlock from '../CatchBlock';
 import InlineComponent from '../InlineComponent';
 import Element from '../Element';
+import SlotTemplate from '../SlotTemplate';
 
-type NodeWithScope = EachBlock | ThenBlock | CatchBlock | InlineComponent | Element;
+type NodeWithScope = EachBlock | ThenBlock | CatchBlock | InlineComponent | Element | SlotTemplate;
 
 export default class TemplateScope {
 	names: Set<string>;
@@ -40,7 +41,7 @@ export default class TemplateScope {
 
 	is_let(name: string) {
 		const owner = this.get_owner(name);
-		return owner && (owner.type === 'Element' || owner.type === 'InlineComponent');
+		return owner && (owner.type === 'Element' || owner.type === 'InlineComponent' || owner.type === 'SlotTemplate');
 	}
 
 	is_await(name: string) {

src/compiler/compile/render_dom/wrappers/Element/index.ts

@@ -2,7 +2,7 @@ import Renderer from '../../Renderer';
 import Element from '../../../nodes/Element';
 import Wrapper from '../shared/Wrapper';
 import Block from '../../Block';
-import { is_void, sanitize } from '../../../../utils/names';
+import { is_void } from '../../../../utils/names';
 import FragmentWrapper from '../Fragment';
 import { escape_html, string_literal } from '../../../utils/stringify';
 import TextWrapper from '../Text';
@@ -14,12 +14,9 @@ import StyleAttributeWrapper from './StyleAttribute';
 import SpreadAttributeWrapper from './SpreadAttribute';
 import { dimensions } from '../../../../utils/patterns';
 import Binding from './Binding';
-import InlineComponentWrapper from '../InlineComponent';
 import add_to_set from '../../../utils/add_to_set';
 import { add_event_handler } from '../shared/add_event_handlers';
 import { add_action } from '../shared/add_actions';
-import create_debugging_comment from '../shared/create_debugging_comment';
-import { get_slot_definition } from '../shared/get_slot_definition';
 import bind_this from '../shared/bind_this';
 import { is_head } from '../shared/is_head';
 import { Identifier } from 'estree';
@@ -176,49 +173,6 @@ export default class ElementWrapper extends Wrapper {
 		}
 
 		this.attributes = this.node.attributes.map(attribute => {
-			if (attribute.name === 'slot') {
-				// TODO make separate subclass for this?
-				let owner = this.parent;
-				while (owner) {
-					if (owner.node.type === 'InlineComponent') {
-						break;
-					}
-
-					if (owner.node.type === 'Element' && /-/.test(owner.node.name)) {
-						break;
-					}
-
-					owner = owner.parent;
-				}
-
-				if (owner && owner.node.type === 'InlineComponent') {
-					const name = attribute.get_static_value() as string;
-
-					if (!(owner as unknown as InlineComponentWrapper).slots.has(name)) {
-						const child_block = block.child({
-							comment: create_debugging_comment(node, this.renderer.component),
-							name: this.renderer.component.get_unique_name(`create_${sanitize(name)}_slot`),
-							type: 'slot'
-						});
-
-						const { scope, lets } = this.node;
-						const seen = new Set(lets.map(l => l.name.name));
-
-						(owner as unknown as InlineComponentWrapper).node.lets.forEach(l => {
-							if (!seen.has(l.name.name)) lets.push(l);
-						});
-
-						(owner as unknown as InlineComponentWrapper).slots.set(
-							name,
-							get_slot_definition(child_block, scope, lets)
-						);
-						this.renderer.blocks.push(child_block);
-					}
-
-					this.slot_block = (owner as unknown as InlineComponentWrapper).slots.get(name).block;
-					block = this.slot_block;
-				}
-			}
 			if (attribute.name === 'style') {
 				return new StyleAttributeWrapper(this, block, attribute);
 			}
@@ -273,26 +227,13 @@ export default class ElementWrapper extends Wrapper {
 		}
 
 		this.fragment = new FragmentWrapper(renderer, block, node.children, this, strip_whitespace, next_sibling);
-
-		if (this.slot_block) {
-			block.parent.add_dependencies(block.dependencies);
-
-			// appalling hack
-			const index = block.parent.wrappers.indexOf(this);
-			block.parent.wrappers.splice(index, 1);
-			block.wrappers.push(this);
-		}
 	}
 
 	render(block: Block, parent_node: Identifier, parent_nodes: Identifier) {
 		const { renderer } = this;
 
 		if (this.node.name === 'noscript') return;
 
-		if (this.slot_block) {
-			block = this.slot_block;
-		}
-
 		const node = this.var;
 		const nodes = parent_nodes && block.get_unique_name(`${this.var.name}_nodes`); // if we're in unclaimable territory, i.e. <head>, parent_nodes is null
 		const children = x`@children(${this.node.name === 'template' ? x`${node}.content` : node})`;

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

@@ -4,6 +4,7 @@ import Renderer from '../../Renderer';
 import Block from '../../Block';
 import InlineComponent from '../../../nodes/InlineComponent';
 import FragmentWrapper from '../Fragment';
+import SlotTemplateWrapper from '../SlotTemplate';
 import { sanitize } from '../../../../utils/names';
 import add_to_set from '../../../utils/add_to_set';
 import { b, x, p } from 'code-red';
@@ -19,11 +20,15 @@ import { extract_names } from 'periscopic';
 import mark_each_block_bindings from '../shared/mark_each_block_bindings';
 import { string_to_member_expression } from '../../../utils/string_to_member_expression';
 
+type SlotDefinition = { block: Block; scope: TemplateScope; get_context?: Node; get_changes?: Node };
+
 export default class InlineComponentWrapper extends Wrapper {
 	var: Identifier;
-	slots: Map<string, { block: Block; scope: TemplateScope; get_context?: Node; get_changes?: Node }> = new Map();
+	slots: Map<string, SlotDefinition> = new Map();
 	node: InlineComponent;
 	fragment: FragmentWrapper;
+	children: Array<Wrapper | FragmentWrapper> = [];
+	default_slot_block: Block;
 
 	constructor(
 		renderer: Renderer,
@@ -76,16 +81,35 @@ export default class InlineComponentWrapper extends Wrapper {
 				});
 			});
 
+			const children = this.node.children.slice();
+			for (let i=children.length - 1; i>=0;) {
+				const child = children[i];
+				if (child.type === 'SlotTemplate' || (child.type === 'Element' && child.attributes.find(attribute => attribute.name === 'slot'))) {
+					const slot_template = new SlotTemplateWrapper(renderer, block, this, child, strip_whitespace, next_sibling);
+					this.children.push(slot_template);
+					children.splice(i, 1);
+					continue;
+				}
+
+				i--;
+			}
+
+			if (this.slots.has('default') && children.filter(node => !(node.type === 'Text' && node.data.trim() === ''))) {
+				throw new Error('Found elements without slot attribute when using slot="default"');
+			}
+
 			const default_slot = block.child({
 				comment: create_debugging_comment(node, renderer.component),
 				name: renderer.component.get_unique_name(`create_default_slot`),
 				type: 'slot'
 			});
 
 			this.renderer.blocks.push(default_slot);
+			this.default_slot_block = default_slot;
 
 			this.slots.set('default', get_slot_definition(default_slot, this.node.scope, this.node.lets));
-			this.fragment = new FragmentWrapper(renderer, default_slot, node.children, this, strip_whitespace, next_sibling);
+			const fragment = new FragmentWrapper(renderer, default_slot, children, this, strip_whitespace, next_sibling);
+			this.children.push(fragment);
 
 			const dependencies: Set<string> = new Set();
 
@@ -102,6 +126,13 @@ export default class InlineComponentWrapper extends Wrapper {
 		block.add_outro();
 	}
 
+	set_slot(name: string, slot_definition: SlotDefinition) {
+		if (this.slots.has(name)) {
+			throw new Error(`Duplicate slot name "${name}" in <${this.node.name}>`);
+		}
+		this.slots.set(name, slot_definition);
+	}
+
 	warn_if_reactive() {
 		const { name } = this.node;
 		const variable = this.renderer.component.var_lookup.get(name);
@@ -135,14 +166,10 @@ export default class InlineComponentWrapper extends Wrapper {
 		const statements: Array<Node | Node[]> = [];
 		const updates: Array<Node | Node[]> = [];
 
-		if (this.fragment) {
+		this.children.forEach((child) => {
 			this.renderer.add_to_context('$$scope', true);
-			const default_slot = this.slots.get('default');
-
-			this.fragment.nodes.forEach((child) => {
-				child.render(default_slot.block, null, x`#nodes` as unknown as Identifier);
-			});
-		}
+			child.render(this.default_slot_block, null, x`#nodes` as Identifier);
+		});
 
 		let props;
 		const name_changes = block.get_unique_name(`${name.name}_changes`);
@@ -194,7 +221,7 @@ export default class InlineComponentWrapper extends Wrapper {
 			component_opts.properties.push(p`$$inline: true`);
 		}
 
-		const fragment_dependencies = new Set(this.fragment ? ['$$scope'] : []);
+		const fragment_dependencies = new Set(this.slots.size ? ['$$scope'] : []);
 		this.slots.forEach(slot => {
 			slot.block.dependencies.forEach(name => {
 				const is_let = slot.scope.is_let(name);

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

@@ -1,63 +1,77 @@
 import Wrapper from "./shared/Wrapper";
 import Renderer from "../Renderer";
 import Block from "../Block";
-import SlotTemplate from "../../nodes/SlotTemplate";
 import FragmentWrapper from "./Fragment";
 import create_debugging_comment from "./shared/create_debugging_comment";
-import { get_slot_definition } from './shared/get_slot_definition';
+import { get_slot_definition } from "./shared/get_slot_definition";
 import { x } from "code-red";
 import { sanitize } from "../../../utils/names";
 import { Identifier } from "estree";
 import InlineComponentWrapper from "./InlineComponent";
+import { extract_names } from "periscopic";
+import { INode } from "../../nodes/interfaces";
+import Let from "../../nodes/Let";
+import TemplateScope from "../../nodes/shared/TemplateScope";
 
-export default class SlotWrapper extends Wrapper {
-	node: SlotTemplate;
+type NodeWithLets = INode & {
+	scope: TemplateScope;
+	lets: Let[];
+	slot_template_name: string;
+};
+
+export default class SlotTemplateWrapper extends Wrapper {
+	node: NodeWithLets;
 	fragment: FragmentWrapper;
 	block: Block;
+	parent: InlineComponentWrapper;
 
 	constructor(
 		renderer: Renderer,
 		block: Block,
 		parent: Wrapper,
-		node: SlotTemplate,
+		node: NodeWithLets,
 		strip_whitespace: boolean,
 		next_sibling: Wrapper
 	) {
 		super(renderer, block, parent, node);
 
-		const { scope, lets, slot_name } = this.node;
+		const { scope, lets, slot_template_name } = this.node;
+
+		lets.forEach(l => {
+			extract_names(l.value || l.name).forEach(name => {
+				renderer.add_to_context(name, true);
+			});
+		});
 
 		this.block = block.child({
-			comment: create_debugging_comment(node, this.renderer.component),
+			comment: create_debugging_comment(this.node, this.renderer.component),
 			name: this.renderer.component.get_unique_name(
-				`create_${sanitize(slot_name)}_slot`
+				`create_${sanitize(slot_template_name)}_slot`
 			),
 			type: "slot"
 		});
 		this.renderer.blocks.push(this.block);
 
-		if (this.parent.node.type === "InlineComponent") {
-			const component = (this.parent as unknown) as InlineComponentWrapper;
-			const seen = new Set(lets.map(l => l.name.name));
-			component.node.lets.forEach(l => {
-				if (!seen.has(l.name.name)) lets.push(l);
-			});
-
-			component.slots.set(
-				slot_name,
-				get_slot_definition(this.block, scope, lets)
-			);
-		}
+		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);
+		});
 
-		this.fragment = new FragmentWrapper(renderer, this.block, node.children, this, strip_whitespace, next_sibling);
+		this.parent.set_slot(
+			slot_template_name,
+			get_slot_definition(this.block, scope, lets)
+		);
 
+		this.fragment = new FragmentWrapper(
+			renderer,
+			this.block,
+			node.type === "SlotTemplate" ? node.children : [node],
+			this,
+			strip_whitespace,
+			next_sibling
+		);
 
 		this.block.parent.add_dependencies(this.block.dependencies);
-
-		// appalling hack, copied from ElementWrapper
-		const index = this.block.parent.wrappers.indexOf(this);
-		this.block.parent.wrappers.splice(index, 1);
-		this.block.wrappers.push(this);
 	}
 
 	render() {

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

@@ -19,7 +19,7 @@ export default function(node: SlotTemplate, renderer: Renderer, options: RenderO
 		if (!seen.has(l.name.name)) lets.push(l);
 	});
 
-	options.slot_scopes.set(node.slot_name, {
+	options.slot_scopes.set(node.slot_template_name, {
 		input: get_slot_scope(node.lets),
 		output: renderer.pop()
 	});

test/parser/samples/error-svelte-selfdestructive/error.json

@@ -1,6 +1,6 @@
 {
 	"code": "invalid-tag-name",
-	"message": "Valid <svelte:...> tag names are svelte:head, svelte:options, svelte:window, svelte:body, svelte:self or svelte:component",
+	"message": "Valid <svelte:...> tag names are svelte:head, svelte:options, svelte:window, svelte:body, svelte:self, svelte:component or svelte:slot",
 	"pos": 10,
 	"start": {
 		"character": 10,

test/runtime/samples/component-slot-duplicate-error-2/Nested.svelte

@@ -0,0 +1 @@
+<slot value="Hi" />

test/runtime/samples/component-slot-duplicate-error-2/_config.js

@@ -0,0 +1,3 @@
+export default {
+	error: 'Duplicate slot name "foo" in <Nested>'
+};

test/runtime/samples/component-slot-duplicate-error-2/main.svelte

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