fix(Separator): forward fall-through attributes to root#6641
Conversation
`createReusableTemplate` placed `DefineContainer` as a sibling root node, turning Separator into a multi-root component that can no longer auto-inherit fall-through attributes. Passing one (e.g. `data-slot` from `AuthForm` when both `fields` and `providers` are present) triggered a Vue "Extraneous non-props attributes" warning. Set `inheritAttrs: false` and forward `$attrs` explicitly to the root, matching the pattern used by the other multi-root components.
📝 WalkthroughWalkthrough
Estimated code review effort🎯 2 (Simple) | ⏱️ ~10 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches📝 Generate docstrings
🧪 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 |
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 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.
Inline comments:
In `@src/runtime/components/Separator.vue`:
- Line 107: Caller-provided data-slot is being overwritten by the Separator root
element because the static data-slot attribute wins over the v-bound attrs
spread. Update the Separator component’s root binding so the default "root"
value is applied inside the v-bind object rather than as a static attribute,
preserving any consumer-supplied $attrs data-slot while still falling back to
"root". Use the Separator template/root element and its existing
rootProps/$attrs merge as the place to fix this.
In `@test/components/Separator.spec.ts`:
- Around line 29-45: The Separator.spec.ts test that spies on console.warn
should always restore the spy in a finally block so failures in mountSuspended,
wrapper.get, or assertions do not leak the mock into later tests. Update the
test around the warn spy in the “forwards fall-through attributes to the root
without warning” case to wrap the assertions in try/finally and call
warn.mockRestore() in the finally path.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 1700b63a-a67a-4dc0-ab12-7cfb26753474
📒 Files selected for processing (2)
src/runtime/components/Separator.vuetest/components/Separator.spec.ts
commit: |
… finally Apply the default `data-slot="root"` inside the v-bind object so a consumer-supplied `$attrs['data-slot']` (e.g. AuthForm's "separator") takes precedence instead of being overwritten by the static attribute. Also wrap the fall-through test assertions in try/finally so the `console.warn` spy is always restored.
Revert the caller-supplied data-slot preservation. `data-slot` precedence is an inconsistency across the whole library (some components honor the caller, others force "root" or drop it) and is purely cosmetic since styling flows through the `class` prop. It will be addressed consistently in a dedicated PR. This keeps #6611 scoped to the actual fix: the multi-root fall-through warning. The try/finally test hygiene fix is retained.
🔗 Linked issue
Resolves #6611
❓ Type of change
📚 Description
AuthFormemitted a[Vue warn]: Extraneous non-props attributes (data-slot)…warning whenever bothfieldsandproviderswere present (i.e. when aSeparatoris rendered between them).The root cause is in
Separator, notAuthForm. #6415 introducedcreateReusableTemplate, which placed<DefineContainer>as a sibling root node of<Separator>. That turnedSeparatorinto a multi-root (fragment) component, and fragment components can't automatically inherit fall-through attributes — so thedata-slot="separator"thatAuthFormpasses (consistent with how it tags every other child, e.g.data-slot="input") could no longer be applied, producing the warning.The fix follows the pattern already used by the other multi-root components in the codebase (
Header,Sidebar,PricingPlan,Table,FileUpload,NavigationMenu):defineOptions({ inheritAttrs: false })v-bind="{ ...rootProps, ...$attrs }"This restores attribute forwarding to the
<Separator>root for any fall-through attribute (id,aria-*, events,data-*), with no change to the rendered DOM (no snapshot changes).📝 Checklist