Merge pull request #674 from sveltejs/gh-655

deconflict name with imports
sveltejs · Jun 25, 2017 · b1d1cea · b1d1cea
2 parents e582691 + 6751a83
commit b1d1cea
Show file tree

Hide file tree

Showing 10 changed files with 63 additions and 50 deletions.
diff --git a/src/generators/Generator.ts b/src/generators/Generator.ts
@@ -54,7 +54,6 @@ export default class Generator {
 
 		this.parsed = parsed;
 		this.source = source;
-		this.name = name;
 		this.options = options;
 
 		this.imports = [];
@@ -81,7 +80,10 @@ export default class Generator {
 		// Svelte's builtin `import { get, ... } from 'svelte/shared.ts'`;
 		this.importedNames = new Set();
 		this.aliases = new Map();
-		this.usedNames = new Set([name]);
+		this.usedNames = new Set();
+
+		this.parseJs();
+		this.name = this.alias(name);
 	}
 
 	addSourcemapLocations(node: Node) {
@@ -389,7 +391,7 @@ export default class Generator {
 		};
 	}
 
-	parseJs(ssr: boolean = false) {
+	parseJs() {
 		const { source } = this;
 		const { js } = this.parsed;
 
@@ -417,7 +419,7 @@ export default class Generator {
 				}
 			}
 
-			const defaultExport = body.find(
+			const defaultExport = this.defaultExport = body.find(
 				(node: Node) => node.type === 'ExportDefaultDeclaration'
 			);
 
@@ -525,34 +527,6 @@ export default class Generator {
 				templateProperties.ondestroy = templateProperties.onteardown;
 			}
 
-			// in an SSR context, we don't need to include events, methods, oncreate or ondestroy
-			if (ssr) {
-				if (templateProperties.oncreate)
-					removeNode(
-						this.code,
-						defaultExport.declaration,
-						templateProperties.oncreate
-					);
-				if (templateProperties.ondestroy)
-					removeNode(
-						this.code,
-						defaultExport.declaration,
-						templateProperties.ondestroy
-					);
-				if (templateProperties.methods)
-					removeNode(
-						this.code,
-						defaultExport.declaration,
-						templateProperties.methods
-					);
-				if (templateProperties.events)
-					removeNode(
-						this.code,
-						defaultExport.declaration,
-						templateProperties.events
-					);
-			}
-
 			// now that we've analysed the default export, we can determine whether or not we need to keep it
 			let hasDefaultExport = !!defaultExport;
 			if (defaultExport && defaultExport.declaration.properties.length === 0) {
@@ -611,11 +585,9 @@ export default class Generator {
 			}
 		}
 
-		return {
-			computations,
-			hasJs,
-			namespace,
-			templateProperties,
-		};
+		this.computations = computations;
+		this.hasJs = hasJs;
+		this.namespace = namespace;
+		this.templateProperties = templateProperties;
 	}
 }
diff --git a/src/generators/dom/index.ts b/src/generators/dom/index.ts
@@ -59,16 +59,16 @@ export default function dom(
 	options: CompileOptions
 ) {
 	const format = options.format || 'es';
-	const name = options.name || 'SvelteComponent';
 
-	const generator = new DomGenerator(parsed, source, name, options);
+	const generator = new DomGenerator(parsed, source, options.name || 'SvelteComponent', options);
 
 	const {
 		computations,
 		hasJs,
+		name,
 		templateProperties,
 		namespace,
-	} = generator.parseJs();
+	} = generator;
 
 	const { block, state } = preprocess(generator, namespace, parsed.html);
 

diff --git a/src/generators/server-side-rendering/index.ts b/src/generators/server-side-rendering/index.ts
@@ -2,6 +2,7 @@ import deindent from '../../utils/deindent';
 import Generator from '../Generator';
 import Block from './Block';
 import visit from './visit';
+import { removeNode, removeObjectKey } from '../../utils/removeNode';
 import { Parsed, Node, CompileOptions } from '../../interfaces';
 
 export class SsrGenerator extends Generator {
@@ -19,6 +20,34 @@ export class SsrGenerator extends Generator {
 		this.bindings = [];
 		this.renderCode = '';
 		this.elementDepth = 0;
+
+		// in an SSR context, we don't need to include events, methods, oncreate or ondestroy
+		const { templateProperties, defaultExport } = this;
+
+		if (templateProperties.oncreate)
+			removeNode(
+				this.code,
+				defaultExport.declaration,
+				templateProperties.oncreate
+			);
+		if (templateProperties.ondestroy)
+			removeNode(
+				this.code,
+				defaultExport.declaration,
+				templateProperties.ondestroy
+			);
+		if (templateProperties.methods)
+			removeNode(
+				this.code,
+				defaultExport.declaration,
+				templateProperties.methods
+			);
+		if (templateProperties.events)
+			removeNode(
+				this.code,
+				defaultExport.declaration,
+				templateProperties.events
+			);
 	}
 
 	append(code: string) {
@@ -32,11 +61,10 @@ export default function ssr(
 	options: CompileOptions
 ) {
 	const format = options.format || 'cjs';
-	const name = options.name || 'SvelteComponent';
 
-	const generator = new SsrGenerator(parsed, source, name, options);
+	const generator = new SsrGenerator(parsed, source, options.name || 'SvelteComponent', options);
 
-	const { computations, hasJs, templateProperties } = generator.parseJs(true);
+	const { computations, name, hasJs, templateProperties } = generator;
 
 	// create main render() function
 	const mainBlock = new Block({

diff --git a/src/validate/index.ts b/src/validate/index.ts
@@ -78,7 +78,7 @@ export default function validate(
 
 	try {
 		if (name && !/^[a-zA-Z_$][a-zA-Z_$0-9]*$/.test(name)) {
-			const error = new Error(`options.name must be a valid identifier`);
+			const error = new Error(`options.name must be a valid identifier (got '${name}')`);
 			throw error;
 		}
 

diff --git a/test/helpers.js b/test/helpers.js
@@ -163,7 +163,7 @@ export function addLineNumbers(code) {
 		.join('\n');
 }
 
-function capitalize(str) {
+function capitalise(str) {
 	return str[0].toUpperCase() + str.slice(1);
 }
 
@@ -173,7 +173,8 @@ export function showOutput(cwd, options = {}) {
 		const { code } = svelte.compile(
 			fs.readFileSync(`${cwd}/${file}`, 'utf-8'),
 			Object.assign(options, {
-				name: capitalize(file.slice(0, -path.extname(file).length))
+				filename: file,
+				name: capitalise(path.basename(file).replace(/\.html$/, ''))
 			})
 		);
 

diff --git a/test/runtime/samples/deconflict-self/_config.js b/test/runtime/samples/deconflict-self/_config.js
@@ -0,0 +1,3 @@
+export default {
+	html: `<p>nested component</p>`
+};
diff --git a/test/runtime/samples/deconflict-self/main.html b/test/runtime/samples/deconflict-self/main.html
@@ -0,0 +1,9 @@
+<Main/>
+
+<script>
+	import Main from './nested/main.html';
+
+	export default {
+		components: { Main }
+	};
+</script>
diff --git a/test/runtime/samples/deconflict-self/nested/main.html b/test/runtime/samples/deconflict-self/nested/main.html
@@ -0,0 +1 @@
+<p>nested component</p>
diff --git a/test/runtime/samples/self-reference/_config.js b/test/runtime/samples/self-reference/_config.js
@@ -1,6 +1,4 @@
 export default {
-	// solo: true,
-
 	data: {
 		depth: 5
 	},

diff --git a/test/server-side-rendering/index.js b/test/server-side-rendering/index.js
@@ -1,6 +1,7 @@
 import assert from "assert";
 import * as fs from "fs";
 import * as path from "path";
+import glob from 'glob';
 
 import {
 	showOutput,
@@ -88,7 +89,7 @@ describe("ssr", () => {
 		(config.skip ? it.skip : config.solo ? it.only : it)(dir, () => {
 			const cwd = path.resolve("test/runtime/samples", dir);
 
-			fs.readdirSync(`test/runtime/samples/${dir}`).forEach(file => {
+			glob.sync('**/*.html', { cwd: `test/runtime/samples/${dir}` }).forEach(file => {
 				const resolved = require.resolve(`../runtime/samples/${dir}/${file}`);
 				delete require.cache[resolved];
 			});