From 49db9e752476513a053e0643bbacbe8468a9fc8c Mon Sep 17 00:00:00 2001
From: Richard Harris <richard.a.harris@gmail.com>
Date: Sun, 30 Dec 2018 12:28:39 -0500
Subject: [PATCH] more simplification

---
 .../render-dom/wrappers/Element/Binding.ts    | 96 +++++++++----------
 .../render-dom/wrappers/Element/index.ts      | 32 ++-----
 2 files changed, 52 insertions(+), 76 deletions(-)

diff --git a/src/compile/render-dom/wrappers/Element/Binding.ts b/src/compile/render-dom/wrappers/Element/Binding.ts
index d5aab913a6..fb5ff10ecd 100644
--- a/src/compile/render-dom/wrappers/Element/Binding.ts
+++ b/src/compile/render-dom/wrappers/Element/Binding.ts
@@ -26,10 +26,9 @@ export default class BindingWrapper {
 		mutation: string;
 		contextual_dependencies: Set<string>
 	};
-	updateDom: string;
+	snippet: string;
 	initialUpdate: string;
 	needsLock: boolean;
-	updateCondition: string;
 
 	constructor(block: Block, node: Binding, parent: ElementWrapper) {
 		this.node = node;
@@ -64,39 +63,25 @@ export default class BindingWrapper {
 
 		// view to model
 		this.handler = getEventHandler(this, parent.renderer, block, this.object, contextless_snippet);
-	}
-
-	isReadOnlyMediaAttribute() {
-		return readOnlyMediaAttributes.has(this.node.name);
-	}
 
-	munge(block: Block) {
-		const { parent } = this;
-		const { renderer } = parent;
-
-		const needsLock = (
-			parent.node.name !== 'input' ||
-			!/radio|checkbox|range|color/.test(parent.node.getStaticAttributeValue('type'))
-		);
+		this.snippet = this.node.expression.render();
 
 		const isReadOnly = (
 			(parent.node.isMediaNode() && readOnlyMediaAttributes.has(this.node.name)) ||
 			dimensions.test(this.node.name)
 		);
 
-		let updateConditions: string[] = [];
-
-		const snippet = this.node.expression.render();
+		this.needsLock = !isReadOnly && (
+			parent.node.name !== 'input' ||
+			!/radio|checkbox|range|color/.test(parent.node.getStaticAttributeValue('type'))
+		);
+	}
 
-		// special case: if you have e.g. `<input type=checkbox bind:checked=selected.done>`
-		// and `selected` is an object chosen with a <select>, then when `checked` changes,
-		// we need to tell the component to update all the values `selected` might be
-		// pointing to
-		// TODO should this happen in preprocess?
+	get_dependencies() {
 		const dependencies = new Set(this.node.expression.dependencies);
 
 		this.node.expression.dependencies.forEach((prop: string) => {
-			const indirectDependencies = renderer.component.indirectDependencies.get(prop);
+			const indirectDependencies = this.parent.renderer.component.indirectDependencies.get(prop);
 			if (indirectDependencies) {
 				indirectDependencies.forEach(indirectDependency => {
 					dependencies.add(indirectDependency);
@@ -104,8 +89,24 @@ export default class BindingWrapper {
 			}
 		});
 
+		return dependencies;
+	}
+
+	isReadOnlyMediaAttribute() {
+		return readOnlyMediaAttributes.has(this.node.name);
+	}
+
+	render(block: Block, lock: string) {
+		// bind:offsetWidth and bind:offsetHeight — readonly
+		if (dimensions.test(this.node.name)) return;
+
+		const { parent } = this;
+		const { renderer } = parent;
+
+		let updateConditions: string[] = this.needsLock ? [`!${lock}`] : [];
+
 		// model to view
-		let updateDom = getDomUpdater(parent, this, snippet);
+		let updateDom = getDomUpdater(parent, this);
 		let initialUpdate = updateDom;
 
 		// special cases
@@ -122,7 +123,7 @@ export default class BindingWrapper {
 		}
 
 		if (this.node.name === 'currentTime' || this.node.name === 'volume') {
-			updateConditions.push(`!isNaN(${snippet})`);
+			updateConditions.push(`!isNaN(${this.snippet})`);
 
 			if (this.node.name === 'currentTime') initialUpdate = null;
 		}
@@ -132,17 +133,11 @@ export default class BindingWrapper {
 			const last = block.getUniqueName(`${parent.var}_is_paused`);
 			block.addVariable(last, 'true');
 
-			updateConditions.push(`${last} !== (${last} = ${snippet})`);
+			updateConditions.push(`${last} !== (${last} = ${this.snippet})`);
 			updateDom = `${parent.var}[${last} ? "pause" : "play"]();`;
 			initialUpdate = null;
 		}
 
-		// bind:offsetWidth and bind:offsetHeight
-		if (dimensions.test(this.node.name)) {
-			initialUpdate = null;
-			updateDom = null;
-		}
-
 		const dependencyArray = [...this.node.expression.dynamic_dependencies]
 
 		if (dependencyArray.length === 1) {
@@ -153,26 +148,21 @@ export default class BindingWrapper {
 			)
 		}
 
-		return {
-			name: this.node.name,
-			object: this.object,
-			handler: this.handler,
-			snippet,
-			updateDom: updateDom,
-			initialUpdate: initialUpdate,
-			needsLock: !isReadOnly && needsLock,
-			updateCondition: updateConditions.length ? updateConditions.join(' && ') : undefined,
-			isReadOnlyMediaAttribute: this.isReadOnlyMediaAttribute(),
-			dependencies,
-			contextual_dependencies: this.node.expression.contextual_dependencies
-		};
+		if (updateDom) {
+			block.builders.update.addLine(
+				updateConditions.length ? `if (${updateConditions.join(' && ')}) ${updateDom}` : updateDom
+			);
+		}
+
+		if (initialUpdate) {
+			block.builders.mount.addBlock(initialUpdate);
+		}
 	}
 }
 
 function getDomUpdater(
 	element: ElementWrapper,
-	binding: BindingWrapper,
-	snippet: string
+	binding: BindingWrapper
 ) {
 	const { node } = element;
 
@@ -186,21 +176,21 @@ function getDomUpdater(
 
 	if (node.name === 'select') {
 		return node.getStaticAttributeValue('multiple') === true ?
-			`@selectOptions(${element.var}, ${snippet})` :
-			`@selectOption(${element.var}, ${snippet})`;
+			`@selectOptions(${element.var}, ${binding.snippet})` :
+			`@selectOption(${element.var}, ${binding.snippet})`;
 	}
 
 	if (binding.node.name === 'group') {
 		const type = node.getStaticAttributeValue('type');
 
 		const condition = type === 'checkbox'
-			? `~${snippet}.indexOf(${element.var}.__value)`
-			: `${element.var}.__value === ${snippet}`;
+			? `~${binding.snippet}.indexOf(${element.var}.__value)`
+			: `${element.var}.__value === ${binding.snippet}`;
 
 		return `${element.var}.checked = ${condition};`
 	}
 
-	return `${element.var}.${binding.node.name} = ${snippet};`;
+	return `${element.var}.${binding.node.name} = ${binding.snippet};`;
 }
 
 function getBindingGroup(renderer: Renderer, value: Node) {
diff --git a/src/compile/render-dom/wrappers/Element/index.ts b/src/compile/render-dom/wrappers/Element/index.ts
index 3d494c4d9f..9381e7cf1f 100644
--- a/src/compile/render-dom/wrappers/Element/index.ts
+++ b/src/compile/render-dom/wrappers/Element/index.ts
@@ -90,7 +90,6 @@ export default class ElementWrapper extends Wrapper {
 	attributes: AttributeWrapper[];
 	bindings: Binding[];
 	classDependencies: string[];
-	initialUpdate: string;
 
 	slotOwner?: InlineComponentWrapper;
 	selectBindingDependencies?: Set<string>;
@@ -291,10 +290,6 @@ export default class ElementWrapper extends Wrapper {
 		this.addActions(block);
 		this.addClasses(block);
 
-		if (this.initialUpdate) {
-			block.builders.mount.addBlock(this.initialUpdate);
-		}
-
 		if (nodes && this.renderer.options.hydratable) {
 			block.builders.claim.addLine(
 				`${nodes}.forEach(@detachNode);`
@@ -378,7 +373,8 @@ export default class ElementWrapper extends Wrapper {
 		const needsLock = this.node.name !== 'input' || !/radio|checkbox|range|color/.test(this.getStaticAttributeValue('type'));
 
 		// TODO munge in constructor
-		const mungedBindings = this.bindings.map(binding => binding.munge(block));
+		// const mungedBindings = this.bindings.map(binding => binding.munge(block));
+		const mungedBindings = this.bindings;
 
 		const lock = mungedBindings.some(binding => binding.needsLock) ?
 			block.getUniqueName(`${this.var}_updating`) :
@@ -390,8 +386,8 @@ export default class ElementWrapper extends Wrapper {
 			.map(event => ({
 				events: event.eventNames,
 				bindings: mungedBindings
-					.filter(binding => binding.name !== 'this')
-					.filter(binding => event.filter(this.node, binding.name))
+					.filter(binding => binding.node.name !== 'this')
+					.filter(binding => event.filter(this.node, binding.node.name))
 			}))
 			.filter(group => group.bindings.length);
 
@@ -400,6 +396,7 @@ export default class ElementWrapper extends Wrapper {
 			renderer.component.declarations.push(handler);
 			renderer.component.template_references.add(handler);
 
+			// TODO figure out how to handle locks
 			const needsLock = group.bindings.some(binding => binding.needsLock);
 
 			const dependencies = new Set();
@@ -407,18 +404,11 @@ export default class ElementWrapper extends Wrapper {
 
 			group.bindings.forEach(binding => {
 				// TODO this is a mess
-				addToSet(dependencies, binding.dependencies);
-				addToSet(contextual_dependencies, binding.contextual_dependencies);
+				addToSet(dependencies, binding.get_dependencies());
+				addToSet(contextual_dependencies, binding.node.expression.contextual_dependencies);
 				addToSet(contextual_dependencies, binding.handler.contextual_dependencies);
 
-				if (!binding.updateDom) return;
-
-				const updateConditions = needsLock ? [`!${lock}`] : [];
-				if (binding.updateCondition) updateConditions.push(binding.updateCondition);
-
-				block.builders.update.addLine(
-					updateConditions.length ? `if (${updateConditions.join(' && ')}) ${binding.updateDom}` : binding.updateDom
-				);
+				binding.render(block, lock);
 			});
 
 			const mutations = group.bindings.map(binding => binding.handler.mutation).filter(Boolean).join('\n');
@@ -432,8 +422,6 @@ export default class ElementWrapper extends Wrapper {
 				block.addVariable(animation_frame);
 			}
 
-			// TODO figure out how to handle locks
-
 			let callee;
 
 			// TODO dry this out — similar code for event handlers and component bindings
@@ -499,7 +487,7 @@ export default class ElementWrapper extends Wrapper {
 				.map(binding => `${binding.snippet} === void 0`)
 				.join(' || ');
 
-			if (this.node.name === 'select' || group.bindings.find(binding => binding.name === 'indeterminate' || binding.isReadOnlyMediaAttribute)) {
+			if (this.node.name === 'select' || group.bindings.find(binding => binding.node.name === 'indeterminate' || binding.isReadOnlyMediaAttribute())) {
 				block.builders.hydrate.addLine(
 					`if (${someInitialStateIsUndefined}) @add_render_callback(() => ${callee}.call(${this.var}));`
 				);
@@ -512,8 +500,6 @@ export default class ElementWrapper extends Wrapper {
 			}
 		});
 
-		this.initialUpdate = mungedBindings.map(binding => binding.initialUpdate).filter(Boolean).join('\n');
-
 		const this_binding = this.bindings.find(b => b.node.name === 'this');
 		if (this_binding) {
 			const name = renderer.component.getUniqueName(`${this.var}_binding`);