Movatterモバイル変換


[0]ホーム

URL:


Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly

Sign up
Appearance settings
/corePublic

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

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to ourterms of service andprivacy statement. We’ll occasionally send you account related emails.

Already on GitHub?Sign in to your account

Open
LordSalmon wants to merge2 commits intovuejs:main
base:main
Choose a base branch
Loading
fromLordSalmon:lordsalmon/fix/v-model-trim-behaviour-in-shadow-dom

Conversation

@LordSalmon
Copy link

@LordSalmonLordSalmon commentedDec 12, 2025
edited
Loading

Cheers everyone

Issue:

The issue is, that usingv-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 thevalue 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 bethat 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 whetherdocument.activeElement is the root node for a shadow dom and if so, return theactiveElement of the shadow dom.

Improvement: To support nested shadow doms,el.getRootNode() is used to directly verify theactiveElement 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.

@coderabbitai
Copy link

coderabbitaibot commentedDec 12, 2025
edited
Loading

Walkthrough

Replace the directdocument.activeElement === el check in vModelText.beforeUpdate with a root-aware focus resolution that usesel.getRootNode() and only considersrootNode.activeElement === el whenrootNode is aDocument orShadowRoot, also excludingel.type === 'range'. No exported signatures changed.

Changes

Cohort / File(s)Summary
vModel focus handling
packages/runtime-dom/src/directives/vModel.ts
Replace directdocument.activeElement === el check with root-aware resolution usingel.getRootNode(); considerDocument orShadowRoot roots and ensurerootNode.activeElement === el andel.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.
    • Confirmel.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 nameStatusExplanationResolution
Docstring Coverage⚠️ WarningDocstring 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 nameStatusExplanation
Title check✅ PassedThe title accurately summarizes the main change: fixing activeElement discovery for Shadow DOM in v-model.trim behavior.
Description Check✅ PassedCheck skipped - CodeRabbit’s high-level summary is enabled.
✨ Finishing touches
  • 📝 Generate docstrings
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Post copyable unit tests in a comment

Thanks for usingCodeRabbit! 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.

Copy link

@coderabbitaicoderabbitaibot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

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

54-58:HardenactiveElement() fornull inner activeElement + nested shadow roots

Right now, ifdocument.activeElement.shadowRoot exists butshadowRoot.activeElement isnull, the helper returnsnull instead of falling back to the document’s active element. Also, nested shadow roots won’t be fully resolved.

Consider:

-function activeElement() {-  return document.activeElement && document.activeElement.shadowRoot-    ? document.activeElement.shadowRoot.activeElement-    : document.activeElement-}+function activeElement(): Element | null {+  let el: Element | null = document.activeElement+  while (el && el.shadowRoot?.activeElement) {+    el = el.shadowRoot.activeElement+  }+  return el+}
📜 Review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and betweenf51d3e2 and287ae18.

📒 Files selected for processing (1)
  • packages/runtime-dom/src/directives/vModel.ts (2 hunks)
🔇 Additional comments (1)
packages/runtime-dom/src/directives/vModel.ts (1)

111-119:Good focus detection fix for Shadow DOM; please add a regression test (likely e2e)

UsingactiveElement() here matches the intended “don’t clobber the user’s in-progress text while focused” behavior in shadow trees. The remaining risk is regression without coverage, and jsdom focus behavior is known to be flaky for shadow DOM.

The current approach grants support for nested shadow doms
@LordSalmon
Copy link
Author

@coderabbitai review

coderabbitai[bot] reacted with eyes emoji

@coderabbitai
Copy link

✅ 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.

@github-actions
Copy link

Size Report

Bundles

FileSizeGzipBrotli
runtime-dom.global.prod.js103 kB (+63 B)39 kB (+22 B)35.2 kB (+49 B)
vue.global.prod.js161 kB (+63 B)59 kB (+17 B)52.5 kB (-71 B)

Usages

NameSizeGzipBrotli
createApp (CAPI only)47 kB18.3 kB16.8 kB
createApp55.1 kB21.4 kB19.6 kB
createSSRApp59.4 kB23.2 kB21.1 kB
defineCustomElement60.7 kB23.1 kB21.1 kB
overall69.4 kB26.6 kB24.3 kB

@pkg-pr-new
Copy link

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/compiler-core@14196.tgz

@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/compiler-dom@14196.tgz

@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/compiler-sfc@14196.tgz

@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/compiler-ssr@14196.tgz

@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/reactivity@14196.tgz

@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/runtime-core@14196.tgz

@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/runtime-dom@14196.tgz

@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/server-renderer@14196.tgz

@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/shared@14196.tgz

vue

pnpm add https://pkg.pr.new/vue@14196
npm i https://pkg.pr.new/vue@14196
yarn add https://pkg.pr.new/vue@14196.tgz

@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/compat@14196.tgz

commit:acebcdd

@edison1105
Copy link
Member

/ecosystem-ci run

github-actions[bot] reacted with thumbs up emoji

@edison1105
Copy link
Member

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 inpackages/vue/__tests__/e2e/vModel.spec.ts

@edison1105edison1105 added scope: v-model 🔨 p3-minor-bugPriority 3: this fixes a bug, but is an edge case that only affects very specific usage. need testThe PR has missing test cases. labelsDec 15, 2025
@vue-bot
Copy link
Contributor

📝 Ran ecosystem CI:Open

suiteresultlatest scheduled
language-toolsfailurefailure
primevuesuccesssuccess
quasarsuccesssuccess
radix-vuesuccesssuccess
nuxtsuccesssuccess
piniasuccesssuccess
vantsuccesssuccess
test-utilssuccesssuccess
vuetifyfailurefailure
vitepresssuccesssuccess
vue-macrosfailurefailure
routersuccesssuccess
vite-plugin-vuesuccesssuccess
vue-i18nfailurefailure
vue-simple-compilersuccesssuccess
vueusesuccesssuccess

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

Reviewers

@coderabbitaicoderabbitai[bot]coderabbitai[bot] left review comments

Assignees

No one assigned

Labels

🔨 p3-minor-bugPriority 3: this fixes a bug, but is an edge case that only affects very specific usage.need testThe PR has missing test cases.scope: v-model

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@LordSalmon@edison1105@vue-bot

[8]ページ先頭

©2009-2025 Movatter.jp