Skip to content

Commit 7e55810

Browse files
committed
remove edge case handling in favor of smaller code size
1 parent 08b9599 commit 7e55810

File tree

3 files changed

+7
-46
lines changed

3 files changed

+7
-46
lines changed

packages/svelte/src/internal/client/dom/elements/attributes.js

Lines changed: 7 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -159,16 +159,7 @@ export function set_attributes(element, prev, next, lowercase_attributes, css_ha
159159

160160
for (var key in prev) {
161161
if (!(key in next)) {
162-
if (is_option_element && key === 'value') {
163-
// Because of the "set falsy option values to the empty string" logic below, which can't
164-
// differentiate between a missing value and an explicitly set value of null or undefined,
165-
// we need to remove the attribute here and delete the key from the object.
166-
element.removeAttribute(key);
167-
delete current[key];
168-
delete next[key];
169-
} else {
170-
next[key] = null;
171-
}
162+
next[key] = null;
172163
}
173164
}
174165

@@ -196,6 +187,12 @@ export function set_attributes(element, prev, next, lowercase_attributes, css_ha
196187
// the value is set to the text content of the option element, and setting the value
197188
// to null or undefined means the value is set to the string "null" or "undefined".
198189
// To align with how we handle this case in non-spread-scenarios, this logic is needed.
190+
// There's a super-edge-case bug here that is left in in favor of smaller code size:
191+
// Because of the "set missing props to null" logic above, we can't differentiate
192+
// between a missing value and an explicitly set value of null or undefined. That means
193+
// that once set, the value attribute of an <option> element can't be removed. This is
194+
// a very rare edge case, and removing the attribute altogether isn't possible either
195+
// for the <option value={undefined}> case, so we're not losing any functionality here.
199196
// @ts-ignore
200197
element.value = element.__value = '';
201198
current[key] = value;

packages/svelte/tests/runtime-runes/samples/select-falsy-value/_config.js

Lines changed: 0 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,3 @@
1-
import { flushSync } from 'svelte';
21
import { test } from '../../test';
32

43
// <option value> is special because falsy values should result in an empty string value attribute
@@ -27,39 +26,6 @@ export default test({
2726
<select>
2827
<option value="">Default</option>
2928
</select>
30-
31-
<button>update reactive spread</button>
32-
`
33-
);
34-
35-
const btn = target.querySelector('button');
36-
btn?.click();
37-
flushSync();
38-
39-
assert.htmlEqual(
40-
target.innerHTML,
41-
`
42-
<select>
43-
<option value="">Default</option>
44-
</select>
45-
46-
<select>
47-
<option value="">Default</option>
48-
</select>
49-
50-
<select>
51-
<option value="">Default</option>
52-
</select>
53-
54-
<select>
55-
<option value="">Default</option>
56-
</select>
57-
58-
<select>
59-
<option>Default</option>
60-
</select>
61-
62-
<button>update reactive spread</button>
6329
`
6430
);
6531
}

packages/svelte/tests/runtime-runes/samples/select-falsy-value/main.svelte

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -24,5 +24,3 @@
2424
<select>
2525
<option {...reactive_spread}>Default</option>
2626
</select>
27-
28-
<button onclick={() => (reactive_spread = {})}>update reactive spread</button>

0 commit comments

Comments
 (0)