Merge pull request #626 from sveltejs/gh-625

Unmount `yield` fragments in parent's unmount method, not destroy

package.json

@@ -57,6 +57,7 @@
     "babel-plugin-transform-es2015-spread": "^6.22.0",
     "babel-preset-env": "^1.2.1",
     "babel-register": "^6.23.0",
+    "chalk": "^1.1.3",
     "codecov": "^1.0.1",
     "console-group": "^0.3.2",
     "css-tree": "1.0.0-alpha16",

src/generators/dom/visitors/YieldTag.ts

@@ -13,7 +13,7 @@ export default function visitYieldTag(
 		`if ( ${block.component}._yield ) ${block.component}._yield.mount( ${parentNode}, null );`
 	);
 
-	block.builders.destroy.addLine(
+	block.builders.unmount.addLine(
 		`if ( ${block.component}._yield ) ${block.component}._yield.unmount();`
 	);
 }

test/helpers.js

@@ -1,11 +1,13 @@
-import jsdom from "jsdom";
-import assert from "assert";
-import * as fs from "fs";
+import jsdom from 'jsdom';
+import assert from 'assert';
+import glob from 'glob';
+import fs from 'fs';
+import chalk from 'chalk';
 
-import * as consoleGroup from "console-group";
+import * as consoleGroup from 'console-group';
 consoleGroup.install();
 
-import * as sourceMapSupport from "source-map-support";
+import * as sourceMapSupport from 'source-map-support';
 sourceMapSupport.install();
 
 // for coverage purposes, we need to test source files,
