core icon indicating copy to clipboard operation
core copied to clipboard

fix(v-model.trim): correct the activeElement discovery for shadow DOM

Open LordSalmon opened this issue 2 weeks ago β€’ 8 comments

Cheers everyone

Issue:

The issue is, that using v-model.trim in the shadow dom does not behave the same as in the light dom. When an input should get trimmed in the light dom, the ref value itself gets trimmed correctly, but the displayed characters in the input won't get trimmed until you refocus the input. I like that approach since it reduces noise for the cursor and doesn't seem glitchy. However, when trimming in the shadow dom, the displayed characters (not equivalent to the value property of the input element at that moment!) are trimmed, which increases visual noise.

A demo with reproduction steps can be seen here: https://vue-3-v-model-trim-misbehaviour.vercel.app/ With the repository: https://github.com/LordSalmon/vue-3-v-model-trim-misbehaviour

The problem seems to be that line which compares the currently focused element to the activeElement of the document. However, when the input is placed inside the shadow dom, document.activeElement returns the shadow dom root node instead of the input.

Fix idea:

Initial Idea: - Checking at that point of execution whether document.activeElement is the root node for a shadow dom and if so, return the activeElement of the shadow dom.

Improvement: To support nested shadow doms, el.getRootNode() is used to directly verify the activeElement property from that root instead of starting at the root of the root document itself.

Tests:

I tried to create a test for that, but it seems as if js-dom's focus api is not entirely consistent. Reproducing the step where the displayed characters in the input still contained the spaces but were gone on refocus was not possible. But If someone has a flash of inspiration, let me know or feel free to contribute.

Summary by CodeRabbit

  • Bug Fixes
    • Improved focus detection for v-model so it correctly respects shadow DOM boundaries and excludes range inputs, preserving existing lazy/trim/number behaviors.
    • No changes to public API or exported signatures.

✏️ Tip: You can customize this high-level summary in your review settings.

LordSalmon avatar Dec 12 '25 16:12 LordSalmon

Walkthrough

Replace the direct document.activeElement === el check in vModelText.beforeUpdate with a root-aware focus resolution that uses el.getRootNode() and only considers rootNode.activeElement === el when rootNode is a Document or ShadowRoot, also excluding el.type === 'range'. No exported signatures changed.

Changes

Cohort / File(s) Summary
vModel focus handling
packages/runtime-dom/src/directives/vModel.ts
Replace direct document.activeElement === el check with root-aware resolution using el.getRootNode(); consider Document or ShadowRoot roots and ensure rootNode.activeElement === el and el.type !== 'range'; preserve existing lazy/trim/number and early-return logic

Estimated code review effort

🎯 2 (Simple) | ⏱️ ~10 minutes

  • Single-file change with focused logic tweak.
  • Review attention:
    • Verify focus resolution across Shadow DOM boundaries and nested shadow roots.
    • Confirm el.type !== 'range' exclusion is preserved and correct.
    • Ensure interaction with lazy/trim/number branches and early-return conditions remains unchanged.

Poem

🐰 I peeked beneath the shadowed dome,
Where focused leaves once lost their home,
I nudge the root, I call it right,
So active fields regain their light,
A tiny hop to fix the roam.

Pre-merge checks and finishing touches

❌ Failed checks (1 warning)
Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 0.00% which is insufficient. The required threshold is 80.00%. You can run @coderabbitai generate docstrings to improve docstring coverage.
βœ… Passed checks (2 passed)
Check name Status Explanation
Description Check βœ… Passed Check skipped - CodeRabbit’s high-level summary is enabled.
Title check βœ… Passed The title directly summarizes the main change: fixing Shadow DOM handling in v-model's activeElement check, which is the core issue addressed by the changeset.
✨ Finishing touches
  • [ ] πŸ“ Generate docstrings
πŸ§ͺ Generate unit tests (beta)
  • [ ] Create PR with unit tests
  • [ ] Post copyable unit tests in a comment

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[bot] avatar Dec 12 '25 16:12 coderabbitai[bot]

@coderabbitai review

LordSalmon avatar Dec 14 '25 09:12 LordSalmon

βœ… Actions performed

Review triggered.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

coderabbitai[bot] avatar Dec 14 '25 09:12 coderabbitai[bot]

Size Report

Bundles

File Size Gzip Brotli
runtime-dom.global.prod.js 103 kB (+63 B) 39.1 kB (+20 B) 35.2 kB (+28 B)
vue.global.prod.js 162 kB (+63 B) 59.1 kB (+20 B) 52.6 kB (-13 B)

Usages

