Repository: sveltejs/svelte
File: packages/svelte/src/internal/client/dom/elements/attributes.js (JavaScript)

Code before:
/**
 * Spreads attributes onto a DOM element, taking into account the currently set attributes
 * @param {Element & ElementCSSInlineStyle} element
 * @param {Record<string, any> | undefined} prev
 * @param {Record<string, any>} next New attributes - this function mutates this object
 * @param {boolean} lowercase_attributes
 * @param {string} css_hash
 * @returns {Record<string, any>}
 */
export function set_attributes(element, prev, next, lowercase_attributes, css_hash) {
	var has_hash = css_hash.length !== 0;
	var current = prev || {};
	var is_option_element = element.tagName === 'OPTION';

	for (var key in prev) {
		if (!(key in next)) {
			if (is_option_element && key === 'value') {
				// Because of the "set falsy option values to the empty string" logic below, which can't
				// differentiate between a missing value and an explicitly set value of null or undefined,
				// we need to remove the attribute here and delete the key from the object.
				element.removeAttribute(key);
				delete current[key];
				delete next[key];
			} else {
				next[key] = null;
			}
		}
	}

	if (has_hash && !next.class) {
		next.class = '';
	}

	var setters = setters_cache.get(element.nodeName);
	if (!setters) setters_cache.set(element.nodeName, (setters = get_setters(element)));

	// @ts-expect-error
	var attributes = /** @type {Record<string, unknown>} **/ (element.__attributes ??= {});
	/** @type {Array<[string, any, () => void]>} */
	var events = [];

	// since key is captured we use const
	for (const key in next) {
		// let instead of var because referenced in a closure
		let value = next[key];

		// Up here because we want to do this for the initial value, too, even if it's undefined,
		// and this wouldn't be reached in case of undefined because of the equality check below
		if (is_option_element && key === 'value' && value == null) {
			// The <option> element is a special case because removing the value attribute means
			// the value is set to the text content of the option element, and setting the value

Review comment (bug):
It's a shame we need this additional code to be 100% correct. Since it's impossible to remove the `value` attribute in the case you set it through a regular (non-spreaded) attribute I'd be totally ok with removing this extra code and adding a comment to the other logic below about a super-edge-case-bug present and the deliberate tradeoff for less code 

Code after:
/**
 * Spreads attributes onto a DOM element, taking into account the currently set attributes
 * @param {Element & ElementCSSInlineStyle} element
 * @param {Record<string, any> | undefined} prev
 * @param {Record<string, any>} next New attributes - this function mutates this object
 * @param {boolean} lowercase_attributes
 * @param {string} css_hash
 * @returns {Record<string, any>}
 */
export function set_attributes(element, prev, next, lowercase_attributes, css_hash) {
	var has_hash = css_hash.length !== 0;
	var current = prev || {};
	var is_option_element = element.tagName === 'OPTION';

	for (var key in prev) {
		if (!(key in next)) {
			next[key] = null;
		}
	}

	if (has_hash && !next.class) {
		next.class = '';
	}

	var setters = setters_cache.get(element.nodeName);
	if (!setters) setters_cache.set(element.nodeName, (setters = get_setters(element)));

	// @ts-expect-error
	var attributes = /** @type {Record<string, unknown>} **/ (element.__attributes ??= {});
	/** @type {Array<[string, any, () => void]>} */
	var events = [];

	// since key is captured we use const
	for (const key in next) {
		// let instead of var because referenced in a closure
		let value = next[key];

		// Up here because we want to do this for the initial value, too, even if it's undefined,
		// and this wouldn't be reached in case of undefined because of the equality check below
		if (is_option_element && key === 'value' && value == null) {
			// The <option> element is a special case because removing the value attribute means
			// the value is set to the text content of the option element, and setting the value
			// to null or undefined means the value is set to the string "null" or "undefined".
			// To align with how we handle this case in non-spread-scenarios, this logic is needed.
			// There's a super-edge-case bug here that is left in in favor of smaller code size:
			// Because of the "set missing props to null" logic above, we can't differentiate
			// between a missing value and an explicitly set value of null or undefined. That means
			// that once set, the value attribute of an <option> element can't be removed. This is
			// a very rare edge case, and removing the attribute altogether isn't possible either
			// for the <option value={undefined}> case, so we're not losing any functionality here.
			// @ts-ignore