From 2865a98e57ee487ecb88efb5e9c062cd39b0586c Mon Sep 17 00:00:00 2001
From: Rich Harris <richard.a.harris@gmail.com>
Date: Tue, 19 Sep 2017 16:24:03 -0400
Subject: [PATCH] move properties out of the template IIFE (#756)

---
 src/generators/Generator.ts                   | 249 +++++++++++-------
 src/generators/dom/index.ts                   |  18 +-
 .../dom/visitors/Element/EventHandler.ts      |   2 +-
 .../dom/visitors/Element/addTransitions.ts    |   6 +-
 src/generators/server-side-rendering/index.ts |  58 ++--
 test/css/index.js                             |  62 +++--
 6 files changed, 240 insertions(+), 155 deletions(-)

diff --git a/src/generators/Generator.ts b/src/generators/Generator.ts
index efd5ff7441..9827c09b0d 100644
--- a/src/generators/Generator.ts
+++ b/src/generators/Generator.ts
@@ -1,6 +1,8 @@
 import MagicString, { Bundle } from 'magic-string';
 import { walk } from 'estree-walker';
 import { getLocator } from 'locate-character';
+import deindent from '../utils/deindent';
+import CodeBuilder from '../utils/CodeBuilder';
 import getCodeFrame from '../utils/getCodeFrame';
 import isReference from '../utils/isReference';
 import flattenReference from '../utils/flattenReference';
@@ -47,6 +49,7 @@ export default class Generator {
 	computations: Computation[];
 	templateProperties: Record<string, Node>;
 	slots: Set<string>;
+	javascript: string;
 
 	code: MagicString;
 
@@ -198,7 +201,11 @@ export default class Generator {
 
 						usedContexts.add(name);
 					} else if (helpers.has(name)) {
-						code.prependRight(node.start, `${self.alias('template')}.helpers.`);
+						let object = node;
+						while (object.type === 'MemberExpression') object = object.object;
+
+						const alias = self.alias(name);
+						if (alias !== name) code.overwrite(object.start, object.end, `${self.alias(name)}`);
 					} else if (indexes.has(name)) {
 						const context = indexes.get(name);
 						usedContexts.add(context); // TODO is this right?
@@ -406,6 +413,7 @@ export default class Generator {
 		const imports = this.imports;
 		const computations: Computation[] = [];
 		const templateProperties: Record<string, Node> = {};
+		const componentDefinition = new CodeBuilder();
 
 		let namespace = null;
 		let hasJs = !!js;
@@ -441,114 +449,175 @@ export default class Generator {
 				defaultExport.declaration.properties.forEach((prop: Node) => {
 					templateProperties[prop.key.name] = prop;
 				});
-			}
 
-			['helpers', 'events', 'components', 'transitions'].forEach(key => {
-				if (templateProperties[key]) {
-					templateProperties[key].value.properties.forEach((prop: Node) => {
-						this[key].add(prop.key.name);
-					});
-				}
-			});
-
-			if (templateProperties.computed) {
-				const dependencies = new Map();
-
-				templateProperties.computed.value.properties.forEach((prop: Node) => {
-					const key = prop.key.name;
-					const value = prop.value;
-
-					const deps = value.params.map(
-						(param: Node) =>
-							param.type === 'AssignmentPattern' ? param.left.name : param.name
-					);
-					dependencies.set(key, deps);
+				['helpers', 'events', 'components', 'transitions'].forEach(key => {
+					if (templateProperties[key]) {
+						templateProperties[key].value.properties.forEach((prop: Node) => {
+							this[key].add(prop.key.name);
+						});
+					}
 				});
 
-				const visited = new Set();
-
-				const visit = function visit(key: string) {
-					if (!dependencies.has(key)) return; // not a computation
-
-					if (visited.has(key)) return;
-					visited.add(key);
-
-					const deps = dependencies.get(key);
-					deps.forEach(visit);
+				const addArrowFunctionExpression = (key: string, node: Node) => {
+					const { body, params } = node;
 
-					computations.push({ key, deps });
-				}
-
-				templateProperties.computed.value.properties.forEach((prop: Node) =>
-					visit(prop.key.name)
-				);
-			}
+					const paramString = params.length ?
+						`[✂${params[0].start}-${params[params.length - 1].end}✂]` :
+						``;
 
-			if (templateProperties.namespace) {
-				const ns = templateProperties.namespace.value.value;
-				namespace = namespaces[ns] || ns;
+					if (body.type === 'BlockStatement') {
+						componentDefinition.addBlock(deindent`
+							function @${key}(${paramString}) [✂${body.start}-${body.end}✂]
+						`);
+					} else {
+						componentDefinition.addBlock(deindent`
+							function @${key}(${paramString}) {
+								return [✂${body.start}-${body.end}✂];
+							}
+						`);
+					}
+				};
+
+				const addFunctionExpression = (key: string, node: Node) => {
+					let c = node.start;
+					while (this.source[c] !== '(') c += 1;
+					componentDefinition.addBlock(deindent`
+						function @${key}[✂${c}-${node.end}✂];
+					`);
+				};
+
+				const addValue = (key: string, node: Node) => {
+					const alias = this.alias(key);
+					if (node.type !== 'Identifier' || node.name !== alias) {
+						componentDefinition.addBlock(deindent`
+							var ${alias} = [✂${node.start}-${node.end}✂];
+						`);
+					}
+				};
+
+				const addDeclaration = (key: string, node: Node) => {
+					// TODO disambiguate between different categories, and ensure
+					// no conflicts with existing aliases
+					if (node.type === 'ArrowFunctionExpression') {
+						addArrowFunctionExpression(key, node);
+					} else if (node.type === 'FunctionExpression') {
+						addFunctionExpression(key, node);
+					} else {
+						addValue(key, node);
+					}
+				};
 
-				removeObjectKey(this.code, defaultExport.declaration, 'namespace');
-			}
+				if (templateProperties.components) {
+					templateProperties.components.value.properties.forEach((property: Node) => {
+						// TODO replace all the guff below with this:
+						// addValue(property.key.name, property.value);
 
-			if (templateProperties.components) {
-				let hasNonImportedComponent = false;
-				templateProperties.components.value.properties.forEach(
-					(property: Node) => {
 						const key = property.key.name;
 						const value = source.slice(
 							property.value.start,
 							property.value.end
 						);
-						if (this.userVars.has(value)) {
-							this.importedComponents.set(key, value);
+
+						if (key !== value) {
+							const alias = this.alias(key);
+							componentDefinition.addLine(
+								`var ${alias} = [✂${property.value.start}-${property.value.end}✂];`
+							);
+							this.importedComponents.set(key, alias);
 						} else {
-							hasNonImportedComponent = true;
+							this.importedComponents.set(key, key);
 						}
-					}
-				);
-				if (hasNonImportedComponent) {
-					// remove the specific components that were imported, as we'll refer to them directly
-					Array.from(this.importedComponents.keys()).forEach(key => {
-						removeObjectKey(
-							this.code,
-							templateProperties.components.value,
-							key
+					});
+				}
+
+				if (templateProperties.computed) {
+					const dependencies = new Map();
+
+					templateProperties.computed.value.properties.forEach((prop: Node) => {
+						const key = prop.key.name;
+						const value = prop.value;
+
+						const deps = value.params.map(
+							(param: Node) =>
+								param.type === 'AssignmentPattern' ? param.left.name : param.name
 						);
+						dependencies.set(key, deps);
+					});
+
+					const visited = new Set();
+
+					const visit = (key: string) => {
+						if (!dependencies.has(key)) return; // not a computation
+
+						if (visited.has(key)) return;
+						visited.add(key);
+
+						const deps = dependencies.get(key);
+						deps.forEach(visit);
+
+						computations.push({ key, deps });
+
+						const prop = templateProperties.computed.value.properties.find((prop: Node) => prop.key.name === key);
+						addDeclaration(key, prop.value);
+					};
+
+					templateProperties.computed.value.properties.forEach((prop: Node) =>
+						visit(prop.key.name)
+					);
+				}
+
+				if (templateProperties.data) {
+					addDeclaration('data', templateProperties.data.value);
+				}
+
+				if (templateProperties.events) {
+					templateProperties.events.value.properties.forEach((property: Node) => {
+						addDeclaration(property.key.name, property.value);
 					});
-				} else {
-					// remove the entire components portion of the export
-					removeObjectKey(this.code, defaultExport.declaration, 'components');
 				}
-			}
 
-			// Remove these after version 2
-			if (templateProperties.onrender) {
-				const { key } = templateProperties.onrender;
-				this.code.overwrite(key.start, key.end, 'oncreate', {
-					storeName: true,
-					contentOnly: false,
-				});
-				templateProperties.oncreate = templateProperties.onrender;
-			}
+				if (templateProperties.helpers) {
+					templateProperties.helpers.value.properties.forEach((property: Node) => {
+						addDeclaration(property.key.name, property.value);
+					});
+				}
 
-			if (templateProperties.onteardown) {
-				const { key } = templateProperties.onteardown;
-				this.code.overwrite(key.start, key.end, 'ondestroy', {
-					storeName: true,
-					contentOnly: false,
-				});
-				templateProperties.ondestroy = templateProperties.onteardown;
-			}
+				if (templateProperties.methods) {
+					addDeclaration('methods', templateProperties.methods.value);
+				}
 
-			if (templateProperties.tag) {
-				this.tag = templateProperties.tag.value.value;
-				removeObjectKey(this.code, defaultExport.declaration, 'tag');
-			}
+				if (templateProperties.namespace) {
+					const ns = templateProperties.namespace.value.value;
+					namespace = namespaces[ns] || ns;
+				}
 
-			if (templateProperties.props) {
-				this.props = templateProperties.props.value.elements.map((element: Node) => element.value);
-				removeObjectKey(this.code, defaultExport.declaration, 'props');
+				if (templateProperties.onrender) templateProperties.oncreate = templateProperties.onrender; // remove after v2
+				if (templateProperties.oncreate) {
+					addDeclaration('oncreate', templateProperties.oncreate.value);
+				}
+
+				if (templateProperties.onteardown) templateProperties.ondestroy = templateProperties.onteardown; // remove after v2
+				if (templateProperties.ondestroy) {
+					addDeclaration('ondestroy', templateProperties.ondestroy.value);
+				}
+
+				if (templateProperties.props) {
+					this.props = templateProperties.props.value.elements.map((element: Node) => element.value);
+				}
+
+				if (templateProperties.setup) {
+					addDeclaration('setup', templateProperties.setup.value);
+				}
+
+				if (templateProperties.tag) {
+					this.tag = templateProperties.tag.value.value;
+				}
+
+				if (templateProperties.transitions) {
+					templateProperties.transitions.value.properties.forEach((property: Node) => {
+						addDeclaration(property.key.name, property.value);
+					});
+				}
 			}
 
 			// now that we've analysed the default export, we can determine whether or not we need to keep it
@@ -572,6 +641,10 @@ export default class Generator {
 				this.code.remove(js.content.start, js.content.end);
 				hasJs = false;
 			}
+
+			this.javascript = hasDefaultExport ?
+				`[✂${js.content.start}-${defaultExport.start}✂]${componentDefinition}[✂${defaultExport.end}-${js.content.end}✂]` :
+				`[✂${js.content.start}-${js.content.end}✂]`;
 		}
 
 		this.computations = computations;
diff --git a/src/generators/dom/index.ts b/src/generators/dom/index.ts
index ebc78e2fd2..7eac27be55 100644
--- a/src/generators/dom/index.ts
+++ b/src/generators/dom/index.ts
@@ -127,7 +127,7 @@ export default function dom(
 
 			const condition = `${deps.map(dep => `changed.${dep}`).join(' || ')}`;
 
-			const statement = `if (@differs(state.${key}, (state.${key} = @template.computed.${key}(${deps
+			const statement = `if (@differs(state.${key}, (state.${key} = @${key}(${deps
 				.map(dep => `state.${dep}`)
 				.join(', ')})))) changed.${key} = true;`;
 
@@ -135,8 +135,8 @@ export default function dom(
 		});
 	}
 
-	if (hasJs) {
-		builder.addBlock(`[✂${parsed.js.content.start}-${parsed.js.content.end}✂]`);
+	if (generator.javascript) {
+		builder.addBlock(generator.javascript);
 	}
 
 	if (generator.needsEncapsulateHelper) {
@@ -173,7 +173,7 @@ export default function dom(
 
 	const prototypeBase =
 		`${name}.prototype` +
-		(templateProperties.methods ? `, @template.methods` : '');
+		(templateProperties.methods ? `, @methods` : '');
 	const proto = sharedPath
 		? `@proto`
 		: deindent`
@@ -192,7 +192,7 @@ export default function dom(
 		@init(this, options);
 		${generator.usesRefs && `this.refs = {};`}
 		this._state = ${templateProperties.data
-			? `@assign(@template.data(), options.data)`
+			? `@assign(@data(), options.data)`
 			: `options.data || {}`};
 		${generator.metaBindings}
 		${computations.length && `this._recompute({ ${Array.from(computationDeps).map(dep => `${dep}: 1`).join(', ')} }, this._state);`}
@@ -204,7 +204,7 @@ export default function dom(
 		${generator.bindingGroups.length &&
 			`this._bindingGroups = [${Array(generator.bindingGroups.length).fill('[]').join(', ')}];`}
 
-		${templateProperties.ondestroy && `this._handlers.destroy = [@template.ondestroy]`}
+		${templateProperties.ondestroy && `this._handlers.destroy = [@ondestroy]`}
 
 		${generator.slots.size && `this._slotted = options.slots || {};`}
 
@@ -217,16 +217,16 @@ export default function dom(
 			`if (!document.getElementById("${generator.stylesheet.id}-style")) @add_css();`)
 		}
 
-		${templateProperties.oncreate && `var oncreate = @template.oncreate.bind(this);`}
+		${templateProperties.oncreate && `var _oncreate = @oncreate.bind(this);`}
 
 		${(templateProperties.oncreate || generator.hasComponents || generator.hasComplexBindings || generator.hasIntroTransitions) && deindent`
 			if (!options._root) {
-				this._oncreate = [${templateProperties.oncreate && `oncreate`}];
+				this._oncreate = [${templateProperties.oncreate && `_oncreate`}];
 				${(generator.hasComponents || generator.hasComplexBindings) && `this._beforecreate = [];`}
 				${(generator.hasComponents || generator.hasIntroTransitions) && `this._aftercreate = [];`}
 			} ${templateProperties.oncreate && deindent`
 				else {
-					this._root._oncreate.push(oncreate);
+					this._root._oncreate.push(_oncreate);
 				}
 			`}
 		`}
diff --git a/src/generators/dom/visitors/Element/EventHandler.ts b/src/generators/dom/visitors/Element/EventHandler.ts
index ded9abc44b..f14c5e145f 100644
--- a/src/generators/dom/visitors/Element/EventHandler.ts
+++ b/src/generators/dom/visitors/Element/EventHandler.ts
@@ -79,7 +79,7 @@ export default function visitEventHandler(
 		block.addVariable(handlerName);
 
 		block.builders.hydrate.addBlock(deindent`
-			${handlerName} = @template.events.${name}.call(#component, ${state.parentNode}, function(event) {
+			${handlerName} = @${name}.call(#component, ${state.parentNode}, function(event) {
 				${handlerBody}
 			});
 		`);
diff --git a/src/generators/dom/visitors/Element/addTransitions.ts b/src/generators/dom/visitors/Element/addTransitions.ts
index 98e76b518e..739fe1b6bb 100644
--- a/src/generators/dom/visitors/Element/addTransitions.ts
+++ b/src/generators/dom/visitors/Element/addTransitions.ts
@@ -20,7 +20,7 @@ export default function addTransitions(
 
 		block.addVariable(name);
 
-		const fn = `@template.transitions.${intro.name}`;
+		const fn = `@${intro.name}`;
 
 		block.builders.intro.addBlock(deindent`
 			#component._root._aftercreate.push(function() {
@@ -48,7 +48,7 @@ export default function addTransitions(
 				? block.contextualise(intro.expression).snippet
 				: '{}';
 
-			const fn = `@template.transitions.${intro.name}`; // TODO add built-in transitions?
+			const fn = `@${intro.name}`; // TODO add built-in transitions?
 
 			if (outro) {
 				block.builders.intro.addBlock(deindent`
@@ -73,7 +73,7 @@ export default function addTransitions(
 				? block.contextualise(outro.expression).snippet
 				: '{}';
 
-			const fn = `@template.transitions.${outro.name}`;
+			const fn = `@${outro.name}`;
 
 			// TODO hide elements that have outro'd (unless they belong to a still-outroing
 			// group) prior to their removal from the DOM
diff --git a/src/generators/server-side-rendering/index.ts b/src/generators/server-side-rendering/index.ts
index d624400b91..1ab3f68bc9 100644
--- a/src/generators/server-side-rendering/index.ts
+++ b/src/generators/server-side-rendering/index.ts
@@ -33,30 +33,32 @@ export class SsrGenerator extends Generator {
 
 		this.stylesheet.warnOnUnusedSelectors(options.onwarn);
 
-		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
-			);
+		// TODO how to exclude non-SSR-able stuff?
+
+		// 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) {
@@ -99,24 +101,24 @@ export default function ssr(
 		generator.stylesheet.render(options.filename, true);
 
 	const result = deindent`
-		${hasJs && `[✂${parsed.js.content.start}-${parsed.js.content.end}✂]`}
+		${generator.javascript}
 
 		var ${name} = {};
 
 		${options.filename && `${name}.filename = ${stringify(options.filename)}`};
 
 		${name}.data = function() {
-			return ${templateProperties.data ? `@template.data()` : `{}`};
+			return ${templateProperties.data ? `@data()` : `{}`};
 		};
 
 		${name}.render = function(state, options) {
 			${templateProperties.data
-				? `state = Object.assign(@template.data(), state || {});`
+				? `state = Object.assign(@data(), state || {});`
 				: `state = state || {};`}
 
 			${computations.map(
 				({ key, deps }) =>
-					`state.${key} = @template.computed.${key}(${deps.map(dep => `state.${dep}`).join(', ')});`
+					`state.${key} = @${key}(${deps.map(dep => `state.${dep}`).join(', ')});`
 			)}
 
 			${generator.bindings.length &&
diff --git a/test/css/index.js b/test/css/index.js
index 65867b294a..3310be8267 100644
--- a/test/css/index.js
+++ b/test/css/index.js
@@ -101,34 +101,44 @@ describe('css', () => {
 			if (expected.html !== null) {
 				const window = env();
 
-				const Component = eval(
-					`(function () { ${dom.code}; return SvelteComponent; }())`
-				);
-				const target = window.document.querySelector('main');
-
-				new Component({ target, data: config.data });
-				const html = target.innerHTML;
-
-				fs.writeFileSync(`test/css/samples/${dir}/_actual.html`, html);
-
 				// dom
-				assert.equal(
-					normalizeHtml(window, html.replace(/svelte-\d+/g, 'svelte-xyz')),
-					normalizeHtml(window, expected.html)
-				);
+				try {
+					const Component = eval(
+						`(function () { ${dom.code}; return SvelteComponent; }())`
+					);
+					const target = window.document.querySelector('main');
+
+					new Component({ target, data: config.data });
+					const html = target.innerHTML;
+
+					fs.writeFileSync(`test/css/samples/${dir}/_actual.html`, html);
+
+					assert.equal(
+						normalizeHtml(window, html.replace(/svelte-\d+/g, 'svelte-xyz')),
+						normalizeHtml(window, expected.html)
+					);
+				} catch (err) {
+					console.log(dom.code);
+					throw err;
+				}
 
 				// ssr
-				const component = eval(
-					`(function () { ${ssr.code}; return SvelteComponent; }())`
-				);
-
-				assert.equal(
-					normalizeHtml(
-						window,
-						component.render(config.data).replace(/svelte-\d+/g, 'svelte-xyz')
-					),
-					normalizeHtml(window, expected.html)
-				);
+				try {
+					const component = eval(
+						`(function () { ${ssr.code}; return SvelteComponent; }())`
+					);
+
+					assert.equal(
+						normalizeHtml(
+							window,
+							component.render(config.data).replace(/svelte-\d+/g, 'svelte-xyz')
+						),
+						normalizeHtml(window, expected.html)
+					);
+				} catch (err) {
+					console.log(ssr.code);
+					throw err;
+				}
 			}
 		});
 	});
@@ -140,4 +150,4 @@ function read(file) {
 	} catch (err) {
 		return null;
 	}
-}
+}
\ No newline at end of file