Merge pull request #3330 from sveltejs/gh-3283

undefined classes

src/compiler/compile/nodes/Element.ts

@@ -703,16 +703,20 @@ export default class Element extends Node {
 		return this.name === 'audio' || this.name === 'video';
 	}
 
-	add_css_class(class_name = this.component.stylesheet.id) {
+	add_css_class() {
+		const { id } = this.component.stylesheet;
+
 		const class_attribute = this.attributes.find(a => a.name === 'class');
+
 		if (class_attribute && !class_attribute.is_true) {
 			if (class_attribute.chunks.length === 1 && class_attribute.chunks[0].type === 'Text') {
-				(class_attribute.chunks[0] as Text).data += ` ${class_name}`;
+				(class_attribute.chunks[0] as Text).data += ` ${id}`;
 			} else {
 				(class_attribute.chunks as Node[]).push(
 					new Text(this.component, this, this.scope, {
 						type: 'Text',
-						data: ` ${class_name}`
+						data: ` ${id}`,
+						synthetic: true
 					})
 				);
 			}
@@ -721,7 +725,7 @@ export default class Element extends Node {
 				new Attribute(this.component, this, this.scope, {
 					type: 'Attribute',
 					name: 'class',
-					value: [{ type: 'Text', data: class_name }]
+					value: [{ type: 'Text', data: id, synthetic: true }]
 				})
 			);
 		}

src/compiler/compile/nodes/Text.ts

@@ -6,9 +6,11 @@ import { INode } from './interfaces';
 export default class Text extends Node {
 	type: 'Text';
 	data: string;
+	synthetic: boolean;
 
 	constructor(component: Component, parent: INode, scope: TemplateScope, info: any) {
 		super(component, parent, scope, info);
 		this.data = info.data;
+		this.synthetic = info.synthetic || false;
 	}
 }

src/compiler/compile/render_dom/wrappers/Element/Attribute.ts

@@ -5,6 +5,7 @@ import ElementWrapper from './index';
 import { stringify } from '../../../utils/stringify';
 import deindent from '../../../utils/deindent';
 import Expression from '../../../nodes/shared/Expression';
+import Text from '../../../nodes/Text';
 
 export default class AttributeWrapper {
 	node: Attribute;
@@ -84,19 +85,13 @@ export default class AttributeWrapper {
 				value = (this.node.chunks[0] as Expression).render(block);
 			} else {
 				// '{foo} {bar}' — treat as string concatenation
-				value =
-					(this.node.chunks[0].type === 'Text' ? '' : `"" + `) +
-					this.node.chunks
-						.map((chunk) => {
-							if (chunk.type === 'Text') {
-								return stringify(chunk.data);
-							} else {
-								return chunk.get_precedence() <= 13
-									? `(${chunk.render()})`
-									: chunk.render();
-							}
-						})
-						.join(' + ');
+				const prefix = this.node.chunks[0].type === 'Text' ? '' : `"" + `;
+
+				const text = this.node.name === 'class'
+					? this.get_class_name_text()
+					: this.render_chunks().join(' + ');
+
+				value = `${prefix}${text}`;
 			}
 
 			const is_select_value_attribute =
@@ -210,6 +205,31 @@ export default class AttributeWrapper {
 		}
 	}
 
+	get_class_name_text() {
+		const scoped_css = this.node.chunks.some((chunk: Text) => chunk.synthetic);
+		const rendered = this.render_chunks();
+
+		if (scoped_css && rendered.length === 2) {
+			// we have a situation like class={possiblyUndefined}
+			rendered[0] = `@null_to_empty(${rendered[0]})`;
+		}
+
+		return rendered.join(' + ');
+	}
+
+	render_chunks() {
+		return this.node.chunks.map((chunk) => {
+			if (chunk.type === 'Text') {
+				return stringify(chunk.data);
+			}
+
+			const rendered = chunk.render();
+			return chunk.get_precedence() <= 13
+				? `(${rendered})`
+				: rendered;
+		});
+	}
+
 	stringify() {
 		if (this.node.is_true) return '';
 

src/compiler/compile/render_dom/wrappers/Element/index.ts

@@ -816,29 +816,4 @@ export default class ElementWrapper extends Wrapper {
 			}
 		});
 	}
-
-	// todo: looks to be dead code copypasted from Element.add_css_class in src/compile/nodes/Element.ts
-	// add_css_class(class_name = this.component.stylesheet.id) {
-	// 	const class_attribute = this.attributes.find(a => a.name === 'class');
-	// 	if (class_attribute && !class_attribute.is_true) {
-	// 		if (class_attribute.chunks.length === 1 && class_attribute.chunks[0].type === 'Text') {
-	// 			(class_attribute.chunks[0] as Text).data += ` ${class_name}`;
-	// 		} else {
-	// 			(class_attribute.chunks as Node[]).push(
-	// 				new Text(this.component, this, this.scope, {
-	// 					type: 'Text',
-	// 					data: ` ${class_name}`
-	// 				})
-	// 			);
-	// 		}
-	// 	} else {
-	// 		this.attributes.push(
-	// 			new Attribute(this.component, this, this.scope, {
-	// 				type: 'Attribute',
-	// 				name: 'class',
-	// 				value: [{ type: 'Text', data: class_name }]
-	// 			})
-	// 		);
-	// 	}
-	// }
 }

