From 4010913b4930d8a175c99545545246a8581b1967 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 13 Nov 2024 11:04:48 +0100 Subject: [PATCH 01/15] tests --- .../samples/form-default-value/_config.js | 245 ++++++++++++++++++ .../samples/form-default-value/main.svelte | 224 ++++++++++++++++ 2 files changed, 469 insertions(+) create mode 100644 packages/svelte/tests/runtime-runes/samples/form-default-value/_config.js create mode 100644 packages/svelte/tests/runtime-runes/samples/form-default-value/main.svelte 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 new file mode 100644 index 000000000000..5f18836e7377 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/form-default-value/_config.js @@ -0,0 +1,245 @@ +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('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, + test8, + test9, + test10, + test11, + test12, + test13 + ] = target.querySelectorAll('div'); + const [test14, test15, test16, test17] = target.querySelectorAll('select'); + + { + /** @type {NodeListOf} */ + const inputs = test1.querySelectorAll('input, textarea'); + check_inputs(inputs, 'value', 'x'); + + for (const input of inputs) { + set_input(input, 'value', 'foo'); + } + flushSync(); + check_inputs(inputs, 'value', 'foo'); + + after_reset.push(() => check_inputs(inputs, 'value', 'x')); + } + + { + /** @type {NodeListOf} */ + const inputs = test2.querySelectorAll('input, textarea'); + check_inputs(inputs, 'value', 'y'); + + for (const input of inputs) { + set_input(input, 'value', 'foo'); + } + flushSync(); + check_inputs(inputs, 'value', 'foo'); + + after_reset.push(() => check_inputs(inputs, 'value', 'x')); + } + + { + /** @type {NodeListOf} */ + const inputs = test3.querySelectorAll('input'); + check_inputs(inputs, 'checked', true); + + for (const input of inputs) { + set_input(input, 'checked', false); + } + flushSync(); + check_inputs(inputs, 'checked', false); + + after_reset.push(() => check_inputs(inputs, 'checked', true)); + } + + { + /** @type {NodeListOf} */ + const inputs = test4.querySelectorAll('input'); + check_inputs(inputs, 'checked', false); + + after_reset.push(() => check_inputs(inputs, 'checked', true)); + } + + { + /** @type {NodeListOf} */ + const inputs = test5.querySelectorAll('input'); + check_inputs(inputs, 'checked', true); + + after_reset.push(() => check_inputs(inputs, 'checked', false)); + } + + { + /** @type {NodeListOf} */ + const inputs = test5.querySelectorAll('input'); + check_inputs(inputs, 'checked', true); + + after_reset.push(() => check_inputs(inputs, 'checked', false)); + } + + { + /** @type {NodeListOf} */ + const inputs = test6.querySelectorAll('input'); + check_inputs(inputs, 'checked', [false, true, false]); + + set_input(inputs[0], 'checked', true); + flushSync(); + check_inputs(inputs, 'checked', [true, false, false]); + + after_reset.push(() => check_inputs(inputs, 'checked', [false, true, false])); + } + + { + /** @type {NodeListOf} */ + const inputs = test7.querySelectorAll('input'); + check_inputs(inputs, 'checked', [false, true, false]); + + set_input(inputs[0], 'checked', true); + flushSync(); + check_inputs(inputs, 'checked', [true, false, false]); + + after_reset.push(() => check_inputs(inputs, 'checked', [false, true, false])); + } + + { + /** @type {NodeListOf} */ + const inputs = test8.querySelectorAll('input'); + check_inputs(inputs, 'checked', [true, false, false]); + + set_input(inputs[3], 'checked', true); + flushSync(); + check_inputs(inputs, 'checked', [false, false, true]); + + after_reset.push(() => check_inputs(inputs, 'checked', [false, true, false])); + } + + { + /** @type {NodeListOf} */ + const inputs = test9.querySelectorAll('input'); + check_inputs(inputs, 'checked', [true, false, false]); + + set_input(inputs[3], 'checked', true); + flushSync(); + check_inputs(inputs, 'checked', [false, false, true]); + + after_reset.push(() => check_inputs(inputs, 'checked', [false, true, false])); + } + + { + /** @type {NodeListOf} */ + const inputs = test12.querySelectorAll('input'); + check_inputs(inputs, 'checked', [true, false, false]); + + set_input(inputs[3], 'checked', true); + flushSync(); + check_inputs(inputs, 'checked', [true, false, true]); + + after_reset.push(() => check_inputs(inputs, 'checked', [false, true, false])); + } + + { + /** @type {NodeListOf} */ + const inputs = test13.querySelectorAll('input'); + check_inputs(inputs, 'checked', [true, false, false]); + + set_input(inputs[3], 'checked', true); + flushSync(); + check_inputs(inputs, 'checked', [true, false, true]); + + after_reset.push(() => check_inputs(inputs, 'checked', [false, true, false])); + } + + { + /** @type {NodeListOf} */ + const options = test14.querySelectorAll('option'); + check_inputs(options, 'selected', [false, true, false]); + + select_option(options[3]); + flushSync(); + check_inputs(options, 'selected', [false, false, true]); + + after_reset.push(() => check_inputs(options, 'selected', [false, true, false])); + } + + { + /** @type {NodeListOf} */ + const options = test15.querySelectorAll('option'); + check_inputs(options, 'selected', [false, true, false]); + + select_option(options[3]); + flushSync(); + check_inputs(options, 'selected', [false, false, true]); + + after_reset.push(() => check_inputs(options, 'selected', [false, true, false])); + } + + { + /** @type {NodeListOf} */ + const options = test16.querySelectorAll('option'); + check_inputs(options, 'selected', [false, false, true]); + + select_option(options[0]); + flushSync(); + check_inputs(options, 'selected', [true, false, false]); + + after_reset.push(() => check_inputs(options, 'selected', [false, true, false])); + } + + { + /** @type {NodeListOf} */ + const options = test17.querySelectorAll('option'); + check_inputs(options, 'selected', [false, false, true]); + + select_option(options[0]); + flushSync(); + check_inputs(options, 'selected', [true, false, false]); + + after_reset.push(() => check_inputs(options, 'selected', [false, true, false])); + } + + reset.click(); + flushSync(); + after_reset.forEach((fn) => fn()); + } +}); diff --git a/packages/svelte/tests/runtime-runes/samples/form-default-value/main.svelte b/packages/svelte/tests/runtime-runes/samples/form-default-value/main.svelte new file mode 100644 index 000000000000..c27bd53796c2 --- /dev/null +++ b/packages/svelte/tests/runtime-runes/samples/form-default-value/main.svelte @@ -0,0 +1,224 @@ + + +
+

Input/Textarea value

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

Input checked

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

Input group (radio)

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

Input group (checkbox)

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

Select (single)

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

Select (multiple)

+ + + + + + + + + + + + + + +
From 6d0b3240fb9141e7f7ab527af283b2a5d0dac5da Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 13 Nov 2024 11:04:53 +0100 Subject: [PATCH 02/15] typings --- packages/svelte/elements.d.ts | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/packages/svelte/elements.d.ts b/packages/svelte/elements.d.ts index 8746b29e250a..75c13284897b 100644 --- a/packages/svelte/elements.d.ts +++ b/packages/svelte/elements.d.ts @@ -1085,6 +1085,11 @@ export interface HTMLInputAttributes extends HTMLAttributes { step?: number | string | undefined | null; type?: HTMLInputTypeAttribute | undefined | null; value?: any; + // needs both casing variants because language tools does lowercase names of non-shorthand attributes + defaultValue?: any; + defaultvalue?: any; + defaultChecked?: any; + defaultchecked?: any; width?: number | string | undefined | null; webkitdirectory?: boolean | undefined | null; @@ -1366,6 +1371,9 @@ export interface HTMLTextareaAttributes extends HTMLAttributes | undefined | null; From b44f949189368562133812c02e1256ef9d5dcf1b Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 13 Nov 2024 11:14:02 +0100 Subject: [PATCH 03/15] implement for defaultValue/defaultChecked on inputs --- .../2-analyze/visitors/RegularElement.js | 6 +-- .../client/visitors/RegularElement.js | 40 +++++++++++-------- .../client/visitors/shared/fragment.js | 3 +- .../server/visitors/shared/element.js | 3 +- .../client/dom/elements/bindings/input.js | 30 +++++++++----- packages/svelte/src/utils.js | 19 ++++++++- 6 files changed, 67 insertions(+), 34 deletions(-) diff --git a/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js index 60bd1dd0c5c7..d5964f9ae12a 100644 --- a/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/2-analyze/visitors/RegularElement.js @@ -1,6 +1,6 @@ /** @import { AST } from '#compiler' */ /** @import { Context } from '../types' */ -import { is_mathml, is_svg, is_void } from '../../../../utils.js'; +import { cannot_be_set_statically, is_mathml, is_svg, is_void } from '../../../../utils.js'; import { is_tag_valid_with_ancestor, is_tag_valid_with_parent @@ -77,9 +77,7 @@ export function RegularElement(node, context) { if ( node.attributes.some( - (attribute) => - attribute.type === 'Attribute' && - (attribute.name === 'autofocus' || attribute.name === 'muted') + (attribute) => attribute.type === 'Attribute' && cannot_be_set_statically(attribute.name) ) ) { mark_subtree_dynamic(context.path); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js index 4d3cebcee6fe..f48e33ef5245 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/RegularElement.js @@ -4,6 +4,7 @@ /** @import { ComponentClientTransformState, ComponentContext } from '../types' */ /** @import { Scope } from '../../../scope' */ import { + cannot_be_set_statically, is_boolean_attribute, is_dom_property, is_load_error_element, @@ -181,20 +182,28 @@ export function RegularElement(node, context) { } } - if ( - node.name === 'input' && - (has_spread || - bindings.has('value') || - bindings.has('checked') || - bindings.has('group') || - attributes.some( - (attribute) => - attribute.type === 'Attribute' && - (attribute.name === 'value' || attribute.name === 'checked') && - !is_text_attribute(attribute) - )) - ) { - context.state.init.push(b.stmt(b.call('$.remove_input_defaults', context.state.node))); + if (node.name === 'input') { + const has_value_attribute = attributes.some( + (attribute) => + attribute.type === 'Attribute' && + (attribute.name === 'value' || attribute.name === 'checked') && + !is_text_attribute(attribute) + ); + const has_default_value_attribute = attributes.some( + (attribute) => + attribute.type === 'Attribute' && + (attribute.name === 'defaultValue' || attribute.name === 'defaultChecked') + ); + if ( + !has_default_value_attribute && + (has_spread || + bindings.has('value') || + bindings.has('checked') || + bindings.has('group') || + (!bindings.has('group') && has_value_attribute)) + ) { + context.state.init.push(b.stmt(b.call('$.remove_input_defaults', context.state.node))); + } } if (node.name === 'textarea') { @@ -272,8 +281,7 @@ export function RegularElement(node, context) { if ( !is_custom_element && - attribute.name !== 'autofocus' && - attribute.name !== 'muted' && + !cannot_be_set_statically(attribute.name) && (attribute.value === true || is_text_attribute(attribute)) ) { const name = get_attribute_name(node, attribute); diff --git a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js index 3b009fb5b589..dbaa11ff4459 100644 --- a/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js +++ b/packages/svelte/src/compiler/phases/3-transform/client/visitors/shared/fragment.js @@ -1,6 +1,7 @@ /** @import { Expression } from 'estree' */ /** @import { AST, SvelteNode } from '#compiler' */ /** @import { ComponentContext } from '../../types' */ +import { cannot_be_set_statically } from '../../../../../../utils.js'; import { is_event_attribute, is_text_attribute } from '../../../../../utils/ast.js'; import * as b from '../../../../../utils/builders.js'; import { is_inlinable_expression } from '../../utils.js'; @@ -141,7 +142,7 @@ function is_static_element(node, state) { return false; } - if (attribute.name === 'autofocus' || attribute.name === 'muted') { + if (cannot_be_set_statically(attribute.name)) { return false; } diff --git a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js index c386c4f7c05e..4a1c7e287bd5 100644 --- a/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js +++ b/packages/svelte/src/compiler/phases/3-transform/server/visitors/shared/element.js @@ -82,7 +82,8 @@ export function build_element_attributes(node, context) { ) { events_to_capture.add(attribute.name); } - } else { + // the defaultValue/defaultChecked properties don't exist as attributes + } else if (attribute.name !== 'defaultValue' && attribute.name !== 'defaultChecked') { if (attribute.name === 'class') { class_index = attributes.length; } else if (attribute.name === 'style') { diff --git a/packages/svelte/src/internal/client/dom/elements/bindings/input.js b/packages/svelte/src/internal/client/dom/elements/bindings/input.js index e528d1699e48..b37cd704b63b 100644 --- a/packages/svelte/src/internal/client/dom/elements/bindings/input.js +++ b/packages/svelte/src/internal/client/dom/elements/bindings/input.js @@ -1,6 +1,6 @@ import { DEV } from 'esm-env'; import { render_effect, teardown } from '../../../reactivity/effects.js'; -import { listen_to_event_and_reset_event, without_reactive_context } from './shared.js'; +import { listen_to_event_and_reset_event } from './shared.js'; import * as e from '../../../errors.js'; import { is } from '../../../proxy.js'; import { queue_micro_task } from '../../task.js'; @@ -34,6 +34,17 @@ export function bind_value(input, get, set = get) { } }); + if ( + // If we are hydrating and the value has since changed, + // then use the update value from the input instead. + (hydrating && input.defaultValue !== input.value) || + // If defaultValue is set, then value == defaultValue + // TODO Svelte 6: remove input.value check and set to empty string? + (get() == null && input.value) + ) { + set(input.value); + } + render_effect(() => { if (DEV && input.type === 'checkbox') { // TODO should this happen in prod too? @@ -42,13 +53,6 @@ export function bind_value(input, get, set = get) { var value = get(); - // If we are hydrating and the value has since changed, then use the update value - // from the input instead. - if (hydrating && input.defaultValue !== input.value) { - set(input.value); - return; - } - if (is_numberlike_input(input) && value === to_number(input.value)) { // handles 0 vs 00 case (see https://github.com/sveltejs/svelte/issues/9959) return; @@ -180,8 +184,14 @@ export function bind_checked(input, get, set = get) { set(value); }); - if (get() == undefined) { - set(false); + if ( + // If we are hydrating and the value has since changed, + // then use the update value from the input instead. + (hydrating && input.defaultChecked !== input.checked) || + // If defaultChecked is set, then checked == defaultChecked + get() == null + ) { + set(input.checked); } render_effect(() => { diff --git a/packages/svelte/src/utils.js b/packages/svelte/src/utils.js index 60ec364e6fdd..987dea465e24 100644 --- a/packages/svelte/src/utils.js +++ b/packages/svelte/src/utils.js @@ -192,7 +192,9 @@ const ATTRIBUTE_ALIASES = { ismap: 'isMap', nomodule: 'noModule', playsinline: 'playsInline', - readonly: 'readOnly' + readonly: 'readOnly', + defaultvalue: 'defaultValue', + defaultchecked: 'defaultChecked' }; /** @@ -212,7 +214,9 @@ const DOM_PROPERTIES = [ 'readOnly', 'value', 'inert', - 'volume' + 'volume', + 'defaultValue', + 'defaultChecked' ]; /** @@ -222,6 +226,17 @@ export function is_dom_property(name) { return DOM_PROPERTIES.includes(name); } +const NON_STATIC_PROPERTIES = ['autofocus', 'muted', 'defaultValue', 'defaultChecked']; + +/** + * Returns `true` if the given attribute cannot be set through the template + * string, i.e. needs some kind of JavaScript handling to work. + * @param {string} name + */ +export function cannot_be_set_statically(name) { + return NON_STATIC_PROPERTIES.includes(name); +} + /** * Subset of delegated events which should be passive by default. * These two are already passive via browser defaults on window, document and body. From bc43171e85ed762fff8a6c8cdd84fdf7138b2903 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Wed, 13 Nov 2024 11:14:17 +0100 Subject: [PATCH 04/15] docs (draft) --- .../docs/03-template-syntax/11-bind.md | 54 ++++++++++++++++++- 1 file changed, 52 insertions(+), 2 deletions(-) diff --git a/documentation/docs/03-template-syntax/11-bind.md b/documentation/docs/03-template-syntax/11-bind.md index 7dd03a6b04d7..d1fcfa2ce047 100644 --- a/documentation/docs/03-template-syntax/11-bind.md +++ b/documentation/docs/03-template-syntax/11-bind.md @@ -53,6 +53,19 @@ In the case of a numeric input (`type="number"` or `type="range"`), the value wi If the input is empty or invalid (in the case of `type="number"`), the value is `undefined`. +You can give the input a default value by setting the `defaultValue` property. This way, when the input is part of a form and its `form.reset()` method is invoked, it will revert to that value instead of the empty string. Note that for the initial render the value of the binding takes precedence if it's not `null` or `undefined`. + +```svelte + + +
+ + +
+``` + ## `` Checkbox and radio inputs can be bound with `bind:checked`: @@ -64,16 +77,29 @@ Checkbox and radio inputs can be bound with `bind:checked`: ``` +You can give the input a default value by setting the `defaultChecked` property. This way, when the input is part of a form and its `form.reset()` method is invoked, it will revert to that value instead of `false`. Note that for the initial render the value of the binding takes precedence if it's not `null` or `undefined`. + +```svelte + + +
+ + +
+``` + ## `` Inputs that work together can use `bind:group`. ```svelte @@ -88,6 +114,20 @@ Inputs that work together can use `bind:group`. ``` +You can give the group a default value by setting the `checked` property on the elements that should be selected initially. This way, when the input is part of a form and its `form.reset()` method is invoked, it will revert to that value instead of the empty string (for radio groups) or the empty array (for checkbox groups). Note that for the initial render the value of the binding takes precedence if it's not `null` or `undefined`. + +```svelte + + +
+ + + +
+``` + > [!NOTE] `bind:group` only works if the inputs are in the same Svelte component. ## `` @@ -146,6 +186,16 @@ When the value of an `