From b22abc7936e14a7dcfb9874408f144a7e07ede2f Mon Sep 17 00:00:00 2001
From: Tan Li Hau <lhtan93@gmail.com>
Date: Fri, 8 Nov 2019 21:54:11 +0800
Subject: [PATCH] fix undefined class with scoped-css

---
 .../render_dom/wrappers/Element/Attribute.ts  | 212 ++++++++++--------
 .../render_dom/wrappers/Element/index.ts      |   2 +-
 .../samples/undefined-with-scope/expected.css |   1 +
 .../undefined-with-scope/expected.html        |   1 +
 .../samples/undefined-with-scope/input.svelte |   3 +
 5 files changed, 127 insertions(+), 92 deletions(-)
 create mode 100644 test/css/samples/undefined-with-scope/expected.css
 create mode 100644 test/css/samples/undefined-with-scope/expected.html
 create mode 100644 test/css/samples/undefined-with-scope/input.svelte

diff --git a/src/compiler/compile/render_dom/wrappers/Element/Attribute.ts b/src/compiler/compile/render_dom/wrappers/Element/Attribute.ts
index d50a8238d9d4..9ee7694b7166 100644
--- a/src/compiler/compile/render_dom/wrappers/Element/Attribute.ts
+++ b/src/compiler/compile/render_dom/wrappers/Element/Attribute.ts
@@ -7,7 +7,6 @@ import { b, x } from 'code-red';
 import Expression from '../../../nodes/shared/Expression';
 import Text from '../../../nodes/Text';
 import { changed } from '../shared/changed';
