Merge pull request #1815 from sveltejs/gh-1434

Don't render undefined/null attributes

src/compile/nodes/Attribute.ts

@@ -1,4 +1,4 @@
-import { escape, escapeTemplate, stringify } from '../../utils/stringify';
+import { stringify } from '../../utils/stringify';
 import addToSet from '../../utils/addToSet';
 import Component from '../Component';
 import Node from './shared/Node';
@@ -99,16 +99,4 @@ export default class Attribute extends Node {
 				? this.chunks[0].data
 				: '';
 	}
-
-	stringifyForSsr() {
-		return this.chunks
-			.map((chunk: Node) => {
-				if (chunk.type === 'Text') {
-					return escapeTemplate(escape(chunk.data).replace(/"/g, '"'));
-				}
-
-				return '${@escape(' + chunk.snippet + ')}';
-			})
-			.join('');
-	}
 }

src/compile/render-ssr/handlers/Element.ts

@@ -1,5 +1,8 @@
 import { quotePropIfNecessary, quoteNameIfNecessary } from '../../../utils/quoteIfNecessary';
 import isVoidElementName from '../../../utils/isVoidElementName';
+import Attribute from '../../nodes/Attribute';
+import Node from '../../nodes/shared/Node';
+import { escape, escapeTemplate } from '../../../utils/stringify';
 
 // source: https://gist.github.com/ArjanSchouten/0b8574a6ad7f5065a5e7
 const boolean_attributes = new Set([
@@ -71,7 +74,7 @@ export default function(node, renderer, options) {
 				args.push(attribute.expression.snippet);
 			} else {
 				if (attribute.name === 'value' && node.name === 'textarea') {
-					textareaContents = attribute.stringifyForSsr();
+					textareaContents = stringifyAttribute(attribute);
 				} else if (attribute.isTrue) {
 					args.push(`{ ${quoteNameIfNecessary(attribute.name)}: true }`);
 				} else if (
@@ -82,18 +85,18 @@ export default function(node, renderer, options) {
 					// a boolean attribute with one non-Text chunk
 					args.push(`{ ${quoteNameIfNecessary(attribute.name)}: ${attribute.chunks[0].snippet} }`);
 				} else {
-					args.push(`{ ${quoteNameIfNecessary(attribute.name)}: \`${attribute.stringifyForSsr()}\` }`);
+					args.push(`{ ${quoteNameIfNecessary(attribute.name)}: \`${stringifyAttribute(attribute)}\` }`);
 				}
 			}
 		});
 
 		openingTag += "${@spread([" + args.join(', ') + "])}";
 	} else {
-		node.attributes.forEach((attribute: Node) => {
+		node.attributes.forEach((attribute: Attribute) => {
 			if (attribute.type !== 'Attribute') return;
 
 			if (attribute.name === 'value' && node.name === 'textarea') {
-				textareaContents = attribute.stringifyForSsr();
+				textareaContents = stringifyAttribute(attribute);
 			} else if (attribute.isTrue) {
 				openingTag += ` ${attribute.name}`;
 			} else if (
@@ -105,9 +108,14 @@ export default function(node, renderer, options) {
 				openingTag += '${' + attribute.chunks[0].snippet + ' ? " ' + attribute.name + '" : "" }';
 			} else if (attribute.name === 'class' && classExpr) {
 				addClassAttribute = false;
-				openingTag += ` class="\${[\`${attribute.stringifyForSsr()}\`, ${classExpr}].join(' ').trim() }"`;
+				openingTag += ` class="\${[\`${stringifyAttribute(attribute)}\`, ${classExpr}].join(' ').trim() }"`;
+			} else if (attribute.chunks.length === 1 && attribute.chunks[0].type !== 'Text') {
+				const { name } = attribute;
+				const { snippet } = attribute.chunks[0];
+
+				openingTag += '${(v => v == null ? "" : ` ' + name + '="${@escape(' + snippet + ')}"`)(' + snippet + ')}';
 			} else {
-				openingTag += ` ${attribute.name}="${attribute.stringifyForSsr()}"`;
+				openingTag += ` ${attribute.name}="${stringifyAttribute(attribute)}"`;
 			}
 		});
 	}
@@ -139,4 +147,16 @@ export default function(node, renderer, options) {
 	if (!isVoidElementName(node.name)) {
 		renderer.append(`</${node.name}>`);
 	}
+}
+
+function stringifyAttribute(attribute: Attribute) {
+	return attribute.chunks
+		.map((chunk: Node) => {
+			if (chunk.type === 'Text') {
+				return escapeTemplate(escape(chunk.data).replace(/"/g, '&quot;'));
+			}
+
+			return '${@escape(' + chunk.snippet + ')}';
+		})
+		.join('');
 }

src/shared/dom.js

@@ -82,7 +82,8 @@ export function removeListener(node, event, handler) {
 }
 
 export function setAttribute(node, attribute, value) {
-	node.setAttribute(attribute, value);
+	if (value == null) node.removeAttribute(attribute);
+	else node.setAttribute(attribute, value);
 }
 
 export function setAttributes(node, attributes) {
@@ -92,8 +93,7 @@ export function setAttributes(node, attributes) {
 		} else if (key in node) {
 			node[key] = attributes[key];
 		} else {
-			if (attributes[key] === undefined) removeAttribute(node, key);
+			setAttribute(node, key, attributes[key]);
-			else setAttribute(node, key, attributes[key]);
 		}
 	}
 }
@@ -104,14 +104,10 @@ export function setCustomElementData(node, prop, value) {
 	} else if (value) {
 		setAttribute(node, prop, value);
 	} else {
-		removeAttribute(node, prop);
+		node.removeAttribute(prop);
 	}
 }
 
-export function removeAttribute(node, attribute) {
-	node.removeAttribute(attribute);
-}
-
 export function setXlinkAttribute(node, attribute, value) {
 	node.setAttributeNS('http://www.w3.org/1999/xlink', attribute, value);
 }

test/runtime/samples/set-undefined-attr/_config.js

@@ -0,0 +1,5 @@
+export default {
+	html: '<div></div>',
+
+	ssrHtml: '<div foo=1></div>'
+};

test/runtime/samples/set-undefined-attr/main.html

@@ -0,0 +1,14 @@
+<div {foo} {bar}></div>
+
+<script>
+	export default {
+		data: () => ({
+			foo: 1,
+			bar: undefined
+		}),
+
+		oncreate () {
+			this.set({ foo: undefined });
+		}
+	};
+</script>