From d38820f4ca20d8a9ae8e7ae76e7c0d9847cf9032 Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Tue, 10 Dec 2024 00:17:46 +0100 Subject: [PATCH 1/4] fix: make `defaultValue` work with spread --- .changeset/silent-tips-cover.md | 5 + .../client/dom/elements/attributes.js | 14 +- .../form-default-value-spread/_config.js | 265 ++++++++++++++++++ .../form-default-value-spread/main.svelte | 199 +++++++++++++ 4 files changed, 482 insertions(+), 1 deletion(-) create mode 100644 .changeset/silent-tips-cover.md create mode 100644 packages/svelte/tests/runtime-runes/samples/form-default-value-spread/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/form-default-value-spread/main.svelte diff --git a/.changeset/silent-tips-cover.md b/.changeset/silent-tips-cover.md new file mode 100644 index 000000000000..1f51572cda24 --- /dev/null +++ b/.changeset/silent-tips-cover.md @@ -0,0 +1,5 @@ +--- +'svelte': patch +--- + +fix: make `defaultValue` work with spread diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index 0276069eee49..3eacf010673e 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -347,15 +347,27 @@ export function set_attributes( } else if (key === '__value' || (key === 'value' && value != null)) { // @ts-ignore element.value = element[key] = element.__value = value; + } else if (key === 'defaultValue') { + /** @type {HTMLInputElement} */ (element).defaultValue = value; + } else if (key === 'defaultChecked') { + /** @type {HTMLInputElement} */ (element).defaultChecked = value; + } else if (key === 'selected' && is_option_element) { + set_selected(/** @type {HTMLOptionElement} */ (element), value); } else { var name = key; if (!preserve_attribute_case) { name = normalize_attribute(name); } - if (value == null && !is_custom_element) { attributes[key] = null; + let default_value_reset = /**@type {HTMLInputElement}*/ (element).defaultValue; + let default_checked_reset = /**@type {HTMLInputElement}*/ (element).defaultChecked; element.removeAttribute(key); + if (key === 'value') { + /**@type {HTMLInputElement}*/ (element).defaultValue = default_value_reset; + } else if (key === 'checked') { + /**@type {HTMLInputElement}*/ (element).defaultChecked = default_checked_reset; + } } else if (setters.includes(name) && (is_custom_element || typeof value !== 'string')) { // @ts-ignore element[name] = value; diff --git a/packages/svelte/tests/runtime-runes/samples/form-default-value-spread/_config.js b/packages/svelte/tests/runtime-runes/samples/form-default-value-spread/_config.js new file mode 100644 index 000000000000..1ada598f443b --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/form-default-value-spread/_config.js @@ -0,0 +1,265 @@ +import { test } from '../../test'; +import { flushSync } from 'svelte'; + +export default test({ + async test({ assert, target }) { + /** + * @param {NodeListOf} inputs + * @param {string} field + * @param {any | any[]} value + */ + function check_inputs(inputs, field, value) { + for (let i = 0; i < inputs.length; i++) { + assert.equal(inputs[i][field], Array.isArray(value) ? value[i] : value, `field ${i}`); + } + } + + /** + * @param {any} input + * @param {string} field + * @param {any} value + */ + function set_input(input, field, value) { + input[field] = value; + input.dispatchEvent( + new Event(typeof value === 'boolean' ? 'change' : 'input', { bubbles: true }) + ); + } + + /** + * @param {HTMLOptionElement} option + */ + function select_option(option) { + option.selected = true; + option.dispatchEvent(new Event('change', { bubbles: true })); + } + + const after_reset = []; + + const reset = /** @type {HTMLInputElement} */ (target.querySelector('input[type=reset]')); + const [test1, test2, test3, test4, test5, test6, test7, test14] = + target.querySelectorAll('div'); + const [test8, test9, test10, test11] = target.querySelectorAll('select'); + const [ + test1_span, + test2_span, + test3_span, + test4_span, + test5_span, + test6_span, + test7_span, + test8_span, + test9_span, + test10_span, + test11_span + ] = target.querySelectorAll('span'); + + { + /** @type {NodeListOf} */ + const inputs = test1.querySelectorAll('input, textarea'); + check_inputs(inputs, 'value', 'x'); + assert.htmlEqual(test1_span.innerHTML, 'x x x x'); + + for (const input of inputs) { + set_input(input, 'value', 'foo'); + } + flushSync(); + check_inputs(inputs, 'value', 'foo'); + assert.htmlEqual(test1_span.innerHTML, 'foo foo foo foo'); + + after_reset.push(() => { + console.log('-------------'); + check_inputs(inputs, 'value', 'x'); + assert.htmlEqual(test1_span.innerHTML, 'x x x x'); + }); + } + + { + /** @type {NodeListOf} */ + const inputs = test2.querySelectorAll('input, textarea'); + check_inputs(inputs, 'value', 'x'); + assert.htmlEqual(test2_span.innerHTML, 'x x x x'); + + for (const input of inputs) { + set_input(input, 'value', 'foo'); + } + flushSync(); + check_inputs(inputs, 'value', 'foo'); + assert.htmlEqual(test2_span.innerHTML, 'foo foo foo foo'); + + after_reset.push(() => { + console.log('-------------'); + check_inputs(inputs, 'value', 'x'); + assert.htmlEqual(test2_span.innerHTML, 'x x x x'); + }); + } + + { + /** @type {NodeListOf} */ + const inputs = test3.querySelectorAll('input, textarea'); + check_inputs(inputs, 'value', 'y'); + assert.htmlEqual(test3_span.innerHTML, 'y y y y'); + + for (const input of inputs) { + set_input(input, 'value', 'foo'); + } + flushSync(); + check_inputs(inputs, 'value', 'foo'); + assert.htmlEqual(test3_span.innerHTML, 'foo foo foo foo'); + + after_reset.push(() => { + check_inputs(inputs, 'value', 'x'); + assert.htmlEqual(test3_span.innerHTML, 'x x x x'); + }); + } + + { + /** @type {NodeListOf} */ + const inputs = test4.querySelectorAll('input'); + check_inputs(inputs, 'checked', true); + assert.htmlEqual(test4_span.innerHTML, 'true true'); + + for (const input of inputs) { + set_input(input, 'checked', false); + } + flushSync(); + check_inputs(inputs, 'checked', false); + assert.htmlEqual(test4_span.innerHTML, 'false false'); + + after_reset.push(() => { + check_inputs(inputs, 'checked', true); + assert.htmlEqual(test4_span.innerHTML, 'true true'); + }); + } + + { + /** @type {NodeListOf} */ + const inputs = test5.querySelectorAll('input'); + check_inputs(inputs, 'checked', true); + assert.htmlEqual(test5_span.innerHTML, 'true true'); + + for (const input of inputs) { + set_input(input, 'checked', false); + } + flushSync(); + check_inputs(inputs, 'checked', false); + assert.htmlEqual(test5_span.innerHTML, 'false false'); + + after_reset.push(() => { + check_inputs(inputs, 'checked', true); + assert.htmlEqual(test5_span.innerHTML, 'true true'); + }); + } + + { + /** @type {NodeListOf} */ + const inputs = test6.querySelectorAll('input'); + check_inputs(inputs, 'checked', false); + assert.htmlEqual(test6_span.innerHTML, 'false false'); + + after_reset.push(() => { + check_inputs(inputs, 'checked', true); + assert.htmlEqual(test6_span.innerHTML, 'true true'); + }); + } + { + /** @type {NodeListOf} */ + const inputs = test7.querySelectorAll('input'); + check_inputs(inputs, 'checked', true); + assert.htmlEqual(test7_span.innerHTML, 'true'); + + after_reset.push(() => { + check_inputs(inputs, 'checked', false); + assert.htmlEqual(test7_span.innerHTML, 'false'); + }); + } + + { + /** @type {NodeListOf} */ + const options = test8.querySelectorAll('option'); + check_inputs(options, 'selected', [false, true, false]); + assert.htmlEqual(test8_span.innerHTML, 'b'); + + select_option(options[2]); + flushSync(); + check_inputs(options, 'selected', [false, false, true]); + assert.htmlEqual(test8_span.innerHTML, 'c'); + + after_reset.push(() => { + check_inputs(options, 'selected', [false, true, false]); + assert.htmlEqual(test8_span.innerHTML, 'b'); + }); + } + + { + /** @type {NodeListOf} */ + const options = test9.querySelectorAll('option'); + check_inputs(options, 'selected', [false, true, false]); + assert.htmlEqual(test9_span.innerHTML, 'b'); + + select_option(options[2]); + flushSync(); + check_inputs(options, 'selected', [false, false, true]); + assert.htmlEqual(test9_span.innerHTML, 'c'); + + after_reset.push(() => { + check_inputs(options, 'selected', [false, true, false]); + assert.htmlEqual(test9_span.innerHTML, 'b'); + }); + } + + { + /** @type {NodeListOf} */ + const options = test10.querySelectorAll('option'); + check_inputs(options, 'selected', [false, false, true]); + assert.htmlEqual(test10_span.innerHTML, 'c'); + + select_option(options[0]); + flushSync(); + check_inputs(options, 'selected', [true, false, false]); + assert.htmlEqual(test10_span.innerHTML, 'a'); + + after_reset.push(() => { + check_inputs(options, 'selected', [false, true, false]); + assert.htmlEqual(test10_span.innerHTML, 'b'); + }); + } + + { + /** @type {NodeListOf} */ + const options = test11.querySelectorAll('option'); + check_inputs(options, 'selected', [false, false, true]); + assert.htmlEqual(test11_span.innerHTML, 'c'); + + select_option(options[0]); + flushSync(); + check_inputs(options, 'selected', [true, false, false]); + assert.htmlEqual(test11_span.innerHTML, 'a'); + + after_reset.push(() => { + check_inputs(options, 'selected', [false, true, false]); + assert.htmlEqual(test11_span.innerHTML, 'b'); + }); + } + + { + /** @type {NodeListOf} */ + const inputs = test14.querySelectorAll('input, textarea'); + assert.equal(inputs[0].value, 'x'); + assert.equal(/** @type {HTMLInputElement} */ (inputs[1]).checked, true); + // this is still missing...i have no idea how to fix this lol + // assert.equal(inputs[2].value, 'x'); + + after_reset.push(() => { + assert.equal(inputs[0].value, 'y'); + assert.equal(/** @type {HTMLInputElement} */ (inputs[1]).checked, false); + assert.equal(inputs[2].value, 'y'); + }); + } + + reset.click(); + await Promise.resolve(); + flushSync(); + after_reset.forEach((fn) => fn()); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/form-default-value-spread/main.svelte b/packages/svelte/tests/runtime-runes/samples/form-default-value-spread/main.svelte new file mode 100644 index 000000000000..3fc7ef11a5e9 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/form-default-value-spread/main.svelte @@ -0,0 +1,199 @@ + + +
+

Input/Textarea value

+ +
+ + + + + + + + +
+ + +
+ + + + + + + + +
+ + +
+ + + + + + + + +
+ +

Input checked

+ +
+ + + + +
+ + +
+ + + + +
+ + +
+ + + + +
+ + +
+ + +
+ + + +

Select (single)

+ + + + + + + + + + + + +

Select (multiple)

+ + + + + + + + +

Static values

+
+ + + +
+ + +
+ +

+ Bound values: + {value1} {value3} {value6} {value8} + {value9} {value12} {value14} {value16} + {value17} {value20} {value22} {value24} + {checked2} {checked4} + {checked6} {checked8} + {checked10} {checked12} + {checked14} + {selected1} + {selected2} + {selected3} + {selected4} + {selected5} + {selected6} +

From ce373ff8fc31d7544aae8090960e5abd3ebb3acb Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Tue, 10 Dec 2024 10:50:56 +0100 Subject: [PATCH 2/4] chore: apply suggestions from review --- .../internal/client/dom/elements/attributes.js | 15 +++++++++------ .../samples/form-default-value-spread/_config.js | 2 -- .../samples/form-default-value/_config.js | 2 -- 3 files changed, 9 insertions(+), 10 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index 3eacf010673e..ec57917e7696 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -347,10 +347,6 @@ export function set_attributes( } else if (key === '__value' || (key === 'value' && value != null)) { // @ts-ignore element.value = element[key] = element.__value = value; - } else if (key === 'defaultValue') { - /** @type {HTMLInputElement} */ (element).defaultValue = value; - } else if (key === 'defaultChecked') { - /** @type {HTMLInputElement} */ (element).defaultChecked = value; } else if (key === 'selected' && is_option_element) { set_selected(/** @type {HTMLOptionElement} */ (element), value); } else { @@ -358,8 +354,12 @@ export function set_attributes( if (!preserve_attribute_case) { name = normalize_attribute(name); } - if (value == null && !is_custom_element) { + let is_default_value_or_checked = name === 'defaultValue' || name === 'defaultChecked'; + + if (value == null && !is_custom_element && !is_default_value_or_checked) { attributes[key] = null; + // if we remove the value/checked attributes this also for some reasons reset + // the default value so we need to keep track of it and reassign it after the remove let default_value_reset = /**@type {HTMLInputElement}*/ (element).defaultValue; let default_checked_reset = /**@type {HTMLInputElement}*/ (element).defaultChecked; element.removeAttribute(key); @@ -368,7 +368,10 @@ export function set_attributes( } else if (key === 'checked') { /**@type {HTMLInputElement}*/ (element).defaultChecked = default_checked_reset; } - } else if (setters.includes(name) && (is_custom_element || typeof value !== 'string')) { + } else if ( + is_default_value_or_checked || + (setters.includes(name) && (is_custom_element || typeof value !== 'string')) + ) { // @ts-ignore element[name] = value; } else if (typeof value !== 'function') { diff --git a/packages/svelte/tests/runtime-runes/samples/form-default-value-spread/_config.js b/packages/svelte/tests/runtime-runes/samples/form-default-value-spread/_config.js index 1ada598f443b..26e90e431b54 100644 --- a/packages/svelte/tests/runtime-runes/samples/form-default-value-spread/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/form-default-value-spread/_config.js @@ -68,7 +68,6 @@ export default test({ assert.htmlEqual(test1_span.innerHTML, 'foo foo foo foo'); after_reset.push(() => { - console.log('-------------'); check_inputs(inputs, 'value', 'x'); assert.htmlEqual(test1_span.innerHTML, 'x x x x'); }); @@ -88,7 +87,6 @@ export default test({ assert.htmlEqual(test2_span.innerHTML, 'foo foo foo foo'); after_reset.push(() => { - console.log('-------------'); check_inputs(inputs, 'value', 'x'); assert.htmlEqual(test2_span.innerHTML, 'x x x x'); }); diff --git a/packages/svelte/tests/runtime-runes/samples/form-default-value/_config.js b/packages/svelte/tests/runtime-runes/samples/form-default-value/_config.js index 5ef72aaa8ec2..35ab6e8ece44 100644 --- a/packages/svelte/tests/runtime-runes/samples/form-default-value/_config.js +++ b/packages/svelte/tests/runtime-runes/samples/form-default-value/_config.js @@ -68,7 +68,6 @@ export default test({ assert.htmlEqual(test1_span.innerHTML, 'foo foo foo foo'); after_reset.push(() => { - console.log('-------------'); check_inputs(inputs, 'value', 'x'); assert.htmlEqual(test1_span.innerHTML, 'x x x x'); }); @@ -88,7 +87,6 @@ export default test({ assert.htmlEqual(test2_span.innerHTML, 'foo foo foo foo'); after_reset.push(() => { - console.log('-------------'); check_inputs(inputs, 'value', 'x'); assert.htmlEqual(test2_span.innerHTML, 'x x x x'); }); From 27f6cbbca9b9d1bbc5b7c534ed5a7435b495ceca Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 12 Dec 2024 14:09:12 -0500 Subject: [PATCH 3/4] tweak --- .../client/dom/elements/attributes.js | 21 +++++++++++-------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index ec57917e7696..d1989fa9ad7e 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -354,22 +354,25 @@ export function set_attributes( if (!preserve_attribute_case) { name = normalize_attribute(name); } - let is_default_value_or_checked = name === 'defaultValue' || name === 'defaultChecked'; - if (value == null && !is_custom_element && !is_default_value_or_checked) { + var is_default = name === 'defaultValue' || name === 'defaultChecked'; + + if (value == null && !is_custom_element && !is_default) { attributes[key] = null; - // if we remove the value/checked attributes this also for some reasons reset - // the default value so we need to keep track of it and reassign it after the remove - let default_value_reset = /**@type {HTMLInputElement}*/ (element).defaultValue; - let default_checked_reset = /**@type {HTMLInputElement}*/ (element).defaultChecked; + + // removing value/checked also removes defaultValue/defaultChecked — preserve + let default_value = /** @type {HTMLInputElement} */ (element).defaultValue; + let default_checked = /** @type {HTMLInputElement} */ (element).defaultChecked; + element.removeAttribute(key); + if (key === 'value') { - /**@type {HTMLInputElement}*/ (element).defaultValue = default_value_reset; + /** @type {HTMLInputElement} */ (element).defaultValue = default_value; } else if (key === 'checked') { - /**@type {HTMLInputElement}*/ (element).defaultChecked = default_checked_reset; + /** @type {HTMLInputElement} */ (element).defaultChecked = default_checked; } } else if ( - is_default_value_or_checked || + is_default || (setters.includes(name) && (is_custom_element || typeof value !== 'string')) ) { // @ts-ignore From 12780ed8635fdffce55fa304963583bfbc6bc852 Mon Sep 17 00:00:00 2001 From: Rich Harris Date: Thu, 12 Dec 2024 14:14:14 -0500 Subject: [PATCH 4/4] only stash defaults when relevant --- .../client/dom/elements/attributes.js | 25 +++++++++++-------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index d1989fa9ad7e..b283fca8cd7b 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -360,16 +360,21 @@ export function set_attributes( if (value == null && !is_custom_element && !is_default) { attributes[key] = null; - // removing value/checked also removes defaultValue/defaultChecked — preserve - let default_value = /** @type {HTMLInputElement} */ (element).defaultValue; - let default_checked = /** @type {HTMLInputElement} */ (element).defaultChecked; - - element.removeAttribute(key); - - if (key === 'value') { - /** @type {HTMLInputElement} */ (element).defaultValue = default_value; - } else if (key === 'checked') { - /** @type {HTMLInputElement} */ (element).defaultChecked = default_checked; + if (name === 'value' || name === 'checked') { + // removing value/checked also removes defaultValue/defaultChecked — preserve + let input = /** @type {HTMLInputElement} */ (element); + + if (name === 'value') { + let prev = input.defaultValue; + input.removeAttribute(name); + input.defaultValue = prev; + } else { + let prev = input.defaultChecked; + input.removeAttribute(name); + input.defaultChecked = prev; + } + } else { + element.removeAttribute(key); } } else if ( is_default ||