set select multiple value with spread (#4894)

Author: tanhauhau

CHANGELOG.md

@@ -4,6 +4,7 @@
 
 * Update `<select>` with `bind:value` when the available `<option>`s change ([#1764](https://github.com/sveltejs/svelte/issues/1764))
 * Fix inconsistencies when setting a two-way bound `<input>` to `undefined` ([#3569](https://github.com/sveltejs/svelte/issues/3569))
+* Fix setting `<select multiple>` when there are spread attributes ([#4392](https://github.com/sveltejs/svelte/issues/4392))
 * Fix resize listening on certain older browsers ([#4752](https://github.com/sveltejs/svelte/issues/4752))
 * Add `a11y-no-onchange` warning ([#4788](https://github.com/sveltejs/svelte/pull/4788))
 * Add `muted` binding for media elements ([#2998](https://github.com/sveltejs/svelte/issues/2998))

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

@@ -102,25 +102,12 @@ export default class AttributeWrapper {
 		} 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}
-				}
-			`;
+
+			if (is_multiple_select) {
+				updater = b`@select_options(${element.var}, ${last});`;
+			} else {
+				updater = b`@select_option(${element.var}, ${last});`;
+			}
 
 			block.chunks.mount.push(b`
 				${last} = ${value};

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

@@ -705,10 +705,27 @@ export default class ElementWrapper extends Wrapper {
 		);
 
 		block.chunks.update.push(b`
-			${fn}(${this.var}, @get_spread_update(${levels}, [
+			${fn}(${this.var}, ${data} = @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);
+			`);
+		}
 	}
 
 	add_transitions(

test/js/samples/select-dynamic-value/expected.js

@@ -7,7 +7,8 @@ import {
 	init,
 	insert,
 	noop,
-	safe_not_equal
+	safe_not_equal,
+	select_option
 } from "svelte/internal";
 
 function create_fragment(ctx) {
@@ -33,26 +34,11 @@ function create_fragment(ctx) {
 			append(select, option0);
 			append(select, option1);
 			select_value_value = /*current*/ ctx[0];
-
-			for (var i = 0; i < select.options.length; i += 1) {
-				var option = select.options[i];
-
-				if (option.__value === select_value_value) {
-					option.selected = true;
-					break;
-				}
-			}
+			select_option(select, select_value_value);
 		},
 		p(ctx, [dirty]) {
 			if (dirty & /*current*/ 1 && select_value_value !== (select_value_value = /*current*/ ctx[0])) {
-				for (var i = 0; i < select.options.length; i += 1) {
-					var option = select.options[i];
-
-					if (option.__value === select_value_value) {
-						option.selected = true;
-						break;
-					}
-				}
+				select_option(select, select_value_value);
 			}
 		},
 		i: noop,

test/runtime/samples/spread-element-input-seelct-multiple/_config.js

@@ -0,0 +1,39 @@
+export default {
+	async test({ assert, component, target, window }) {
+		const [input1, input2] = target.querySelectorAll('input');
+		const select = target.querySelector('select');
+		const [option1, option2] = select.childNodes;
+
+		let selections = Array.from(select.selectedOptions);
+		assert.equal(selections.length, 2);
+		assert.ok(selections.includes(option1));
+		assert.ok(selections.includes(option2));
+
+		const event = new window.Event('change');
+
+		input1.checked = false;
+		await input1.dispatchEvent(event);
+
+		selections = Array.from(select.selectedOptions);
+		assert.equal(selections.length, 1);
+		assert.ok(!selections.includes(option1));
+		assert.ok(selections.includes(option2));
+
+		input2.checked = false;
+		await input2.dispatchEvent(event);
+		input1.checked = true;
+		await input1.dispatchEvent(event);
+
+		selections = Array.from(select.selectedOptions);
+		assert.equal(selections.length, 1);
+		assert.ok(selections.includes(option1));
+		assert.ok(!selections.includes(option2));
+
+		component.spread = { value: ['Hello', 'World'] };
+
+		selections = Array.from(select.selectedOptions);
+		assert.equal(selections.length, 2);
+		assert.ok(selections.includes(option1));
+		assert.ok(selections.includes(option2));
+	}
+};

test/runtime/samples/spread-element-input-seelct-multiple/main.svelte

@@ -0,0 +1,12 @@
+<script>
+  let value = ['Hello', 'World'];
+  export let spread = {};
+</script>
+
+<select multiple {value} {...spread}>
+  <option>Hello</option>
+  <option>World</option>
+</select>
+
+<input type="checkbox" value="Hello" bind:group={value}>
+<input type="checkbox" value="World" bind:group={value}>