apply spread only for dynamic dependencies

Author: tanhauhau

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

@@ -665,28 +665,45 @@ export default class ElementWrapper extends Wrapper {
 		const initial_props = [];
 		const updates = [];
 
+		const all_dependencies = new Set();
+		this.attributes.forEach(attr => {
+			add_to_set(all_dependencies, attr.node.get_dependencies());
+		});
+
 		this.attributes
 			.forEach(attr => {
-				const condition = attr.node.dependencies.size > 0
-					? block.renderer.dirty(Array.from(attr.node.dependencies))
+				const dependencies = attr.node.get_dependencies();
+
+				const condition = dependencies.length > 0 && (dependencies.length !== all_dependencies.size)
+					? block.renderer.dirty(dependencies)
 					: null;
+				const unchanged = dependencies.length === 0;
 
+				let snippet;
 				if (attr.node.is_spread) {
-					const snippet = attr.node.expression.manipulate(block);
+					snippet = attr.node.expression.manipulate(block);
 
 					initial_props.push(snippet);
 
-					updates.push(condition ? x`${condition} && ${snippet}` : x`false`);
+					if (attr.node.expression.node.type !== 'ObjectExpression') {
+						snippet = x`@get_spread_object(${snippet})`;
+					}
 				} else {
 					const metadata = attr.get_metadata();
 					const name = attr.is_indirectly_bound_value()
 						? '__value'
 						: (metadata && metadata.property_name) || fix_attribute_casing(attr.node.name);
-					const snippet = x`{ ${name}: ${attr.get_value(block)} }`;
+					snippet = x`{ ${name}: ${attr.get_value(block)} }`;
 					initial_props.push(snippet);
-
-					updates.push(condition ? x`${condition} && ${snippet}` : x`false`);
 				}
+
+				updates.push(
+					unchanged
+					 ? x`false`
+					 : condition
+					 ? x`${condition} && ${snippet}`
+					 : snippet
+				);
 			});
 
 		block.chunks.init.push(b`
@@ -701,26 +718,22 @@ export default class ElementWrapper extends Wrapper {
 			b`${fn}(${this.var}, ${data});`
 		);
 
-		block.chunks.update.push(b`
-			${fn}(${this.var}, ${data} = @get_spread_update(${levels}, [
-				${updates}
-			]));
-		`);
+		if (all_dependencies.size > 0) {
+			block.chunks.update.push(b`
+			if (${block.renderer.dirty(Array.from(all_dependencies))})
+				${fn}(${this.var}, @get_spread_update(${levels}, [
+					${updates}
+				]));
+			`);
+		}
 
 		// handle edge cases for elements
 		if (this.node.name === 'select') {
-			const dependencies = new Set();
-			for (const attr of this.attributes) {
-				for (const dep of attr.node.dependencies) {
-					dependencies.add(dep);
-				}
-			}
-
 			block.chunks.mount.push(b`
 				if (${data}.multiple) @select_options(${this.var}, ${data}.value);
 			`);
 			block.chunks.update.push(b`
-				if (${block.renderer.dirty(Array.from(dependencies))} && ${data}.multiple) @select_options(${this.var}, ${data}.value);
+				if (${block.renderer.dirty(Array.from(all_dependencies))} && ${data}.multiple) @select_options(${this.var}, ${data}.value);
 			`);
 		}
 	}

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

@@ -223,16 +223,17 @@ export default class InlineComponentWrapper extends Wrapper {
 				const all_dependencies: Set<string> = new Set();
 
 				this.node.attributes.forEach(attr => {
-					add_to_set(all_dependencies, attr.dependencies);
+					add_to_set(all_dependencies, attr.get_dependencies());
 				});
 
-				this.node.attributes.forEach((attr, i) => {
-					const { name, dependencies } = attr;
+				this.node.attributes.forEach((attr) => {
+					const { name } = attr;
+					const dependencies = attr.get_dependencies();
 
-					const condition = dependencies.size > 0 && (dependencies.size !== all_dependencies.size)
+					const condition = dependencies.length > 0 && (dependencies.length !== all_dependencies.size)
 						? renderer.dirty(Array.from(dependencies))
 						: null;
-					const unchanged = dependencies.size === 0;
+					const unchanged = dependencies.length === 0;
 
 					let change_object;
 					if (attr.is_spread) {

test/runtime/samples/spread-element-dynamic-non-object/_config.js

@@ -0,0 +1,47 @@
+export default {
+	props: {
+		props: {
+			foo: 'lol',
+			baz: 40 + 2,
+		}
+	},
+
+	html: `
+		<div baz="42" foo="lol" qux="named"></div>
+	`,
+
+	test({ assert, component, target }) {
+		const html = `
+			<div qux="named"></div>
+		`;
+
+		// test undefined
+		component.props = undefined;
+		assert.htmlEqual(target.innerHTML, html);
+
+		// set object props
+		component.props = this.props.props;
+		assert.htmlEqual(target.innerHTML, this.html);
+
+		// test null
+		component.props = null;
+		assert.htmlEqual(target.innerHTML, html);
+
+		// set object props
+		component.props = this.props.props;
+		assert.htmlEqual(target.innerHTML, this.html);
+
+		// test boolean
+		component.props = true;
+		assert.htmlEqual(target.innerHTML, html);
+
+		// set object props
+		component.props = this.props.props;
+		assert.htmlEqual(target.innerHTML, this.html);
+
+		// test number
+		component.props = 123;
+		assert.htmlEqual(target.innerHTML, html);
+
+	}
+};

test/runtime/samples/spread-element-dynamic-non-object/main.svelte

@@ -0,0 +1,5 @@
+<script>
+	export let props;
+</script>
+
+<div {...props} qux="named"/>