From 95b87ff59614562803f236adfb916a77a0b70070 Mon Sep 17 00:00:00 2001 From: Simon Holthausen Date: Tue, 10 Dec 2024 17:03:31 +0100 Subject: [PATCH] chore: more default value spread work This is a WIP, which I'm not sure if it goes anywhere. It started out to deeper understand the fix in #14640, and to fix some more theoretical loopholes, but this turns out to not fix the issue yet, and I'm not sure if it even will, so I'm punting on it for now but putting it up for others to see. --- .../server/visitors/shared/element.js | 34 ++++++++--- .../client/dom/elements/attributes.js | 56 +++++++++++++------ packages/svelte/src/internal/server/index.js | 16 ++++++ 3 files changed, 82 insertions(+), 24 deletions(-) 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 d626bb08db30..2838a30f6d6e 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 @@ -46,7 +46,7 @@ export function build_element_attributes(node, context) { /** @type {Expression | null} */ let content = null; - let has_spread = false; + let needs_spread = false; // Use the index to keep the attributes order which is important for spreading let class_index = -1; let style_index = -1; @@ -82,8 +82,28 @@ export function build_element_attributes(node, context) { ) { events_to_capture.add(attribute.name); } - // the defaultValue/defaultChecked properties don't exist as attributes - } else if (attribute.name !== 'defaultValue' && attribute.name !== 'defaultChecked') { + } else if ( + (node.name === 'input' || node.name === 'textarea') && + (attribute.name === 'defaultValue' || attribute.name === 'defaultChecked') + ) { + // If there's a defaultValue but not value attribute, we turn it into a value attribute (same for checked). + // If we can't, then we need to use a spread in order to be able to detect at runtime whether or not + // the value/checked value is nullish, in which case defaultValue/defaultChecked should be used. + const replacement_name = attribute.name === 'defaultValue' ? 'value' : 'checked'; + if ( + !node.attributes.some( + (attr) => + attr.type === 'SpreadAttribute' || + ((attr.type === 'BindDirective' || attr.type === 'Attribute') && + attr.name === replacement_name) + ) + ) { + attributes.push({ ...attribute, name: replacement_name }); + } else { + needs_spread = true; + attributes.push(attribute); + } + } else { if (attribute.name === 'class') { class_index = attributes.length; } else if (attribute.name === 'style') { @@ -173,7 +193,7 @@ export function build_element_attributes(node, context) { } } else if (attribute.type === 'SpreadAttribute') { attributes.push(attribute); - has_spread = true; + needs_spread = true; if (is_load_error_element(node.name)) { events_to_capture.add('onload'); events_to_capture.add('onerror'); @@ -194,7 +214,7 @@ export function build_element_attributes(node, context) { } } - if (class_directives.length > 0 && !has_spread) { + if (class_directives.length > 0 && !needs_spread) { const class_attribute = build_class_directives( class_directives, /** @type {AST.Attribute | null} */ (attributes[class_index] ?? null) @@ -204,7 +224,7 @@ export function build_element_attributes(node, context) { } } - if (style_directives.length > 0 && !has_spread) { + if (style_directives.length > 0 && !needs_spread) { build_style_directives( style_directives, /** @type {AST.Attribute | null} */ (attributes[style_index] ?? null), @@ -215,7 +235,7 @@ export function build_element_attributes(node, context) { } } - if (has_spread) { + if (needs_spread) { build_element_spread_attributes(node, attributes, style_directives, class_directives, context); } else { for (const attribute of /** @type {AST.Attribute[]} */ (attributes)) { diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index ec57917e7696..9a219ae6d03c 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -344,36 +344,58 @@ export function set_attributes( element.style.cssText = value + ''; } else if (key === 'autofocus') { autofocus(/** @type {HTMLElement} */ (element), Boolean(value)); - } else if (key === '__value' || (key === 'value' && value != null)) { - // @ts-ignore - element.value = element[key] = element.__value = value; - } else if (key === 'selected' && is_option_element) { + } else if ( + key === '__value' || + (key === 'value' && + // For element we don't want to fall through to removeAttribute below, + // because that attribute corresponds to the defaultValue, not the value property + (value != null || !is_custom_element)) + ) { + set_value(element, value); + } else if (key === 'checked' && (value != null || !is_custom_element)) { + set_checked(element, value); + } else if (is_option_element && key === 'selected') { set_selected(/** @type {HTMLOptionElement} */ (element), value); } else { var name = key; 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) { + if (value == null && !is_custom_element && key !== '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); - if (key === 'value') { - /**@type {HTMLInputElement}*/ (element).defaultValue = default_value_reset; - } else if (key === 'checked') { - /**@type {HTMLInputElement}*/ (element).defaultChecked = default_checked_reset; - } } else if ( - is_default_value_or_checked || - (setters.includes(name) && (is_custom_element || typeof value !== 'string')) + // is_default_value_or_checked || + setters.includes(name) && + (is_custom_element || + typeof value !== 'string' || + name === 'defaultValue' || + name === 'defaultChecked') ) { + // if we adjust 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; + let default_checked_reset; + if (hydrating) { + default_value_reset = /**@type {HTMLInputElement}*/ (element).value; + default_checked_reset = /**@type {HTMLInputElement}*/ (element).checked; + } + // @ts-ignore element[name] = value; + + if (hydrating) { + if (key === 'defaultValue') { + /**@type {HTMLInputElement}*/ (element).value = /** @type {string} */ ( + default_value_reset + ); + } else if (key === 'defaultChecked') { + /**@type {HTMLInputElement}*/ (element).checked = /** @type {boolean} */ ( + default_checked_reset + ); + } + } } else if (typeof value !== 'function') { if (hydrating && (name === 'src' || name === 'href' || name === 'srcset')) { if (!skip_warning) check_src_in_dev_hydration(element, name, value ?? ''); diff --git a/packages/svelte/src/internal/server/index.js b/packages/svelte/src/internal/server/index.js index b944c602b884..08b8472bcd6b 100644 --- a/packages/svelte/src/internal/server/index.js +++ b/packages/svelte/src/internal/server/index.js @@ -224,6 +224,22 @@ export function spread_attributes(attrs, classes, styles, flags = 0) { var value = attrs[name]; + if (is_html) { + if (name === 'defaultvalue') { + if (attrs.value == null) { + name = 'value'; + } else { + continue; + } + } else if (name === 'defaultchecked') { + if (attrs.checked == null) { + name = 'checked'; + } else { + continue; + } + } + } + if (lowercase) { name = name.toLowerCase(); }