Unexpected DOM Updates When Wrapping Checkbox in a Vue Component

[Mini-ghost]

Vue version

v3.4.38

Link to minimal reproduction

https://play.vuejs.org/#eNp9VE1v2zAM/SuCL3EBz8G6nbIkxVYE2IZlLbpiO8w7ODGTqZMlQR9phiD/vZQU2WoS5GBAIh+pR/KZu+yjlOXGQjbKxnqpqDREg7FyWnHaSqEM2REFK7InKyVaMkDooHPd/oXlv4XYHnzlMBpcRoQtBdeGLJ0Rmrdk4jLlq5ppuDpyXh85x8NABmngxUArWW0Ab4SMKZfWkM2bVjTAJlUW81cZMf8lRAvSQMtoUzPrbJY3sKIcGjQOQ6KO/kmu6wNoPEyezorMaGS9ouvySQuOLdu5PBglWkkZqDtpKFZVZSPiPc5XMyaev3qbURaKaPcvnbE/aaQ9wsO9Ag1qA1XW+Uyt1mCCe/bjO2zx3DmxBMsQfcH5AFow6zgG2CdsCtJOcJ7tFz9dytePerY1wHUsyhF1yL3HVxlO+fZC6T3dd+V7H1fxPXYxVcmx7gir+RoHYTBVqkHB58JyA80rJX5IEG4IFgHFiV4RFeUmlZAatRa0cO9u+e+Bn/5PJ5RBgSH+8KeTKLTUdCEzvLgQKxtUxSiJdAExhIllzQ6S9k+WPbDL25t+KWrqBQOMiFXkvovYvvwqNlRhfxQPJST5yM1NeK/0xPv5YD9zb+oyuEryM9QLEnABlSRDQn1SHF5Sov8LH7DTvsrx58f5txmDFriZ5gjr5pVjAZNpYBAixQJjODyTuTW1U83dwgtdpdAAFgxKJtb5oD1AB55j4EEwUylCbB7pBN5FTFIbo+gCG6qT38yFu+/Skmnoxh+6fYNFoizjM7gf+qVxOsh08YQGntlN4aFheOn1ptm/ACJ26e0=

Steps to reproduce

1. Open the Devtools Elements panel.
2. Click on the first Checkbox, and notice that the corresponding <input> does not show any updates.
3. Click on the second Checkbox, and observe that the corresponding <input> flickers (updates).

2024-08-19.12.28.35.mov

What is expected?

In this straightforward reproduction, the corresponding <input> DOM should not be updated when clicking on the Checkbox, as there are no updates besides modelValue.

What is actually happening?

The following code does not update the DOM, which is expected:

<input v-model="checked1" type="checkbox" :value="undefined" />

However, when this code is wrapped into a component, the DOM flickers (updates) every time the Checkbox is clicked.

Checkbox.vue

<script setup lang="ts">
import { computed } from 'vue';

const props = defineProps(['modelValue', 'value']);
const emit = defineEmits(['update:modelValue']);

const modelValueWritable = computed({
  get() {
    return props.modelValue;
  },
  set(value) {
    emit('update:modelValue', value);
  },
});
</script>

<template>
  <div>
    <input v-model="modelValueWritable" :value="value" type="checkbox">
  </div>
</template>

<Checkbox v-model="checked2" />

System Info

System:
    OS: macOS 14.5
    CPU: (8) arm64 Apple M1
    Memory: 80.08 MB / 16.00 GB
    Shell: 5.9 - /bin/zsh
  Binaries:
    Node: 20.9.0 - ~/.nvm/versions/node/v20.9.0/bin/node
    Yarn: 1.22.19 - ~/.nvm/versions/node/v20.9.0/bin/yarn
    npm: 10.5.0 - ~/.nvm/versions/node/v20.9.0/bin/npm
    pnpm: 9.4.0 - ~/.nvm/versions/node/v20.9.0/bin/pnpm
  Browsers:
    Safari: 17.5
    Safari Technology Preview: 17.4
  npmPackages:
    vue: ^3.4.38 => 3.4.38

