fix(runtime-core): resolve $el for dev root comment fragment - #15313
fix(runtime-core): resolve $el for dev root comment fragment#15313oedumoreira wants to merge 3 commits into
Conversation
In dev mode, a leading comment before a component's single root element causes the compiler to wrap the root in a fragment flagged DEV_ROOT_FRAGMENT. $el (and template refs pointing at the component) resolved to the fragment's anchor node instead of the real rendered element, silently breaking any code relying on $el being an actual DOM element (measuring, focusing, third-party DOM integrations, etc). Resolve $el to the real single root via filterSingleRoot, the same utility already used to resolve attrs/scopeId fallthrough for this exact scenario in renderComponentRoot. Production builds are unaffected since dev comments are stripped from templates outside of dev. close vuejs#12680
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
Included review availability: Your plan provides up to 10 included reviews per hour; 9 remain after this review. 📝 WalkthroughWalkthroughDevelopment-mode ChangesDevelopment root fragment element resolution
Estimated code review effort: 3 (Moderate) | ~20 minutes Merge Risk: ⚪ Minimal · up to This localized change makes component references resolve to the actual root element in development builds without changing production behavior; no actionable merge-blocking risk remains after normal checks and review. Sequence Diagram(s)sequenceDiagram
participant ComponentInstance
participant resolveRootEl
participant RootVNode
participant RenderedElement
ComponentInstance->>resolveRootEl: Resolve development root
resolveRootEl->>RootVNode: Inspect fragment, component, or Suspense subtree
RootVNode-->>resolveRootEl: Return resolved root vnode
resolveRootEl->>RenderedElement: Resolve actual element
RenderedElement-->>ComponentInstance: Set `$el`
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
edison1105
left a comment
There was a problem hiding this comment.
Thanks for working on this. The overall direction looks correct: DEV_ROOT_FRAGMENT is the right way to distinguish a comment-only pseudo-fragment from a real multi-root component, and resolving this in the public $el getter preserves the renderer's internal fragment-anchor semantics.
However, the current resolver stops at component boundaries. For example:
<!-- Inner.vue -->
<!-- comment -->
<div /><!-- Outer.vue -->
<Inner />Outer.subTree is the Inner component VNode, so getDevRootFragmentEl(Outer) falls back to Outer.vnode.el. That value follows the normal component host-element chain to Inner.vnode.el, which is still the DEV_ROOT_FRAGMENT anchor. As a result, Outer.$el remains an anchor instead of the <div>. The same issue remains when both Outer and Inner have root comments: resolving the outer fragment returns the Inner VNode's .el, which is still the inner fragment anchor.
Could we make the public-root resolver recursively treat both DEV_ROOT_FRAGMENT and single-root component VNodes as transparent layers, following vnode.component.subTree where applicable? Please also add a regression test for an outer component rendering an inner component whose root is comment + div; covering the case where both layers are DEV_ROOT_FRAGMENT would be useful as well.
One minor test-fidelity point: the compiler emits PatchFlags.STABLE_FRAGMENT | PatchFlags.DEV_ROOT_FRAGMENT, so the hand-written test should preferably use that combination rather than DEV_ROOT_FRAGMENT alone.
The public $el resolver previously only unwrapped a component's own DEV_ROOT_FRAGMENT, so it stopped at component boundaries: a parent whose root is a child component (or another DEV_ROOT_FRAGMENT wrapping one) would still return the fragment anchor instead of the child's real rendered element. Resolve recursively through both DEV_ROOT_FRAGMENT layers and single-root component vnodes via vnode.component.subTree.
ea51f15 to
76149a6
Compare
|
Thanks so much for the detailed review, @edison1105, really appreciate you taking the time to walk through the component-boundary case, it helped me understand the resolution flow much better. I've updated the PR to address both points:
The branch is rebased on the latest main. Let me know if there's anything else you'd like adjusted, I'm still learning the codebase and really enjoying contributing to Vue core, so I'm happy to iterate further if needed. |
@vue/compiler-core
@vue/compiler-dom
@vue/compiler-sfc
@vue/compiler-ssr
@vue/reactivity
@vue/runtime-core
@vue/runtime-dom
@vue/server-renderer
@vue/shared
vue
@vue/compat
commit: |
Size ReportBundles
Usages
|
|
/ecosystem-ci run |
|
📝 Ran ecosystem CI: Open
|
close #12680
In dev mode, Vue preserves HTML comments written in a template. When a component's root is a single element preceded by a leading comment, e.g.: