Skip to content

fix(runtime-dom): re-sync select when v-model is overridden in change handler (fix #10505)#14960

Open
zzlw wants to merge 1 commit into
vuejs:mainfrom
zzlw:fix/select-vmodel-override-10505
Open

fix(runtime-dom): re-sync select when v-model is overridden in change handler (fix #10505)#14960
zzlw wants to merge 1 commit into
vuejs:mainfrom
zzlw:fix/select-vmodel-override-10505

Conversation

@zzlw

@zzlw zzlw commented Jun 14, 2026

Copy link
Copy Markdown

Summary

Fixes #10505.

When a <select v-model> has a change handler (or watcher) that reassigns
the model to a value different from the option the user just selected
(e.g. resetting it to null), the select's DOM state is left out of sync
with the model.

Reproduction

From the issue: https://play.vuejs.org/#eNp9Uk1P4zAQ/SuzvtBKkO5q91SllXYRh+VAEXACc4iSSWtwbMsfoVLIf2ds00IRkEMSz3sz8+Z5BvbXmKIPyOas9NgZWXlccgVQOpRYe+hPOt2gXHCWA0IrziJh9/QnWs3rTaXWSCT6NhJX6jQFOEu1qJo2MRP6SoZI+0nILVpdzjLwBe0X0VYKD1jlLAvJB0P9W22jPBAKMoQN5Q0DOBjHcmaIWc7eDUdHV1thPEgSSanecUapPkSq6Iy2Hgaw2MIIrdUdHJFDRzGx1sr51y5R6CKySuetUGt4BhWkXE7ie3rIxeaAene/nNzdE4erNqhc6tA5mEzn0GvRwBDnFC1M9l2LZA/8WCxSw2mmwL5VxgsT3OZjErUEGOPrY7VciysCyeFkz5IdkzU0RivWxYPTipYkteKs1p0REu0q3QvZN9+J4KySUj+dp5i3AY938XqD9eMn8Qe3jTHOLi06tD1tzR7zlV2jz/DZ9QVu6X8P0mIGSexvwCt0Woa8tJH2L0ST7TteUvs/3TldzI0723pUbjdUFJosS3zOaA9Ovxn9Te7v4s+r1SMbXwCvMCS3

A select whose value is cleared/overridden inside the update handler keeps
showing the option the user picked instead of reflecting the new model value.

Root cause

2ffb956 (perf: optimize v-model multiple select w/ large lists) introduced
an _assigning flag so the updated hook can skip the redundant setSelected
call right after a user-initiated change. However, the flag is checked
unconditionally:

updated(el, { value }) {
  if (!el._assigning) {
    setSelected(el, value)
  }
}

If the model is changed to a value other than the one the user selected during
the same flush, _assigning is still true, so setSelected is skipped and
the DOM never gets re-synced.

Fix

Remember the value produced by the user interaction (_assignedValue) and only
skip the DOM sync while the model still matches it. When the model has been
overridden, fall back to setSelected, keeping the original perf optimization
for the common case (the looseEqual check short-circuits on reference
equality, which is the case for unchanged large lists).

Tests

Added a regression test under vModel.spec.ts that fails without the fix
(selectedIndex stays -1 instead of pointing to the overridden option) and
passes with it. All existing runtime-dom unit tests continue to pass.

Summary by CodeRabbit

  • Bug Fixes
    • Fixed v-model synchronization with select elements when update handlers modify the bound model value, ensuring the DOM's selected option state correctly matches the final component value and remains synchronized throughout the update cycle.

… handler (fix vuejs#10505)

When a `<select v-model>` change handler reassigns the model to a value
different from the user's selection (e.g. resetting it to null), the
`_assigning` flag added in 2ffb956 for perf caused the `updated` hook to
skip `setSelected`, leaving the DOM out of sync with the model.

Remember the value produced by the user interaction and only skip the
redundant DOM sync while it still matches the model; otherwise re-sync.
@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds an _assignedValue field to the internal vModelSelect element state. The change handler now stores the computed value on el._assignedValue after invoking the model assigner. The updated hook's guard condition is extended to also run setSelected when the current model value differs from el._assignedValue, fixing DOM sync when a change handler overrides the model to a different value.

Changes

vModelSelect override fix

Layer / File(s) Summary
_assignedValue tracking and updated hook condition
packages/runtime-dom/src/directives/vModel.ts
ModelDirective's internal element type gains _assignedValue?: any. The change handler computes and stores assignedValue to el._assignedValue. The updated hook condition adds || !looseEqual(value, el._assignedValue) so setSelected correctly re-syncs the DOM when the model is overridden to a different value inside the change handler.
Regression test
packages/runtime-dom/__tests__/directives/vModel.spec.ts
Adds test // #10505`` for a select whose `onUpdate:modelValue` handler forces the value to `'foo'` regardless of user selecting `'bar'`; asserts that both `data.value` and DOM option selected states reflect `'foo'` after the change event.

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

Suggested labels

:hammer: p3-minor-bug

Poem

🐰 A select once forgot what was chosen for real,
When a handler said "foo!" the DOM wouldn't heal.
Now _assignedValue remembers the last-assigned thought,
And the updated hook syncs what the override wrought.
The options align, no more stale selected state —
A fix small as a carrot, but precisely set straight! 🥕

🚥 Pre-merge checks | ✅ 5
✅ Passed checks (5 passed)
Check name Status Explanation
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check ✅ Passed The title accurately and concisely describes the main change: fixing the re-sync of select elements when v-model is overridden in a change handler, directly addressing issue #10505.
Linked Issues check ✅ Passed The code changes fully implement the fix for issue #10505 by tracking assigned values and conditionally skipping DOM sync when the model is overridden, including a regression test.
Out of Scope Changes check ✅ Passed All changes are directly related to fixing the v-model override issue in select elements; no unrelated modifications detected.
Docstring Coverage ✅ Passed No functions found in the changed files to evaluate docstring coverage. Skipping docstring coverage check.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🧹 Nitpick comments (1)
packages/runtime-dom/__tests__/directives/vModel.spec.ts (1)

1012-1056: ⚡ Quick win

Consider adding test coverage for additional scenarios.

The current test covers the core regression (override to a different non-null value). Consider adding tests for:

  1. Override to null (specifically mentioned in issue #10505)
  2. No override (verify optimization still works - no redundant DOM updates when model matches user selection)

These would provide more comprehensive coverage of the fix and ensure the performance optimization is preserved.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@packages/runtime-dom/__tests__/directives/vModel.spec.ts` around lines 1012 -
1056, Add two additional test cases to complement the existing test in the
vModel.spec.ts file. First, create a test case similar to the existing "should
sync DOM when the model is overridden in the change handler" test but configure
the onUpdate:modelValue handler to override the model value to null instead of
'foo', then verify the DOM correctly reflects null as the selected value.
Second, add another test case that verifies the optimization is preserved when
there is no override—set up a select element where the onUpdate:modelValue
handler allows the model to update to the value the user selected (without
override), then verify the DOM updates correctly and that there are no redundant
updates performed when the model already matches the user selection.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Nitpick comments:
In `@packages/runtime-dom/__tests__/directives/vModel.spec.ts`:
- Around line 1012-1056: Add two additional test cases to complement the
existing test in the vModel.spec.ts file. First, create a test case similar to
the existing "should sync DOM when the model is overridden in the change
handler" test but configure the onUpdate:modelValue handler to override the
model value to null instead of 'foo', then verify the DOM correctly reflects
null as the selected value. Second, add another test case that verifies the
optimization is preserved when there is no override—set up a select element
where the onUpdate:modelValue handler allows the model to update to the value
the user selected (without override), then verify the DOM updates correctly and
that there are no redundant updates performed when the model already matches the
user selection.

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro

Run ID: da3a30c6-6dbc-42ba-9ffd-1f0763f7b32d

📥 Commits

Reviewing files that changed from the base of the PR and between 478e3e8 and e3cc84e.

📒 Files selected for processing (2)
  • packages/runtime-dom/__tests__/directives/vModel.spec.ts
  • packages/runtime-dom/src/directives/vModel.ts

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Clearing select's value in change handler behaviour changed since 3.4.15 via webdriver

1 participant