src/compiler/compile/render_ssr/handlers/Element.ts

@@ -2,7 +2,7 @@ import { is_void, quote_prop_if_necessary, quote_name_if_necessary } from '../..
 import Attribute from '../../nodes/Attribute';
 import Class from '../../nodes/Class';
 import { snip } from '../../utils/snip';
-import { stringify_attribute } from '../../utils/stringify_attribute';
+import { stringify_attribute, stringify_class_attribute } from '../../utils/stringify_attribute';
 import { get_slot_scope } from './shared/get_slot_scope';
 import Renderer, { RenderOptions } from '../Renderer';
 import Element from '../../nodes/Element';
@@ -111,9 +111,9 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
 					args.push(`{ ${quote_name_if_necessary(attribute.name)}: ${snip(attribute.chunks[0])} }`);
 				} else if (attribute.name === 'class' && class_expression) {
 					// Add class expression
-					args.push(`{ ${quote_name_if_necessary(attribute.name)}: [\`${stringify_attribute(attribute, true)}\`, \`\${${class_expression}}\`].join(' ').trim() }`);
+					args.push(`{ ${quote_name_if_necessary(attribute.name)}: [\`${stringify_class_attribute(attribute)}\`, \`\${${class_expression}}\`].join(' ').trim() }`);
 				} else {
-					args.push(`{ ${quote_name_if_necessary(attribute.name)}: \`${stringify_attribute(attribute, true)}\` }`);
+					args.push(`{ ${quote_name_if_necessary(attribute.name)}: \`${attribute.name === 'class' ? stringify_class_attribute(attribute) : stringify_attribute(attribute, true)}\` }`);
 				}
 			}
 		});
@@ -136,14 +136,13 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
 				opening_tag += '${' + snip(attribute.chunks[0]) + ' ? " ' + attribute.name + '" : "" }';
 			} else if (attribute.name === 'class' && class_expression) {
 				add_class_attribute = false;
-				opening_tag += ` class="\${[\`${stringify_attribute(attribute, true)}\`, ${class_expression}].join(' ').trim() }"`;
+				opening_tag += ` class="\${[\`${stringify_class_attribute(attribute)}\`, ${class_expression}].join(' ').trim() }"`;
 			} else if (attribute.chunks.length === 1 && attribute.chunks[0].type !== 'Text') {
 				const { name } = attribute;
 				const snippet = snip(attribute.chunks[0]);
-
-				opening_tag += '${(v => v == null ? "" : ` ' + name + '="${@escape(' + snippet + ')}"`)(' + snippet + ')}';
+				opening_tag += '${@add_attribute("' + name + '", ' + snippet + ', ' + (boolean_attributes.has(name) ? 1 : 0) + ')}';
 			} else {
-				opening_tag += ` ${attribute.name}="${stringify_attribute(attribute, true)}"`;
+				opening_tag += ` ${attribute.name}="${attribute.name === 'class' ? stringify_class_attribute(attribute) : stringify_attribute(attribute, true)}"`;
 			}
 		});
 	}
@@ -165,7 +164,7 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
 			node_contents = '${(' + snippet + ') || ""}';
 		} else {
 			const snippet = snip(expression);
-			opening_tag += '${@add_attribute("' + name + '", ' + snippet + ')}';
+			opening_tag += '${@add_attribute("' + name + '", ' + snippet + ', 1)}';
 		}
 	});
 

src/compiler/compile/utils/stringify_attribute.ts

@@ -1,6 +1,16 @@
 import Attribute from '../nodes/Attribute';
 import { escape_template, escape } from './stringify';
 import { snip } from './snip';
+import Text from '../nodes/Text';
+
+export function stringify_class_attribute(attribute: Attribute) {
+	// handle special case — `class={possiblyUndefined}` with scoped CSS
+	if (attribute.chunks.length === 2 && (attribute.chunks[1] as Text).synthetic) {
+		return '${@escape(@null_to_empty(' + snip(attribute.chunks[0]) + '))}' + (attribute.chunks[1] as Text).data;
+	}
+
+	return stringify_attribute(attribute, true);
+}
 
 export function stringify_attribute(attribute: Attribute, is_ssr: boolean) {
 	return attribute.chunks

src/runtime/internal/ssr.ts

@@ -120,9 +120,9 @@ export function create_ssr_component(fn) {
 	};
 }
 