Name Size Gzip Brotli
createApp (CAPI only) 47 kB 18.4 kB 16.8 kB
createApp 55.2 kB 21.4 kB 19.6 kB
createSSRApp 59.4 kB 23.2 kB 21.1 kB
defineCustomElement 60.7 kB 23.1 kB 21.1 kB
overall 69.5 kB 26.7 kB 24.3 kB

github-actions[bot] avatar Dec 15 '25 01:12 github-actions[bot]

Open in StackBlitz

@vue/compiler-core

pnpm add https://pkg.pr.new/@vue/compiler-core@14196
npm i https://pkg.pr.new/@vue/compiler-core@14196
yarn add https://pkg.pr.new/@vue/[email protected]
@vue/compiler-dom

pnpm add https://pkg.pr.new/@vue/compiler-dom@14196
npm i https://pkg.pr.new/@vue/compiler-dom@14196
yarn add https://pkg.pr.new/@vue/[email protected]
@vue/compiler-sfc

pnpm add https://pkg.pr.new/@vue/compiler-sfc@14196
npm i https://pkg.pr.new/@vue/compiler-sfc@14196
yarn add https://pkg.pr.new/@vue/[email protected]
@vue/compiler-ssr

pnpm add https://pkg.pr.new/@vue/compiler-ssr@14196
npm i https://pkg.pr.new/@vue/compiler-ssr@14196
yarn add https://pkg.pr.new/@vue/[email protected]
@vue/reactivity

pnpm add https://pkg.pr.new/@vue/reactivity@14196
npm i https://pkg.pr.new/@vue/reactivity@14196
yarn add https://pkg.pr.new/@vue/[email protected]
@vue/runtime-core

pnpm add https://pkg.pr.new/@vue/runtime-core@14196
npm i https://pkg.pr.new/@vue/runtime-core@14196
yarn add https://pkg.pr.new/@vue/[email protected]
@vue/runtime-dom

pnpm add https://pkg.pr.new/@vue/runtime-dom@14196
npm i https://pkg.pr.new/@vue/runtime-dom@14196
yarn add https://pkg.pr.new/@vue/[email protected]
@vue/server-renderer

pnpm add https://pkg.pr.new/@vue/server-renderer@14196
npm i https://pkg.pr.new/@vue/server-renderer@14196
yarn add https://pkg.pr.new/@vue/[email protected]
@vue/shared

pnpm add https://pkg.pr.new/@vue/shared@14196
npm i https://pkg.pr.new/@vue/shared@14196
yarn add https://pkg.pr.new/@vue/[email protected]
vue

pnpm add https://pkg.pr.new/vue@14196
npm i https://pkg.pr.new/vue@14196
yarn add https://pkg.pr.new/[email protected]
@vue/compat

pnpm add https://pkg.pr.new/@vue/compat@14196
npm i https://pkg.pr.new/@vue/compat@14196
yarn add https://pkg.pr.new/@vue/[email protected]

commit: e700b25

pkg-pr-new[bot] avatar Dec 15 '25 01:12 pkg-pr-new[bot]

/ecosystem-ci run

edison1105 avatar Dec 15 '25 01:12 edison1105

Thanks for the PR. LGTM.

I tried to create a test for that, but it seems as if js-dom's focus api is not entirely consistent. Reproducing the step where the displayed characters in the input still contained the spaces but were gone on refocus was not possible. But If someone has a flash of inspiration, let me know or feel free to contribute.

Try adding an e2e test in packages/vue/__tests__/e2e/vModel.spec.ts

edison1105 avatar Dec 15 '25 02:12 edison1105

πŸ“ Ran ecosystem CI: Open

suite result latest scheduled
language-tools :x: failure :x: failure
primevue :white_check_mark: success :white_check_mark: success
quasar :white_check_mark: success :white_check_mark: success
radix-vue :white_check_mark: success :white_check_mark: success
nuxt :white_check_mark: success :white_check_mark: success
pinia :white_check_mark: success :white_check_mark: success
vant :white_check_mark: success :white_check_mark: success
test-utils :white_check_mark: success :white_check_mark: success
vuetify :x: failure :x: failure
vitepress :white_check_mark: success :white_check_mark: success
vue-macros :x: failure :x: failure
router :white_check_mark: success :white_check_mark: success
vite-plugin-vue :white_check_mark: success :white_check_mark: success
vue-i18n :x: failure :x: failure
vue-simple-compiler :white_check_mark: success :white_check_mark: success
vueuse :white_check_mark: success :white_check_mark: success

vue-bot avatar Dec 15 '25 09:12 vue-bot