fix render fallback slot content due to whitespace (#4500)

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 * In `vars` array, correctly indicate whether `module` variables are `mutated` or `reassigned` ([#3215](https://github.com/sveltejs/svelte/issues/3215))
 * Fix spread props not updating in certain situations ([#3521](https://github.com/sveltejs/svelte/issues/3521), [#4480](https://github.com/sveltejs/svelte/issues/4480))
+* Use the fallback content for slots if they are passed only whitespace ([#4092](https://github.com/sveltejs/svelte/issues/4092))
 * In `dev` mode, check for unknown props even if the component has no writable props ([#4323](https://github.com/sveltejs/svelte/issues/4323))
 * Exclude global variables from `$capture_state` ([#4463](https://github.com/sveltejs/svelte/issues/4463))
 * Fix bitmask overflow for slots ([#4481](https://github.com/sveltejs/svelte/issues/4481))

src/compiler/compile/nodes/Text.ts

@@ -3,6 +3,18 @@ import Component from '../Component';
 import TemplateScope from './shared/TemplateScope';
 import { INode } from './interfaces';
 
+// Whitespace inside one of these elements will not result in
+// a whitespace node being created in any circumstances. (This
+// list is almost certainly very incomplete)
+const elements_without_text = new Set([
+	'audio',
+	'datalist',
+	'dl',
+	'optgroup',
+	'select',
+	'video',
+]);
+
 export default class Text extends Node {
 	type: 'Text';
 	data: string;
@@ -13,4 +25,21 @@ export default class Text extends Node {
 		this.data = info.data;
 		this.synthetic = info.synthetic || false;
 	}
+
+	should_skip() {
+		if (/\S/.test(this.data)) return false;
+
+		const parent_element = this.find_nearest(/(?:Element|InlineComponent|Head)/);
+		if (!parent_element) return false;
+
+		if (parent_element.type === 'Head') return true;
+		if (parent_element.type === 'InlineComponent') return parent_element.children.length === 1 && this === parent_element.children[0];
+
+		// svg namespace exclusions
+		if (/svg$/.test(parent_element.namespace)) {
+			if (this.prev && this.prev.type === "Element" && this.prev.name === "tspan") return false;
+		}
+
+		return parent_element.namespace || elements_without_text.has(parent_element.name);
+	}
 }

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

@@ -17,6 +17,7 @@ import { INode } from '../../nodes/interfaces';
 import Renderer from '../Renderer';
 import Block from '../Block';
 import { trim_start, trim_end } from '../../../utils/trim';
+import { link } from '../../../utils/link';
 import { Identifier } from 'estree';
 
 const wrappers = {
@@ -38,11 +39,6 @@ const wrappers = {
 	Window
 };
 
-function link(next: Wrapper, prev: Wrapper) {
-	prev.next = next;
-	if (next) next.prev = prev;
-}
-
 function trimmable_at(child: INode, next_sibling: Wrapper): boolean {
 	// Whitespace is trimmable if one of the following is true:
 	// The child and its sibling share a common nearest each block (not at an each block boundary)

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

@@ -138,11 +138,27 @@ export default class InlineComponentWrapper extends Wrapper {
 		const statements: Array<Node | Node[]> = [];
 		const updates: Array<Node | Node[]> = [];
 
+		if (this.fragment) {
+			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);
+			});
+		}
+
 		let props;
 		const name_changes = block.get_unique_name(`${name.name}_changes`);
 
 		const uses_spread = !!this.node.attributes.find(a => a.is_spread);
 
+		// removing empty slot
+		for (const slot of this.slots.keys()) {
+			if (!this.slots.get(slot).block.has_content()) {
+				this.slots.delete(slot);
+			}
+		}
+
 		const initial_props = this.slots.size > 0
 			? [
 				p`$$slots: {
@@ -172,15 +188,6 @@ export default class InlineComponentWrapper extends Wrapper {
 			}
 		}
 
-		if (this.fragment) {
-			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);
-			});
-		}
-
 		if (component.compile_options.dev) {
 			// TODO this is a terrible hack, but without it the component
 			// will complain that options.target is missing. This would

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

@@ -5,36 +5,6 @@ import Wrapper from './shared/Wrapper';
 import { x } from 'code-red';
 import { Identifier } from 'estree';
 
-// Whitespace inside one of these elements will not result in
-// a whitespace node being created in any circumstances. (This
-// list is almost certainly very incomplete)
-const elements_without_text = new Set([
-	'audio',
-	'datalist',
-	'dl',
-	'optgroup',
-	'select',
-	'video',
-]);
-
-// TODO this should probably be in Fragment
-function should_skip(node: Text) {
-	if (/\S/.test(node.data)) return false;
-
-	const parent_element = node.find_nearest(/(?:Element|InlineComponent|Head)/);
-	if (!parent_element) return false;
-
-	if (parent_element.type === 'Head') return true;
-	if (parent_element.type === 'InlineComponent') return parent_element.children.length === 1 && node === parent_element.children[0];
-
-	// svg namespace exclusions
-	if (/svg$/.test(parent_element.namespace)) {
-		if (node.prev && node.prev.type === "Element" && node.prev.name === "tspan") return false;
-	}
-
-	return parent_element.namespace || elements_without_text.has(parent_element.name);
-}
-
 export default class TextWrapper extends Wrapper {
 	node: Text;
 	data: string;
@@ -50,7 +20,7 @@ export default class TextWrapper extends Wrapper {
 	) {
 		super(renderer, block, parent, node);
 
-		this.skip = should_skip(this.node);
+		this.skip = this.node.should_skip();
 		this.data = data;
 		this.var = (this.skip ? null : x`t`) as unknown as Identifier;
 	}

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

@@ -6,10 +6,14 @@ import Renderer, { RenderOptions } from '../Renderer';
 import Element from '../../nodes/Element';
 import { x } from 'code-red';
 import Expression from '../../nodes/shared/Expression';
+import remove_whitespace_children from './utils/remove_whitespace_children';
 
 export default function(node: Element, renderer: Renderer, options: RenderOptions & {
 	slot_scopes: Map<any, any>;
 }) {
+
+	const children = remove_whitespace_children(node.children, node.next);
+
 	// awkward special case
 	let node_contents;
 
@@ -133,7 +137,7 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
 	if (node_contents !== undefined) {
 		if (contenteditable) {
 			renderer.push();
-			renderer.render(node.children, options);
+			renderer.render(children, options);
 			const result = renderer.pop();
 
 			renderer.add_expression(x`($$value => $$value === void 0 ? ${result} : $$value)(${node_contents})`);
@@ -145,7 +149,7 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
 			renderer.add_string(`</${node.name}>`);
 		}
 	} else if (slot && nearest_inline_component) {
-		renderer.render(node.children, options);
+		renderer.render(children, options);
 
 		if (!is_void(node.name)) {
 			renderer.add_string(`</${node.name}>`);
@@ -163,10 +167,11 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
 			output: renderer.pop()
 		});
 	} else {
-		renderer.render(node.children, options);
+		renderer.render(children, options);
 
 		if (!is_void(node.name)) {
 			renderer.add_string(`</${node.name}>`);
 		}
 	}
 }
+

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

@@ -2,6 +2,7 @@ import { string_literal } from '../../utils/stringify';
 import Renderer, { RenderOptions } from '../Renderer';
 import { get_slot_scope } from './shared/get_slot_scope';
 import InlineComponent from '../../nodes/InlineComponent';
+import remove_whitespace_children from './utils/remove_whitespace_children';
 import { p, x } from 'code-red';
 
 function get_prop_value(attribute) {
@@ -67,12 +68,14 @@ export default function(node: InlineComponent, renderer: Renderer, options: Rend
 
 	const slot_fns = [];
 
-	if (node.children.length) {
+	const children = remove_whitespace_children(node.children, node.next);
+
+	if (children.length) {
 		const slot_scopes = new Map();
 
 		renderer.push();
 
-		renderer.render(node.children, Object.assign({}, options, {
+		renderer.render(children, Object.assign({}, options, {
 			slot_scopes
 		}));
 
@@ -82,9 +85,11 @@ export default function(node: InlineComponent, renderer: Renderer, options: Rend
 		});
 
 		slot_scopes.forEach(({ input, output }, name) => {
-			slot_fns.push(
-				p`${name}: (${input}) => ${output}`
-			);
+			if (!is_empty_template_literal(output)) {
+				slot_fns.push(
+					p`${name}: (${input}) => ${output}`
+				);
+			}
 		});
 	}
 
@@ -94,3 +99,11 @@ export default function(node: InlineComponent, renderer: Renderer, options: Rend
 
 	renderer.add_expression(x`@validate_component(${expression}, "${node.name}").$$render($$result, ${props}, ${bindings}, ${slots})`);
 }
+
+function is_empty_template_literal(template_literal) {
+	return (
+		template_literal.expressions.length === 0 &&
+		template_literal.quasis.length === 1 &&
+		template_literal.quasis[0].value.raw === ""
+	);
+}

src/compiler/compile/render_ssr/handlers/utils/remove_whitespace_children.ts

@@ -0,0 +1,73 @@
+import { INode } from '../../../nodes/interfaces';
+import { trim_end, trim_start } from '../../../../utils/trim';
+import { link } from '../../../../utils/link';
+
+// similar logic from `compile/render_dom/wrappers/Fragment`
+// We want to remove trailing whitespace inside an element/component/block,
+// *unless* there is no whitespace between this node and its next sibling
+export default function remove_whitespace_children(children: INode[], next?: INode): INode[] {
+	const nodes: INode[] = [];
+	let last_child: INode;
+	let i = children.length;
+	while (i--) {
+		const child = children[i];
+
+		if (child.type === 'Text') {
+			if (child.should_skip()) {
+				continue;
+			}
+
+			let { data } = child;
+
+			if (nodes.length === 0) {
+				const should_trim = next
+					? next.type === 'Text' &&
+					  /^\s/.test(next.data) &&
+					  trimmable_at(child, next)
+					: !child.has_ancestor('EachBlock');
+
+				if (should_trim) {
+					data = trim_end(data);
+					if (!data) continue;
+				}
+			}
+
+			// glue text nodes (which could e.g. be separated by comments) together
+			if (last_child && last_child.type === 'Text') {
+				last_child.data = data + last_child.data;
+				continue;
+			}
+
+			nodes.unshift(child);
+			link(last_child, last_child = child);
+		} else {
+			nodes.unshift(child);
+			link(last_child, last_child = child);
+		}
+	}
+
+	const first = nodes[0];
+	if (first && first.type === 'Text') {
+		first.data = trim_start(first.data);
+		if (!first.data) {
+			first.var = null;
+			nodes.shift();
+
+			if (nodes[0]) {
+				nodes[0].prev = null;
+			}
+		}
+	}
+
+	return nodes;
+}
+
+function trimmable_at(child: INode, next_sibling: INode): boolean {
+	// Whitespace is trimmable if one of the following is true:
+	// The child and its sibling share a common nearest each block (not at an each block boundary)
+	// The next sibling's previous node is an each block
+	return (
+		next_sibling.find_nearest(/EachBlock/) ===
+			child.find_nearest(/EachBlock/) || next_sibling.prev.type === 'EachBlock'
+	);
+}

src/compiler/utils/link.ts

@@ -0,0 +1,4 @@
+export function link<T extends { next?: T; prev?: T }>(next: T, prev: T) {
+	prev.next = next;
+	if (next) next.prev = prev;
+}

test/runtime/samples/component-slot-fallback-empty/Nested.svelte

@@ -0,0 +1,4 @@
+<div>
+	<slot><p class='default'>default fallback content</p></slot>
+	<slot name='bar'>bar fallback</slot>
+</div>

test/runtime/samples/component-slot-fallback-empty/_config.js

@@ -0,0 +1,13 @@
+export default {
+	html: `
+	<div>
+		<p class="default">default fallback content</p>
+		<input slot="bar">
+	</div>
+	
+	<div>
+		<p class="default">default fallback content</p>
+		bar fallback
+	</div>
+	`
+};

test/runtime/samples/component-slot-fallback-empty/main.svelte

@@ -0,0 +1,10 @@
+<script>
+	import Nested from "./Nested.svelte";
+</script>
+
+<Nested>
+	<input slot="bar">
+</Nested>
+
+<Nested>
+</Nested>