-export function add_attribute(name, value) {
-	if (!value) return '';
-	return ` ${name}${value === true ? '' : `=${typeof value === 'string' ? JSON.stringify(value) : `"${value}"`}`}`;
+export function add_attribute(name, value, boolean) {
+	if (value == null || (boolean && !value)) return '';
+	return ` ${name}${value === true ? '' : `=${typeof value === 'string' ? JSON.stringify(escape(value)) : `"${value}"`}`}`;
 }
 
 export function add_classes(classes) {

src/runtime/internal/utils.ts

@@ -96,3 +96,7 @@ export function once(fn) {
 		fn.call(this, ...args);
 	};
 }
+
+export function null_to_empty(value) {
+	return value == null ? '' : value;
+}

test/runtime/samples/attribute-false/_config.js

@@ -0,0 +1,3 @@
+export default {
+	html: `<div class="false"></div>`,
+};

test/runtime/samples/attribute-false/main.svelte

@@ -0,0 +1 @@
+<div class={false}></div>

test/runtime/samples/attribute-null-classname-no-style/_config.js

@@ -0,0 +1,42 @@
+export default {
+	props: {
+		testName: "testClassName"
+	},
+
+	html: `<div class="testClassName"></div>`,
+
+	test({ assert, component, target }) {
+		const div = target.querySelector('div');
+		assert.equal(div.className, 'testClassName');
+
+		component.testName = null;
+		assert.equal(div.className, '');
+
+		component.testName = undefined;
+		assert.equal(div.className, '');
+
+		component.testName = undefined + '';
+		assert.equal(div.className, 'undefined');
+
+		component.testName = null + '';
+		assert.equal(div.className, 'null');
+
+		component.testName = 1;
+		assert.equal(div.className, '1');
+
+		component.testName = 0;
+		assert.equal(div.className, '0');
+
+		component.testName = false;
+		assert.equal(div.className, 'false');
+
+		component.testName = true;
+		assert.equal(div.className, 'true');
+
+		component.testName = {};
+		assert.equal(div.className, '[object Object]');
+
+		component.testName = '';
+		assert.equal(div.className, '');
+	}
+};

test/runtime/samples/attribute-null-classname-no-style/main.svelte

@@ -0,0 +1,5 @@
+<script>
+	export let testName;
+</script>
+
+<div class={testName}></div>

test/runtime/samples/attribute-null-classname-with-style/_config.js

@@ -0,0 +1,40 @@
+export default {
+	html: `<div class=" svelte-x1o6ra"></div>`,
+
+	test({ assert, component, target }) {
+		const div = target.querySelector('div');
+
+		component.testName = null;
+		assert.equal(div.className, ' svelte-x1o6ra');
+
+		component.testName = undefined;
+		assert.equal(div.className, ' svelte-x1o6ra');
+
+		component.testName = undefined + '';
+		assert.equal(div.className, 'undefined svelte-x1o6ra');
+
+		component.testName = null + '';
+		assert.equal(div.className, 'null svelte-x1o6ra');
+
+		component.testName = 1;
+		assert.equal(div.className, '1 svelte-x1o6ra');
+
+		component.testName = 0;
+		assert.equal(div.className, '0 svelte-x1o6ra');
+
+		component.testName = false;
+		assert.equal(div.className, 'false svelte-x1o6ra');
+
+		component.testName = true;
+		assert.equal(div.className, 'true svelte-x1o6ra');
+
+		component.testName = {};
+		assert.equal(div.className, '[object Object] svelte-x1o6ra');
+
+		component.testName = '';
+		assert.equal(div.className, ' svelte-x1o6ra');
+
+		component.testName = 'testClassName';
+		assert.equal(div.className, 'testClassName svelte-x1o6ra');
+	}
+};

test/runtime/samples/attribute-null-classname-with-style/main.svelte

@@ -0,0 +1,11 @@
+<script>
+	export let testName;
+</script>
+
+<style>
+  div {
+    color: purple;
+  }
+</style>
+
+<div class={testName}></div>

test/runtime/samples/attribute-null-classnames-no-style/_config.js

@@ -0,0 +1,45 @@
+export default {
+	props: {
+		testName1: "test1",
+		testName2: "test2",
+	},
+
+	html: `<div class="test1test2"></div>`,
+
+	test({ assert, component, target }) {
+		const div = target.querySelector('div');
+		assert.equal(div.className, 'test1test2');
+
+		component.testName1 = null;
+		component.testName2 = null;
+		assert.equal(div.className, '0');
+
+		component.testName1 = null;
+		component.testName2 = "test";
+		assert.equal(div.className, 'nulltest');
+
+		component.testName1 = undefined;
+		component.testName2 = "test";
+		assert.equal(div.className, 'undefinedtest');
+
+		component.testName1 = undefined;
+		component.testName2 = undefined;
+		assert.equal(div.className, 'NaN');
+
+		component.testName1 = null;
+		component.testName2 = 1;
+		assert.equal(div.className, '1');
+
+		component.testName1 = undefined;
+		component.testName2 = 1;
+		assert.equal(div.className, 'NaN');
+
+		component.testName1 = null;
+		component.testName2 = 0;
+		assert.equal(div.className, '0');
+
+		component.testName1 = undefined;
+		component.testName2 = 0;
+		assert.equal(div.className, 'NaN');
+	}
+};

test/runtime/samples/attribute-null-classnames-no-style/main.svelte

@@ -0,0 +1,6 @@
+<script>
+	export let testName1;
+	export let testName2;
+</script>
+
+<div class={testName1 + testName2}></div>