fix handling of style scoping and class: with spread scopes (#3792)

CHANGELOG.md

@@ -4,8 +4,8 @@
 
 * Fix `{#each}` context not shadowing outer scope when using `bind:` ([#1565](https://github.com/sveltejs/svelte/issues/1565))
 * Fix edge cases in matching selectors against elements ([#1710](https://github.com/sveltejs/svelte/issues/1710))
+* Fix several bugs related to interaction of `{...spread}` attributes with other features ([#2721](https://github.com/sveltejs/svelte/issues/2721), [#3421](https://github.com/sveltejs/svelte/issues/3421), [#3681](https://github.com/sveltejs/svelte/issues/3681), [#3764](https://github.com/sveltejs/svelte/issues/3764), [#3790](https://github.com/sveltejs/svelte/issues/3790))
 * Allow exiting a reactive block early with `break $` ([#2828](https://github.com/sveltejs/svelte/issues/2828))
-* Don't lose `class:` directive classes on an element with `{...spread}` attributes when updating ([#3421](https://github.com/sveltejs/svelte/issues/3421))
 * Fix application of style scoping class in cases of ambiguity ([#3544](https://github.com/sveltejs/svelte/issues/3544))
 * Check attributes have changed before setting them to avoid image flicker ([#3579](https://github.com/sveltejs/svelte/pull/3579))
 * Fix generating malformed code for `{@debug}` tags with no dependencies ([#3588](https://github.com/sveltejs/svelte/issue/3588))
@@ -19,8 +19,6 @@
 * Flush changes in newly attached block when using `{#await}` ([#3660](https://github.com/sveltejs/svelte/issues/3660))
 * Throw exception immediately when calling `createEventDispatcher()` after component instantiation ([#3667](https://github.com/sveltejs/svelte/pull/3667))
 * Fix globals shadowing contextual template scope ([#3674](https://github.com/sveltejs/svelte/issues/3674))
-* Fix error resulting from trying to set a read-only property when spreading element attributes ([#3681](https://github.com/sveltejs/svelte/issues/3681))
-* Fix handling of boolean attributes in presence of other spread attributes ([#3764](https://github.com/sveltejs/svelte/issues/3764))
 
 ## 3.12.1
 

src/compiler/compile/nodes/Element.ts

@@ -105,6 +105,7 @@ export default class Element extends Node {
 	animation?: Animation = null;
 	children: INode[];
 	namespace: string;
+	needs_manual_style_scoping: boolean;
 
 	constructor(component, parent, scope, info: any) {
 		super(component, parent, scope, info);
@@ -712,6 +713,11 @@ export default class Element extends Node {
 	}
 
 	add_css_class() {
+		if (this.attributes.some(attr => attr.is_spread)) {
+			this.needs_manual_style_scoping = true;
+			return;
+		}
+
 		const { id } = this.component.stylesheet;
 
 		const class_attribute = this.attributes.find(a => a.name === 'class');

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

@@ -344,6 +344,7 @@ export default class ElementWrapper extends Wrapper {
 		this.add_animation(block);
 		this.add_actions(block);
 		this.add_classes(block);
+		this.add_manual_style_scoping(block);
 
 		if (nodes && this.renderer.options.hydratable) {
 			block.chunks.claim.push(
@@ -838,6 +839,14 @@ export default class ElementWrapper extends Wrapper {
 			}
 		});
 	}
+
+	add_manual_style_scoping(block) {
+		if (this.node.needs_manual_style_scoping) {
+			const updater = b`@toggle_class(${this.var}, "${this.node.component.stylesheet.id}", true);`;
+			block.chunks.hydrate.push(updater);
+			block.chunks.update.push(updater);
+		}
+	}
 }
 
 function to_html(wrappers: Array<ElementWrapper | TextWrapper>, block: Block, literal: any, state: any) {

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

@@ -1,5 +1,4 @@
 import { is_void } from '../../../utils/names';
-import Class from '../../nodes/Class';
 import { get_attribute_value, get_class_attribute_value } from './shared/get_attribute_value';
 import { get_slot_scope } from './shared/get_slot_scope';
 import Renderer, { RenderOptions } from '../Renderer';
@@ -69,15 +68,17 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
 
 	renderer.add_string(`<${node.name}`);
 
-	const class_expression = node.classes.length > 0 && node.classes
-		.map((class_directive: Class) => {
-			const { expression, name } = class_directive;
-			const snippet = expression ? expression.node : x`#ctx.${name}`;
-			return x`${snippet} ? "${name}" : ""`;
-		})
-		.reduce((lhs, rhs) => x`${lhs} + ' ' + ${rhs}`);
-
-	let add_class_attribute = class_expression ? true : false;
+	const class_expression_list = node.classes.map(class_directive => {
+		const { expression, name } = class_directive;
+		const snippet = expression ? expression.node : x`#ctx.${name}`;
+		return x`${snippet} ? "${name}" : ""`;
+	});
+	if (node.needs_manual_style_scoping) {
+		class_expression_list.push(x`"${node.component.stylesheet.id}"`);
+	}
+	const class_expression =
+		class_expression_list.length > 0 &&
+		class_expression_list.reduce((lhs, rhs) => x`${lhs} + ' ' + ${rhs}`);
 
 	if (node.attributes.some(attr => attr.is_spread)) {
 		// TODO dry this out
@@ -98,17 +99,15 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
 				) {
 					// a boolean attribute with one non-Text chunk
 					args.push(x`{ ${attribute.name}: ${(attribute.chunks[0] as Expression).node} || null }`);
-				} else if (name === 'class' && class_expression) {
-					// Add class expression
-					args.push(x`{ ${attribute.name}: [${get_class_attribute_value(attribute)}, ${class_expression}].join(' ').trim() }`);
 				} else {
-					args.push(x`{ ${attribute.name}: ${(name === 'class' ? get_class_attribute_value : get_attribute_value)(attribute)} }`);
+					args.push(x`{ ${attribute.name}: ${get_attribute_value(attribute)} }`);
 				}
 			}
 		});
 
-		renderer.add_expression(x`@spread([${args}])`);
+		renderer.add_expression(x`@spread([${args}], ${class_expression});`);
 	} else {
+		let add_class_attribute = !!class_expression;
 		node.attributes.forEach(attribute => {
 			const name = attribute.name.toLowerCase();
 			if (name === 'value' && node.name.toLowerCase() === 'textarea') {
@@ -137,6 +136,9 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
 				renderer.add_string(`"`);
 			}
 		});
+		if (add_class_attribute) {
+			renderer.add_expression(x`@add_classes([${class_expression}].join(' ').trim())`);
+		}
 	}
 
 	node.bindings.forEach(binding => {
@@ -162,10 +164,6 @@ export default function(node: Element, renderer: Renderer, options: RenderOption
 		}
 	});
 
-	if (add_class_attribute) {
-		renderer.add_expression(x`@add_classes([${class_expression}].join(' ').trim())`);
-	}
-
 	renderer.add_string('>');
 
 	if (node_contents !== undefined) {

src/runtime/internal/ssr.ts

@@ -5,8 +5,15 @@ export const invalid_attribute_name_character = /[\s'">/=\u{FDD0}-\u{FDEF}\u{FFF
 // https://html.spec.whatwg.org/multipage/syntax.html#attributes-2
 // https://infra.spec.whatwg.org/#noncharacter
 
-export function spread(args) {
+export function spread(args, classes_to_add) {
 	const attributes = Object.assign({}, ...args);
+	if (classes_to_add) {
+		if (attributes.class == null) {
+			attributes.class = classes_to_add;
+		} else {
+			attributes.class += ' ' + classes_to_add;
+		}
+	}
 	let str = '';
 
 	Object.keys(attributes).forEach(name => {

test/runtime/samples/spread-element-class/main.svelte

@@ -2,4 +2,4 @@
 	export let blah = 'hello';
 </script>
 
-<div class='foo' class:bar={true} {...{}}>{blah}</div>
+<div class:bar={true} {...{ class: 'foo' }}>{blah}</div>

test/runtime/samples/spread-element-scope/_config.js

@@ -0,0 +1,7 @@
+export default {
+	html: `
+		<div class="foo svelte-xg5rbo">red</div>
+		<div class="qux svelte-xg5rbo">red</div>
+		<div class="bar svelte-xg5rbo">red and bold</div>
+	`
+};

test/runtime/samples/spread-element-scope/main.svelte

@@ -0,0 +1,10 @@
+<style>
+	div { color: red; }
+	.bar { font-weight: bold; }
+</style>
+
+<div {...{ class: 'bar' }} class='foo'>red</div>
+
+<div class='foo' {...{ class: 'qux' }}>red</div>
+
+<div {...{ class: 'bar' }}>red and bold</div>