@@ -14,8 +16,8 @@ export function loadSvelte(test) {
 	if (test) global.__svelte_test = true;
 
 	const resolved = process.env.COVERAGE
-		? require.resolve("../src/index.js")
-		: require.resolve("../compiler/svelte.js");
+		? require.resolve('../src/index.js')
+		: require.resolve('../compiler/svelte.js');
 
 	delete require.cache[resolved];
 	return require(resolved);
@@ -36,23 +38,23 @@ export function tryToLoadJson(file) {
 	try {
 		return JSON.parse(fs.readFileSync(file));
 	} catch (err) {
-		if (err.code !== "ENOENT") throw err;
+		if (err.code !== 'ENOENT') throw err;
 		return null;
 	}
 }
 
 export function tryToReadFile(file) {
 	try {
-		return fs.readFileSync(file, "utf-8");
+		return fs.readFileSync(file, 'utf-8');
 	} catch (err) {
-		if (err.code !== "ENOENT") throw err;
+		if (err.code !== 'ENOENT') throw err;
 		return null;
 	}
 }
 
 export function env() {
 	return new Promise((fulfil, reject) => {
-		jsdom.env("<main></main>", (err, window) => {
+		jsdom.env('<main></main>', (err, window) => {
 			if (err) {
 				reject(err);
 			} else {
@@ -75,19 +77,19 @@ function cleanChildren(node) {
 
 		if (child.nodeType === 3) {
 			if (
-				node.namespaceURI === "http://www.w3.org/2000/svg" &&
-				node.tagName !== "text" &&
-				node.tagName !== "tspan"
+				node.namespaceURI === 'http://www.w3.org/2000/svg' &&
+				node.tagName !== 'text' &&
+				node.tagName !== 'tspan'
 			) {
 				node.removeChild(child);
 			}
 
-			child.data = child.data.replace(/\s{2,}/, "\n");
+			child.data = child.data.replace(/\s{2,}/, '\n');
 
 			// text
 			if (previous && previous.nodeType === 3) {
 				previous.data += child.data;
-				previous.data = previous.data.replace(/\s{2,}/, "\n");
+				previous.data = previous.data.replace(/\s{2,}/, '\n');
 
 				node.removeChild(child);
 				child = previous;
@@ -101,12 +103,12 @@ function cleanChildren(node) {
 
 	// collapse whitespace
 	if (node.firstChild && node.firstChild.nodeType === 3) {
-		node.firstChild.data = node.firstChild.data.replace(/^\s+/, "");
+		node.firstChild.data = node.firstChild.data.replace(/^\s+/, '');
 		if (!node.firstChild.data) node.removeChild(node.firstChild);
 	}
 
 	if (node.lastChild && node.lastChild.nodeType === 3) {
-		node.lastChild.data = node.lastChild.data.replace(/\s+$/, "");
+		node.lastChild.data = node.lastChild.data.replace(/\s+$/, '');
 		if (!node.lastChild.data) node.removeChild(node.lastChild);
 	}
 }
@@ -115,15 +117,15 @@ export function setupHtmlEqual() {
 	return env().then(window => {
 		assert.htmlEqual = (actual, expected, message) => {
 			window.document.body.innerHTML = actual
-				.replace(/>[\s\r\n]+</g, "><")
+				.replace(/>[\s\r\n]+</g, '><')
 				.trim();
-			cleanChildren(window.document.body, "");
+			cleanChildren(window.document.body, '');
 			actual = window.document.body.innerHTML;
 
 			window.document.body.innerHTML = expected
-				.replace(/>[\s\r\n]+</g, "><")
+				.replace(/>[\s\r\n]+</g, '><')
 				.trim();
-			cleanChildren(window.document.body, "");
+			cleanChildren(window.document.body, '');
 			expected = window.document.body.innerHTML;
 
 			assert.deepEqual(actual, expected, message);
@@ -137,7 +139,7 @@ export function loadConfig(file) {
 		delete require.cache[resolved];
 		return require(resolved).default;
 	} catch (err) {
-		if (err.code === "E_NOT_FOUND") {
+		if (err.code === 'E_NOT_FOUND') {
 			return {};
 		}
 
@@ -147,16 +149,32 @@ export function loadConfig(file) {
 
 export function addLineNumbers(code) {
 	return code
-		.split("\n")
+		.split('\n')
 		.map((line, i) => {
 			i = String(i + 1);
 			while (i.length < 3) i = ` ${i}`;
 
-			return `${i}: ${line.replace(/^\t+/, match =>
-				match.split("\t").join("    ")
-			)}`;
+			return (
+				chalk.grey(`  ${i}: `) +
+				line.replace(/^\t+/, match => match.split('\t').join('    '))
+			);
 		})
-		.join("\n");
+		.join('\n');
+}
+
+export function showOutput(cwd, shared) {
+	glob.sync('**/*.html', { cwd }).forEach(file => {
+		const { code } = svelte.compile(
+			fs.readFileSync(`${cwd}/${file}`, 'utf-8'),
+			{
+				shared
+			}
+		);
+
+		console.log( // eslint-disable-line no-console
+			`\n>> ${chalk.cyan.bold(file)}\n${addLineNumbers(code)}\n<< ${chalk.cyan.bold(file)}`
+		);
+	});
 }
 
 const start = /\n(\t+)/;

test/runtime/index.js

@@ -2,11 +2,10 @@ import assert from "assert";
 import * as path from "path";
 import * as fs from "fs";
 import * as acorn from "acorn";
-import * as babel from "babel-core";
 import { transitionManager } from "../../shared.js";
 
 import {
-	addLineNumbers,
+	showOutput,
 	loadConfig,
 	loadSvelte,
 	env,
@@ -16,7 +15,6 @@ import {
 
 let svelte;
 
-let showCompiledCode = false;
 let compileOptions = null;
 
 function getName(filename) {
@@ -35,9 +33,7 @@ require.extensions[".html"] = function(module, filename) {
 	);
 	let { code } = svelte.compile(fs.readFileSync(filename, "utf-8"), options);
 
-	if (showCompiledCode) console.log(addLineNumbers(code)); // eslint-disable-line no-console
-
-	if (legacy) code = babel.transform(code, babelrc).code;
+	if (legacy) code = require('babel-core').transform(code, babelrc).code;
 
 	return module._compile(code, filename);
 };
@@ -50,6 +46,8 @@ describe("runtime", () => {
 		return setupHtmlEqual();
 	});
 
+	const failed = new Set();
+
 	function runTest(dir, shared) {
 		if (dir[0] === ".") return;
 
@@ -59,10 +57,15 @@ describe("runtime", () => {
 			throw new Error("Forgot to remove `solo: true` from test");
 		}
 
-		(config.skip ? it.skip : config.solo ? it.only : it)(dir, () => {
+		(config.skip ? it.skip : config.solo ? it.only : it)(`${dir} (${shared ? 'shared' : 'inline'} helpers`, () => {
+			if (failed.has(dir)) {
+				// this makes debugging easier, by only printing compiled output once
+				throw new Error('skipping inline helpers test');
+			}
+
+			const cwd = path.resolve(`test/runtime/samples/${dir}`);
 			let compiled;
 
-			showCompiledCode = config.show;
 			compileOptions = config.compileOptions || {};
 			compileOptions.shared = shared;
 			compileOptions.dev = config.dev;
@@ -78,6 +81,8 @@ describe("runtime", () => {
 					config.compileError(err);
 					return;
 				} else {
+					failed.add(dir);
+					showOutput(cwd, shared);
 					throw err;
 				}
 			}
@@ -91,11 +96,12 @@ describe("runtime", () => {
 					if (startIndex === -1)
 						throw new Error("missing create_main_fragment");
 					const es5 =
-						spaces(startIndex) +
+						code.slice(0, startIndex).split('\n').map(x => spaces(x.length)).join('\n') +
 						code.slice(startIndex).replace(/export default .+/, "");
 					acorn.parse(es5, { ecmaVersion: 5 });
 				} catch (err) {
-					if (!config.show) console.log(addLineNumbers(code)); // eslint-disable-line no-console
+					failed.add(dir);
+					showOutput(cwd, shared); // eslint-disable-line no-console
 					throw err;
 				}
 			}
@@ -140,7 +146,7 @@ describe("runtime", () => {
 					try {
 						SvelteComponent = require(`./samples/${dir}/main.html`).default;
 					} catch (err) {
-						if (!config.show) console.log(addLineNumbers(code)); // eslint-disable-line no-console
+						showOutput(cwd, shared); // eslint-disable-line no-console
 						throw err;
 					}
 
@@ -203,24 +209,21 @@ describe("runtime", () => {
 					if (config.error && !unintendedError) {
 						config.error(assert, err);
 					} else {
-						if (!config.show) console.log(addLineNumbers(code)); // eslint-disable-line no-console
+						failed.add(dir);
+						showOutput(cwd, shared); // eslint-disable-line no-console
 						throw err;
 					}
+				})
+				.then(() => {
+					if (config.show) showOutput(cwd, shared);
 				});
 		});
 	}
 
-	describe("inline helpers", () => {
-		fs.readdirSync("test/runtime/samples").forEach(dir => {
-			runTest(dir, null);
-		});
-	});
-
 	const shared = path.resolve("shared.js");
-	describe("shared helpers", () => {
-		fs.readdirSync("test/runtime/samples").forEach(dir => {
-			runTest(dir, shared);
-		});
+	fs.readdirSync("test/runtime/samples").forEach(dir => {
+		runTest(dir, shared);
+		runTest(dir, null);
 	});
 
 	it("fails if options.target is missing in dev mode", () => {

test/runtime/samples/component-nested-deep/Level1.html

@@ -0,0 +1,15 @@
+<Level2>
+	<Level3 />
+</Level2>
+
+<script>
+	import Level2 from './Level2.html';
+	import Level3 from './Level3.html';
+
+	export default {
+		components: {
+			Level2,
+			Level3
+		}
+	}
+</script>

test/runtime/samples/component-nested-deep/Level2.html

@@ -0,0 +1,12 @@
+<span>level 2</span>
+{{yield}}
+
+<script>
+	import Level3 from './Level3.html';
+
+	export default {
+		components: {
+			Level3
+		}
+	};
+</script>

test/runtime/samples/component-nested-deep/Level3.html

@@ -0,0 +1 @@
+<span>level 3</span>

test/runtime/samples/component-nested-deep/_config.js

@@ -0,0 +1,5 @@
+export default {
+	test(assert, component) {
+		component.refs.l1.destroy();
+	}
+};

test/runtime/samples/component-nested-deep/main.html

@@ -0,0 +1,11 @@
+<Level1 ref:l1 />
+
+<script>
+	import Level1 from './Level1.html';
+
+	export default {
+		components: {
+			Level1
+		}
+	}
+</script>