From 37bd61d3f168fbf4fc1dd691624f9ac3b0e4c8ea Mon Sep 17 00:00:00 2001 From: paoloricciuti Date: Mon, 30 Mar 2026 16:09:12 +0200 Subject: [PATCH] fix: attributes dom access --- .../client/visitors/RegularElement.js | 20 +++- .../client/dom/elements/attributes.js | 103 +++++++++++------- .../src/internal/client/dom/operations.js | 74 +++++++++++++ .../value-checked-attributes/_config.js | 65 +++++++++++ .../value-checked-attributes/main.svelte | 20 ++++ 5 files changed, 237 insertions(+), 45 deletions(-) create mode 100644 packages/svelte/tests/custom-renderers/samples/value-checked-attributes/_config.js create mode 100644 packages/svelte/tests/custom-renderers/samples/value-checked-attributes/main.svelte 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 0746c5f867..f5f574f328 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 @@ -270,7 +270,14 @@ export function RegularElement(node, context) { (value, metadata) => context.state.memoizer.add(value, metadata) ); - const update = build_element_attribute_update(node, node_id, name, value, attributes); + const update = build_element_attribute_update( + node, + node_id, + name, + value, + attributes, + !!context.state.options.customRenderer + ); (has_state ? context.state.update : context.state.init).push(b.stmt(update)); } @@ -597,7 +604,14 @@ export function build_style_directives_object( * @param {Expression} value * @param {Array} attributes */ -function build_element_attribute_update(element, node_id, name, value, attributes) { +function build_element_attribute_update( + element, + node_id, + name, + value, + attributes, + custom_renderer = false +) { if (name === 'muted') { // Special case for Firefox who needs it set as a property in order to work return b.assignment('=', b.member(node_id, b.id('muted')), value); @@ -644,7 +658,7 @@ function build_element_attribute_update(element, node_id, name, value, attribute } return b.call( - name.startsWith('xlink') ? '$.set_xlink_attribute' : '$.set_attribute', + name.startsWith('xlink') && !custom_renderer ? '$.set_xlink_attribute' : '$.set_attribute', node_id, b.literal(name), value, diff --git a/packages/svelte/src/internal/client/dom/elements/attributes.js b/packages/svelte/src/internal/client/dom/elements/attributes.js index a15fc48596..1bf298c38f 100644 --- a/packages/svelte/src/internal/client/dom/elements/attributes.js +++ b/packages/svelte/src/internal/client/dom/elements/attributes.js @@ -23,6 +23,19 @@ import { ATTACHMENT_KEY, NAMESPACE_HTML, UNINITIALIZED } from '../../../../const import { branch, destroy_effect, effect, managed } from '../../reactivity/effects.js'; import { init_select, select_option } from './bindings/select.js'; import { flatten } from '../../reactivity/async.js'; +import { + has_attribute, + get_attribute, + remove_attribute, + set_attribute as set_attribute_op, + remove_event_listener, + node_name, + set_element_value, + set_element_checked, + set_element_default_value, + set_element_default_checked +} from '../operations.js'; +import { renderer } from '../../custom-renderer/state.js'; export const CLASS = Symbol('class'); export const STYLE = Symbol('style'); @@ -45,6 +58,8 @@ const PROGRESS_TAG = IS_XHTML ? 'progress' : 'PROGRESS'; export function remove_input_defaults(input) { if (!hydrating) return; + // we should be safe here from custom renderers as this only run during hydration + var already_removed = false; // We try and remove the default attributes later, rather than sync during hydration. @@ -56,13 +71,13 @@ export function remove_input_defaults(input) { already_removed = true; // Remove the attributes but preserve the values - if (input.hasAttribute('value')) { + if (has_attribute(input, 'value')) { var value = input.value; set_attribute(input, 'value', null); input.value = value; } - if (input.hasAttribute('checked')) { + if (has_attribute(input, 'checked')) { var checked = input.checked; set_attribute(input, 'checked', null); input.checked = checked; @@ -89,13 +104,12 @@ export function set_value(element, value) { value ?? undefined) || // @ts-expect-error // `progress` elements always need their value set when it's `0` - (element.value === value && (value !== 0 || element.nodeName !== PROGRESS_TAG)) + (element.value === value && (value !== 0 || node_name(element) !== PROGRESS_TAG)) ) { return; } - // @ts-expect-error - element.value = value ?? ''; + set_element_value(element, value); } /** @@ -114,8 +128,7 @@ export function set_checked(element, checked) { return; } - // @ts-expect-error - element.checked = checked; + set_element_checked(element, checked); } /** @@ -129,11 +142,11 @@ export function set_selected(element, selected) { if (selected) { // The selected option could've changed via user selection, and // setting the value without this check would set it back. - if (!element.hasAttribute('selected')) { - element.setAttribute('selected', ''); + if (!has_attribute(element, 'selected')) { + set_attribute_op(element, 'selected', ''); } } else { - element.removeAttribute('selected'); + remove_attribute(element, 'selected'); } } @@ -143,9 +156,7 @@ export function set_selected(element, selected) { * @param {boolean} checked */ export function set_default_checked(element, checked) { - const existing_value = element.checked; - element.defaultChecked = checked; - element.checked = existing_value; + set_element_default_checked(element, checked); } /** @@ -154,9 +165,7 @@ export function set_default_checked(element, checked) { * @param {string} value */ export function set_default_value(element, value) { - const existing_value = element.value; - element.defaultValue = value; - element.value = existing_value; + set_element_default_value(element, value); } /** @@ -169,12 +178,12 @@ export function set_attribute(element, attribute, value, skip_warning) { var attributes = get_attributes(element); if (hydrating) { - attributes[attribute] = element.getAttribute(attribute); + attributes[attribute] = get_attribute(element, attribute); if ( attribute === 'src' || attribute === 'srcset' || - (attribute === 'href' && element.nodeName === LINK_TAG) + (attribute === 'href' && node_name(element) === LINK_TAG) ) { if (!skip_warning) { check_src_in_dev_hydration(element, attribute, value ?? ''); @@ -196,12 +205,12 @@ export function set_attribute(element, attribute, value, skip_warning) { } if (value == null) { - element.removeAttribute(attribute); + remove_attribute(element, attribute); } else if (typeof value !== 'string' && get_setters(element).includes(attribute)) { // @ts-ignore element[attribute] = value; } else { - element.setAttribute(attribute, value); + set_attribute_op(element, attribute, value); } } @@ -211,6 +220,7 @@ export function set_attribute(element, attribute, value, skip_warning) { * @param {string} value */ export function set_xlink_attribute(dom, attribute, value) { + // custom renderer safe since we never emit this dom.setAttributeNS('http://www.w3.org/1999/xlink', attribute, value); } @@ -220,6 +230,7 @@ export function set_xlink_attribute(dom, attribute, value) { * @param {any} value */ export function set_custom_element_data(node, prop, value) { + // custom renderer safe since we never emit this // We need to ensure that setting custom element props, which can // invoke lifecycle methods on other custom elements, does not also // associate those lifecycle methods with the current active reaction @@ -244,10 +255,10 @@ export function set_custom_element_data(node, prop, value) { // Don't compute setters for custom elements while they aren't registered yet, // because during their upgrade/instantiation they might add more setters. // Instead, fall back to a simple "an object, then set as property" heuristic. - (setters_cache.has(node.getAttribute('is') || node.nodeName) || + (setters_cache.has(get_attribute(node, 'is') || (node_name(node) ?? '')) || // customElements may not be available in browser extension contexts !customElements || - customElements.get(node.getAttribute('is') || node.nodeName.toLowerCase()) + customElements.get(get_attribute(node, 'is') || (node_name(node) ?? '').toLowerCase()) ? get_setters(node).includes(prop) : value && typeof value === 'object') ) { @@ -286,7 +297,7 @@ function set_attributes( should_remove_defaults = false, skip_warning = false ) { - if (hydrating && should_remove_defaults && element.nodeName === INPUT_TAG) { + if (hydrating && should_remove_defaults && node_name(element) === INPUT_TAG) { var input = /** @type {HTMLInputElement} */ (element); var attribute = input.type === 'checkbox' ? 'defaultChecked' : 'defaultValue'; @@ -308,7 +319,7 @@ function set_attributes( } var current = prev || {}; - var is_option_element = element.nodeName === OPTION_TAG; + var is_option_element = node_name(element) === OPTION_TAG; for (var key in prev) { if (!(key in next)) { @@ -370,7 +381,7 @@ function set_attributes( var prev_value = current[key]; // Skip if value is unchanged, unless it's `undefined` and the element still has the attribute - if (value === prev_value && !(value === undefined && element.hasAttribute(key))) { + if (value === prev_value && !(value === undefined && has_attribute(element, key))) { continue; } @@ -398,7 +409,7 @@ function set_attributes( // https://github.com/sveltejs/svelte/issues/11903 if (value != null) continue; - element.removeEventListener(event_name, current[event_handle_key], opts); + remove_event_listener(element, event_name, current[event_handle_key], opts); current[event_handle_key] = null; } @@ -424,7 +435,8 @@ function set_attributes( } else if (!is_custom_element && (key === '__value' || (key === 'value' && value != null))) { // @ts-ignore We're not running this for custom elements because __value is actually // how Lit stores the current value on the element, and messing with that would break things. - element.value = element.__value = value; + element.__value = value; + set_element_value(element, value); } else if (key === 'selected' && is_option_element) { set_selected(/** @type {HTMLOptionElement} */ (element), value); } else { @@ -437,25 +449,28 @@ function set_attributes( if (value == null && !is_custom_element && !is_default) { attributes[key] = null; - - if (name === 'value' || name === 'checked') { + if ((name === 'value' || name === 'checked') && renderer == null) { // removing value/checked also removes defaultValue/defaultChecked — preserve let input = /** @type {HTMLInputElement} */ (element); const use_default = prev === undefined; if (name === 'value') { let previous = input.defaultValue; - input.removeAttribute(name); - input.defaultValue = previous; + remove_attribute(input, name); + set_element_default_value(input, previous); // @ts-ignore - input.value = input.__value = use_default ? previous : null; + set_element_value(input, (input.__value = use_default ? previous : null)); } else { let previous = input.defaultChecked; - input.removeAttribute(name); - input.defaultChecked = previous; - input.checked = use_default ? previous : false; + remove_attribute(input, name); + set_element_default_checked(input, previous); + set_element_checked(input, use_default ? previous : false); } } else { - element.removeAttribute(key); + remove_attribute(element, key); + if (name === 'value') { + // @ts-ignore + element.__value = null; + } } } else if ( is_default || @@ -505,7 +520,7 @@ export function attribute_effect( /** @type {Record} */ var effects = {}; - var is_select = element.nodeName === SELECT_TAG; + var is_select = node_name(element) === SELECT_TAG; var inited = false; managed(() => { @@ -563,7 +578,7 @@ function get_attributes(element) { return /** @type {Record} **/ ( // @ts-expect-error element.__attributes ??= { - [IS_CUSTOM_ELEMENT]: element.nodeName.includes('-'), + [IS_CUSTOM_ELEMENT]: (node_name(element) ?? '').includes('-'), [IS_HTML]: element.namespaceURI === NAMESPACE_HTML } ); @@ -574,7 +589,9 @@ var setters_cache = new Map(); /** @param {Element} element */ function get_setters(element) { - var cache_key = element.getAttribute('is') || element.nodeName; + // if we have a custom renderer we just skip the check all together + if (renderer) return []; + var cache_key = get_attribute(element, 'is') || (node_name(element) ?? ''); var setters = setters_cache.get(cache_key); if (setters) return setters; setters_cache.set(cache_key, (setters = [])); @@ -606,12 +623,13 @@ function get_setters(element) { * @param {string} value */ function check_src_in_dev_hydration(element, attribute, value) { - if (!DEV) return; + if (!DEV || renderer != null) return; if (attribute === 'srcset' && srcset_url_equal(element, value)) return; - if (src_url_equal(element.getAttribute(attribute) ?? '', value)) return; + if (src_url_equal(get_attribute(element, attribute) ?? '', value)) return; w.hydration_attribute_changed( attribute, + // TODO RENDERER figure out what to do here element.outerHTML.replace(element.innerHTML, element.innerHTML && '...'), String(value) ); @@ -623,7 +641,7 @@ function check_src_in_dev_hydration(element, attribute, value) { * @returns {boolean} */ function src_url_equal(element_src, url) { - if (element_src === url) return true; + if (element_src === url || renderer != null) return true; return new URL(element_src, document.baseURI).href === new URL(url, document.baseURI).href; } @@ -638,6 +656,7 @@ function split_srcset(srcset) { * @returns {boolean} */ function srcset_url_equal(element, srcset) { + if (renderer != null) return true; var element_urls = split_srcset(element.srcset); var urls = split_srcset(srcset); diff --git a/packages/svelte/src/internal/client/dom/operations.js b/packages/svelte/src/internal/client/dom/operations.js index 7b592b84f9..065b0f51ab 100644 --- a/packages/svelte/src/internal/client/dom/operations.js +++ b/packages/svelte/src/internal/client/dom/operations.js @@ -540,6 +540,80 @@ export function get_node_value(node) { return node.nodeValue; } +/** + * Sets the `value` property on an element. For custom renderers, uses `setAttribute`. + * @param {Element} element + * @param {any} value + */ +export function set_element_value(element, value) { + if (renderer) { + renderer.setAttribute(element, 'value', value ?? ''); + return; + } + // @ts-expect-error + element.value = value ?? ''; +} + +/** + * Sets the `checked` property on an element. For custom renderers, uses `setAttribute`. + * @param {Element} element + * @param {boolean} checked + */ +export function set_element_checked(element, checked) { + if (renderer) { + if (checked) { + renderer.setAttribute(element, 'checked', ''); + } else { + renderer.removeAttribute(element, 'checked'); + } + return; + } + // @ts-expect-error + element.checked = checked; +} + +/** + * Sets the `defaultValue` property on an element without affecting the current `value`. + * For custom renderers, uses `setAttribute` on `defaultvalue`. + * @param {Element} element + * @param {string} value + */ +export function set_element_default_value(element, value) { + if (renderer) { + renderer.setAttribute(element, 'defaultvalue', value); + return; + } + // @ts-expect-error + const existing_value = element.value; + // @ts-expect-error + element.defaultValue = value; + // @ts-expect-error + element.value = existing_value; +} + +/** + * Sets the `defaultChecked` property on an element without affecting the current `checked` state. + * For custom renderers, uses `setAttribute` on `defaultchecked`. + * @param {Element} element + * @param {boolean} checked + */ +export function set_element_default_checked(element, checked) { + if (renderer) { + if (checked) { + renderer.setAttribute(element, 'defaultchecked', ''); + } else { + renderer.removeAttribute(element, 'defaultchecked'); + } + return; + } + // @ts-expect-error + const existing_value = element.checked; + // @ts-expect-error + element.defaultChecked = checked; + // @ts-expect-error + element.checked = existing_value; +} + /** * @param {Element} element * @param {string} name diff --git a/packages/svelte/tests/custom-renderers/samples/value-checked-attributes/_config.js b/packages/svelte/tests/custom-renderers/samples/value-checked-attributes/_config.js new file mode 100644 index 0000000000..b656ac37df --- /dev/null +++ b/packages/svelte/tests/custom-renderers/samples/value-checked-attributes/_config.js @@ -0,0 +1,65 @@ +import { flushSync } from 'svelte'; +import { test } from '../../test'; + +export default test({ + test({ assert, target, serialize, dispatch_event }) { + // Initial state + const html = serialize(target); + + // Find all inputs and the button + const inputs = target.children.filter( + (/** @type {any} */ n) => n.type === 'element' && n.name === 'input' + ); + const button = target.children.find( + (/** @type {any} */ n) => n.type === 'element' && n.name === 'button' + ); + + assert.equal(inputs.length, 5); + assert.ok(button); + + // Input 1: value="hello" + const input_value = inputs[0]; + assert.equal(input_value.attributes['value'], 'hello'); + + // Input 2: type="checkbox" checked="" + const input_checked = inputs[1]; + assert.equal(input_checked.attributes['type'], 'checkbox'); + assert.equal(input_checked.attributes['checked'], ''); + + // Input 3: value="fixed" defaultvalue="default_val" + const input_default_value = inputs[2]; + assert.equal(input_default_value.attributes['value'], 'fixed'); + assert.equal(input_default_value.attributes['defaultvalue'], 'default_val'); + + // Input 4: type="checkbox" checked="" defaultchecked="" + const input_default_checked = inputs[3]; + assert.equal(input_default_checked.attributes['type'], 'checkbox'); + assert.equal(input_default_checked.attributes['checked'], ''); + assert.equal(input_default_checked.attributes['defaultchecked'], ''); + + // Input 5: spread attributes + const input_spread = inputs[4]; + assert.equal(input_spread.attributes['value'], 'hello'); + + // Click the button to update all values + dispatch_event(button, 'click'); + flushSync(); + + // After update: + // Input 1: value="world" + assert.equal(input_value.attributes['value'], 'world'); + + // Input 2: checked should be removed + assert.equal(input_checked.attributes['checked'], undefined); + + // Input 3: defaultvalue="new_default", value still "fixed" + assert.equal(input_default_value.attributes['value'], 'fixed'); + assert.equal(input_default_value.attributes['defaultvalue'], 'new_default'); + + // Input 4: defaultchecked should be removed + assert.equal(input_default_checked.attributes['defaultchecked'], undefined); + + // Input 5: spread attributes should update value to "world" + assert.equal(input_spread.attributes['value'], 'world'); + } +}); diff --git a/packages/svelte/tests/custom-renderers/samples/value-checked-attributes/main.svelte b/packages/svelte/tests/custom-renderers/samples/value-checked-attributes/main.svelte new file mode 100644 index 0000000000..097bdc0a39 --- /dev/null +++ b/packages/svelte/tests/custom-renderers/samples/value-checked-attributes/main.svelte @@ -0,0 +1,20 @@ + + + + + + + + +