Any additional comments?

If the default value for value is changed to an empty string, this issue does not occur.

Link to reproduction 2

[Procrustes5]

I found something weird with vModelCheckbox. It's doing extra DOM updates when it's in a component, but vModelText doesn't do this. I think it's because of how beforeUpdate works differently

reproduce

packages/runtime-dom/src/directives/vModel.ts
https://github.com/vuejs/core/blob/c183405f9f58a6619ebc2ce9c653ce3726138a53/packages/runtime-dom/src/directives/vModel.ts
Here's vModelText:

beforeUpdate(el, { value, oldValue, modifiers: { lazy, trim, number } }, vnode) {
  // ...
  if (elValue === newValue) {
    return
  }
  // ...
  el.value = newValue
},

And here's vModelCheckbox:

beforeUpdate(el, binding, vnode) {
  el[assignKey] = getModelAssigner(vnode)
  setChecked(el, binding, vnode)
},

function setChecked(el, { value, oldValue }, vnode) {
  el._modelValue = value
  if (Array.isArray(value)) {
    el.checked = value.indexOf(vnode.props.value) > -1
  } else if (value instanceof Set) {
    el.checked = value.has(vnode.props.value)
  } else if (value !== oldValue) {
    el.checked = value == getCheckboxValue(el, true)
  }
}

See how vModelText checks if the value changed before updating? But vModelCheckbox just calls setChecked every time.

I think there are a few problems:

1. It only checks value !== oldValue, but this doesn't work well for arrays or Sets.
2. For arrays and Sets, it does loops or searches every time, even if nothing changed.
3. It sets checked without checking if it's actually different.

Maybe we could make setChecked smarter? Like, only update checked if it's really different. That way, it wouldn't do extra DOM stuff when it doesn't need to.

What do you think? Should I try to make a fix for this?

[edison1105]

@Procrustes5
PR welcome

[Mini-ghost]

Upon further investigation, I discovered that this might be one of the causes of the update issue.

In the patchDOMProp function, it checks whether the new and old value are the same. When the <input type="checkbox"> has value set to undefined, the retrieved oldValue will be 'on', while the newValue, after evaluation, will be an empty string. This mismatch leads to an update of the value, which in turn triggers a DOM update.

core/packages/runtime-dom/src/modules/props.ts

Lines 25 to 46 in c183405

	if (
	key === 'value' &&
	tag !== 'PROGRESS' &&
	// custom elements may use _value internally
	!tag.includes('-')
	) {
	// #4956: <option> value will fallback to its text content so we need to
	// compare against its attribute value instead.
	const oldValue =
	tag === 'OPTION' ? el.getAttribute('value') || '' : el.value
	const newValue = value == null ? '' : String(value)
	if (oldValue !== newValue || !('_value' in el)) {
	el.value = newValue
	}
	if (value == null) {
	el.removeAttribute(key)
	}
	// store value as _value as well since
	// non-string values will be stringified.
	el._value = value
	return
	}

I attempted to adjust the condition so that when value is either null or undefined and el.type is checkbox, the value passed in would be 'on' instead of an empty string.

const oldValue = tag === 'OPTION' ? el.getAttribute('value') || '' : el.value 
const newValue = value == null 
 ? el.type === 'checkbox'
   ? 'on'
   : ''
 : String(value) 

This adjustment appears to resolve the DOM update issue.

May I open a PR with this approach? If there are any aspects I have overlooked, I would be very grateful for your feedback.

[Mini-ghost]

@yyx990803

I apologize for the inconvenience, but the issue still persists after the merge of PR #11656. It appears that it might only be resolved after the merge of PR #11657. Could we possibly reopen this issue on GitHub in the meantime? Thank you very much for your attention.