-import { Literal } from 'estree';
 
 export default class AttributeWrapper {
 	node: Attribute;
@@ -72,85 +71,69 @@ export default class AttributeWrapper {
 		const is_legacy_input_type = element.renderer.component.compile_options.legacy && name === 'type' && this.parent.node.name === 'input';
 
 		const dependencies = this.node.get_dependencies();
-		if (dependencies.length > 0) {
-			let value;
-
-			// TODO some of this code is repeated in Tag.ts — would be good to
-			// DRY it out if that's possible without introducing crazy indirection
-			if (this.node.chunks.length === 1) {
-				// single {tag} — may be a non-string
-				value = (this.node.chunks[0] as Expression).manipulate(block);
-			} else {
-				value = this.node.name === 'class'
-					? this.get_class_name_text()
-					: this.render_chunks().reduce((lhs, rhs) => x`${lhs} + ${rhs}`);
-
-				// '{foo} {bar}' — treat as string concatenation
-				if (this.node.chunks[0].type !== 'Text') {
-					value = x`"" + ${value}`;
-				}
-			}
+		const value = this.get_value(block);
 
-			const is_select_value_attribute =
-				name === 'value' && element.node.name === 'select';
+		const is_select_value_attribute =
+			name === 'value' && element.node.name === 'select';
 
-			const should_cache = is_select_value_attribute; // TODO is this necessary?
+		const should_cache = is_select_value_attribute; // TODO is this necessary?
 
-			const last = should_cache && block.get_unique_name(
-				`${element.var.name}_${name.replace(/[^a-zA-Z_$]/g, '_')}_value`
-			);
+		const last = should_cache && block.get_unique_name(
+			`${element.var.name}_${name.replace(/[^a-zA-Z_$]/g, '_')}_value`
+		);
 
-			if (should_cache) block.add_variable(last);
-
-			let updater;
-			const init = should_cache ? x`${last} = ${value}` : value;
-
-			if (is_legacy_input_type) {
-				block.chunks.hydrate.push(
-					b`@set_input_type(${element.var}, ${init});`
-				);
-				updater = b`@set_input_type(${element.var}, ${should_cache ? last : value});`;
-			} else if (is_select_value_attribute) {
-				// annoying special case
-				const is_multiple_select = element.node.get_static_attribute_value('multiple');
-				const i = block.get_unique_name('i');
-				const option = block.get_unique_name('option');
-
-				const if_statement = is_multiple_select
-					? b`
-						${option}.selected = ~${last}.indexOf(${option}.__value);`
-					: b`
-						if (${option}.__value === ${last}) {
-							${option}.selected = true;
-							${{ type: 'BreakStatement' }};
-						}`; // TODO the BreakStatement is gross, but it's unsyntactic otherwise...
-
-				updater = b`
-					for (var ${i} = 0; ${i} < ${element.var}.options.length; ${i} += 1) {
-						var ${option} = ${element.var}.options[${i}];
-
-						${if_statement}
-					}
-				`;
-
-				block.chunks.mount.push(b`
-					${last} = ${value};
-					${updater}
-				`);
-			} else if (property_name) {
-				block.chunks.hydrate.push(
-					b`${element.var}.${property_name} = ${init};`
-				);
-				updater = block.renderer.options.dev
-					? b`@prop_dev(${element.var}, "${property_name}", ${should_cache ? last : value});`
-					: b`${element.var}.${property_name} = ${should_cache ? last : value};`;
-			} else {
-				block.chunks.hydrate.push(
-					b`${method}(${element.var}, "${name}", ${init});`
-				);
-				updater = b`${method}(${element.var}, "${name}", ${should_cache ? last : value});`;
-			}
+		if (should_cache) block.add_variable(last);
+
+		let updater;
+		const init = should_cache ? x`${last} = ${value}` : value;
+
+		if (is_legacy_input_type) {
+			block.chunks.hydrate.push(
+				b`@set_input_type(${element.var}, ${init});`
+			);
+			updater = b`@set_input_type(${element.var}, ${should_cache ? last : value});`;
+		} else if (is_select_value_attribute) {
+			// annoying special case
+			const is_multiple_select = element.node.get_static_attribute_value('multiple');
+			const i = block.get_unique_name('i');
+			const option = block.get_unique_name('option');
+
+			const if_statement = is_multiple_select
+				? b`
+					${option}.selected = ~${last}.indexOf(${option}.__value);`
+				: b`
+					if (${option}.__value === ${last}) {
+						${option}.selected = true;
+						${{ type: 'BreakStatement' }};
+					}`; // TODO the BreakStatement is gross, but it's unsyntactic otherwise...
+
+			updater = b`
+				for (var ${i} = 0; ${i} < ${element.var}.options.length; ${i} += 1) {
+					var ${option} = ${element.var}.options[${i}];
+
+					${if_statement}
+				}
+			`;
+
+			block.chunks.mount.push(b`
+				${last} = ${value};
+				${updater}
+			`);
+		} else if (property_name) {
+			block.chunks.hydrate.push(
+				b`${element.var}.${property_name} = ${init};`
+			);
+			updater = block.renderer.options.dev
+				? b`@prop_dev(${element.var}, "${property_name}", ${should_cache ? last : value});`
+				: b`${element.var}.${property_name} = ${should_cache ? last : value};`;
+		} else {
+			block.chunks.hydrate.push(
+				b`${method}(${element.var}, "${name}", ${init});`
+			);
+			updater = b`${method}(${element.var}, "${name}", ${should_cache ? last : value});`;
+		}
 
+		if (dependencies.length > 0) {
 			let condition = changed(dependencies);
 
 			if (should_cache) {
@@ -165,23 +148,11 @@ export default class AttributeWrapper {
 				if (${condition}) {
 					${updater}
 				}`);
-		} else {
-			const value = this.node.get_value(block);
-
-			const statement = (
-				is_legacy_input_type
-					? b`@set_input_type(${element.var}, ${value});`
-					: property_name
-						? b`${element.var}.${property_name} = ${value};`
-						: b`${method}(${element.var}, "${name}", ${value.type === 'Literal' && (value as Literal).value === true ? x`""` : value});`
-			);
-
-			block.chunks.hydrate.push(statement);
+		}
 
-			// special case – autofocus. has to be handled in a bit of a weird way
-			if (this.node.is_true && name === 'autofocus') {
-				block.autofocus = element.var;
-			}
+		// special case – autofocus. has to be handled in a bit of a weird way
+		if (this.node.is_true && name === 'autofocus') {
+			block.autofocus = element.var;
 		}
 
 		if (is_indirectly_bound_value) {
@@ -199,6 +170,36 @@ export default class AttributeWrapper {
 		return metadata;
 	}
 
+	get_value(block) {
+		if (this.node.is_true) {
+			const metadata = this.get_metadata();
+			if (metadata && boolean_attribute.has(metadata.property_name.toLowerCase())) {
+				return x`true`;
+			}
+			return x`""`;
+		}
+		if (this.node.chunks.length === 0) return x`""`;
+
+		// TODO some of this code is repeated in Tag.ts — would be good to
+		// DRY it out if that's possible without introducing crazy indirection
+		if (this.node.chunks.length === 1) {
+			return this.node.chunks[0].type === 'Text'
+				? string_literal((this.node.chunks[0] as Text).data)
+				: (this.node.chunks[0] as Expression).manipulate(block);
+		}
+
+		let value = this.node.name === 'class'
+			? this.get_class_name_text()
+			: this.render_chunks().reduce((lhs, rhs) => x`${lhs} + ${rhs}`);
+
+		// '{foo} {bar}' — treat as string concatenation
+		if (this.node.chunks[0].type !== 'Text') {
+			value = x`"" + ${value}`;
+		}
+
+		return value;
+	}
+
 	get_class_name_text() {
 		const scoped_css = this.node.chunks.some((chunk: Text) => chunk.synthetic);
 		const rendered = this.render_chunks();
@@ -292,3 +293,32 @@ Object.keys(attribute_lookup).forEach(name => {
 	const metadata = attribute_lookup[name];
 	if (!metadata.property_name) metadata.property_name = name;
 });
+
+// source: https://html.spec.whatwg.org/multipage/indices.html
+const boolean_attribute = new Set([
+	'allowfullscreen',
+	'allowpaymentrequest',
+	'async',
+	'autofocus',
+	'autoplay',
+	'checked',
+	'controls',
+	'default',
+	'defer',
+	'disabled',
+	'formnovalidate',
+	'hidden',
+	'ismap',
+	'itemscope',
+	'loop',
+	'multiple',
+	'muted',
+	'nomodule',
+	'novalidate',
+	'open',
+	'playsinline',
+	'readonly',
+	'required',
+	'reversed',
+	'selected'
+]);
\ No newline at end of file
diff --git a/src/compiler/compile/render_dom/wrappers/Element/index.ts b/src/compiler/compile/render_dom/wrappers/Element/index.ts
index 9b7bbfef7489..80939ecf533c 100644
--- a/src/compiler/compile/render_dom/wrappers/Element/index.ts
+++ b/src/compiler/compile/render_dom/wrappers/Element/index.ts
@@ -617,7 +617,7 @@ export default class ElementWrapper extends Wrapper {
 					const snippet = x`{ ${
 						(metadata && metadata.property_name) ||
 						fix_attribute_casing(attr.node.name)
-					}: ${attr.node.get_value(block)} }`;
+					}: ${attr.get_value(block)} }`;
 					initial_props.push(snippet);
 
 					updates.push(condition ? x`${condition} && ${snippet}` : snippet);
diff --git a/test/css/samples/undefined-with-scope/expected.css b/test/css/samples/undefined-with-scope/expected.css
new file mode 100644
index 000000000000..5d8d69ac33a7
--- /dev/null
+++ b/test/css/samples/undefined-with-scope/expected.css
@@ -0,0 +1 @@
+p.svelte-xyz{color:red}
\ No newline at end of file
diff --git a/test/css/samples/undefined-with-scope/expected.html b/test/css/samples/undefined-with-scope/expected.html
new file mode 100644
index 000000000000..ddb9429bc891
--- /dev/null
+++ b/test/css/samples/undefined-with-scope/expected.html
@@ -0,0 +1 @@
+<p class=" svelte-xyz">Foo</p>
\ No newline at end of file
diff --git a/test/css/samples/undefined-with-scope/input.svelte b/test/css/samples/undefined-with-scope/input.svelte
new file mode 100644
index 000000000000..c68fb40dea09
--- /dev/null
+++ b/test/css/samples/undefined-with-scope/input.svelte
@@ -0,0 +1,3 @@
+<style>p { color: red; }</style>
+
+<p class={undefined}>Foo</p>
\ No newline at end of file