From d1a6f9c11e8771fe1dbae1aa67db9281362a5255 Mon Sep 17 00:00:00 2001
From: Rich Harris <richard.a.harris@gmail.com>
Date: Wed, 30 Aug 2017 11:42:40 -0400
Subject: [PATCH] only use comments around <slot> as necessary

---
 src/generators/dom/visitors/Slot.ts | 60 +++++++++++++++++++++--------
 src/shared/dom.js                   | 13 +++++++
 2 files changed, 57 insertions(+), 16 deletions(-)

diff --git a/src/generators/dom/visitors/Slot.ts b/src/generators/dom/visitors/Slot.ts
index 85f53f2ca3..5249879f40 100644
--- a/src/generators/dom/visitors/Slot.ts
+++ b/src/generators/dom/visitors/Slot.ts
@@ -20,11 +20,19 @@ export default function visitSlot(
 	const content_name = block.getUniqueName(`slot_content_${slotName}`);
 	block.addVariable(content_name, `#component._slotted.${slotName}`);
 
-	// TODO use surrounds as anchors where possible, a la if/each blocks
-	const before = block.getUniqueName(`${content_name}_before`);
-	const after = block.getUniqueName(`${content_name}_after`);
-	block.addVariable(before);
-	block.addVariable(after);
+	const needsAnchorBefore = node.prev ? node.prev.type !== 'Element' : !state.parentNode;
+	const needsAnchorAfter = node.next ? node.next.type !== 'Element' : !state.parentNode;
+
+	const anchorBefore = needsAnchorBefore
+		? block.getUniqueName(`${content_name}_before`)
+		: (node.prev && node.prev.var) || 'null';
+
+	const anchorAfter = needsAnchorAfter
+		? block.getUniqueName(`${content_name}_after`)
+		: (node.next && node.next.var) || 'null';
+
+	if (needsAnchorBefore) block.addVariable(anchorBefore);
+	if (needsAnchorAfter) block.addVariable(anchorAfter);
 
 	block.builders.create.pushCondition(`!${content_name}`);
 	block.builders.hydrate.pushCondition(`!${content_name}`);
@@ -46,17 +54,17 @@ export default function visitSlot(
 	if (state.parentNode) {
 		block.builders.mount.addBlock(deindent`
 			if (${content_name}) {
-				@appendNode(${before} || (${before} = @createComment()), ${state.parentNode});
+				${needsAnchorBefore && `@appendNode(${anchorBefore} || (${anchorBefore} = @createComment()), ${state.parentNode});`}
 				@appendNode(${content_name}, ${state.parentNode});
-				@appendNode(${after} || (${after} = @createComment()), ${state.parentNode});
+				${needsAnchorAfter && `@appendNode(${anchorAfter} || (${anchorAfter} = @createComment()), ${state.parentNode});`}
 			}
 		`);
 	} else {
 		block.builders.mount.addBlock(deindent`
 			if (${content_name}) {
-				@insertNode(${before} || (${before} = @createComment()), #target, anchor);
+				${needsAnchorBefore && `@insertNode(${anchorBefore} || (${anchorBefore} = @createComment()), #target, anchor);`}
 				@insertNode(${content_name}, #target, anchor);
-				@insertNode(${after} || (${after} = @createComment()), #target, anchor);
+				${needsAnchorAfter && `@insertNode(${anchorAfter} || (${anchorAfter} = @createComment()), #target, anchor);`}
 			}
 		`);
 	}
@@ -66,11 +74,31 @@ export default function visitSlot(
 	// TODO so that this can work with public API, component._slotted should
 	// be all fragments, derived from options.slots. Not === options.slots
 	// TODO can we use an else here?
-	block.builders.unmount.addBlock(deindent`
-		if (${content_name}) {
-			@reinsertBetween(${before}, ${after}, ${content_name});
-			@detachNode(${before});
-			@detachNode(${after});
-		}
-	`);
+	if (anchorBefore === 'null' && anchorAfter === 'null') {
+		block.builders.unmount.addBlock(deindent`
+			if (${content_name}) {
+				@reinsertChildren(${state.parentNode}, ${content_name});
+			}
+		`);
+	} else if (anchorBefore === 'null') {
+		block.builders.unmount.addBlock(deindent`
+			if (${content_name}) {
+				@reinsertBefore(${anchorAfter}, ${content_name});
+			}
+		`);
+	} else if (anchorAfter === 'null') {
+		block.builders.unmount.addBlock(deindent`
+			if (${content_name}) {
+				@reinsertAfter(${anchorBefore}, ${content_name});
+			}
+		`);
+	} else {
+		block.builders.unmount.addBlock(deindent`
+			if (${content_name}) {
+				@reinsertBetween(${anchorBefore}, ${anchorAfter}, ${content_name});
+				@detachNode(${anchorBefore});
+				@detachNode(${anchorAfter});
+			}
+		`);
+	}
 }
diff --git a/src/shared/dom.js b/src/shared/dom.js
index 86da87c726..0372d96fdc 100644
--- a/src/shared/dom.js
+++ b/src/shared/dom.js
@@ -34,6 +34,19 @@ export function reinsertBetween(before, after, target) {
 	}
 }
 
+export function reinsertChildren(parent, target) {
+	while (parent.firstChild) target.appendChild(parent.firstChild);
+}
+
+export function reinsertAfter(before, target) {
+	while (before.nextSibling) target.appendChild(before.nextSibling);
+}
+
+export function reinsertBefore(after, target) {
+	var parent = after.parentNode;
+	while (parent.firstChild !== after) target.appendChild(parent.firstChild);
+}
+
 // TODO this is out of date
 export function destroyEach(iterations, detach, start) {
 	for (var i = start; i < iterations.length; i += 1) {