fix(runtime-core): resolve kebab-case slot names from in-DOM templates#14302
fix(runtime-core): resolve kebab-case slot names from in-DOM templates#14302edison1105 merged 6 commits intomainfrom
Conversation
📝 WalkthroughWalkthroughCentralizes slots proxy behavior, adds kebab-case (hyphenated) slot name lookup for in‑DOM/browser contexts in renderSlot and proxies, updates KeepAlive condition from BROWSER to GLOBAL, and adds tests covering camelCase↔kebab-case slot resolution and exact‑match precedence across dev/prod modes. Changes
Sequence Diagram(s)sequenceDiagram
participant Component
participant renderSlot
participant SlotsProxy
participant Hyphenate
Component->>renderSlot: renderSlot(name, slots, props, ...)
renderSlot->>SlotsProxy: lookup slots[name]
alt slot found
SlotsProxy-->>renderSlot: slot fn
else not found and __GLOBAL__ (browser/in‑DOM)
renderSlot->>Hyphenate: hyphenate(name)
Hyphenate-->>renderSlot: hyphenatedName
renderSlot->>SlotsProxy: lookup slots[hyphenatedName]
SlotsProxy-->>renderSlot: slot fn (if exists)
end
renderSlot-->>Component: invoke slot fn / render content
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
📜 Recent review detailsConfiguration used: defaults Review profile: CHILL Plan: Pro 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
⏰ Context from checks skipped due to timeout of 90000ms. You can increase the timeout in your CodeRabbit configuration to a maximum of 15 minutes (900000ms). (1)
Comment |
Size ReportBundles
Usages
|
@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: |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In @packages/runtime-core/src/component.ts:
- Around line 1114-1124: The Proxy get trap in createSlotsProxyHandlers calls
hyphenate(key) but doesn't guard against symbol keys, causing runtime
exceptions; update the get handler in createSlotsProxyHandlers to first detect
if key is a symbol and in that case return target[key] directly (or use
Reflect.get) without calling hyphenate, otherwise continue existing logic
(including the __BROWSER__ hyphenate fallback for string keys) so symbol
property accesses on InternalSlots are safe.
🧹 Nitpick comments (1)
packages/runtime-core/__tests__/componentSlots.spec.ts (1)
467-579: Improve test isolation: restore previous__BROWSER__/__DEV__values instead of forcingfalse/true.
This makes the suite robust to different harness defaults and future refactors.Proposed patch
describe('in-DOM template kebab-case slot name resolution', () => { + let prevBrowser: boolean beforeEach(() => { + prevBrowser = __BROWSER__ __BROWSER__ = true }) afterEach(() => { - __BROWSER__ = false + __BROWSER__ = prevBrowser }) test('should resolve camelCase slot access to kebab-case via slots (PROD)', () => { - __DEV__ = false + const prevDev = __DEV__ + __DEV__ = false try { const Comp = { setup(_: any, { slots }: any) { // Access with camelCase, but slot is passed with kebab-case return () => slots.dropdownRender() }, } @@ createApp(App).mount(root) expect(serializeInner(root)).toBe('dropdown content') } finally { - __DEV__ = true + __DEV__ = prevDev } })
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/runtime-core/__tests__/componentSlots.spec.tspackages/runtime-core/src/component.tspackages/runtime-core/src/helpers/renderSlot.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/runtime-core/src/helpers/renderSlot.ts (1)
packages/shared/src/general.ts (1)
hyphenate(118-120)
packages/runtime-core/src/component.ts (2)
packages/runtime-core/src/componentSlots.ts (1)
InternalSlots(34-36)packages/shared/src/general.ts (1)
hyphenate(118-120)
🔇 Additional comments (5)
packages/runtime-core/src/helpers/renderSlot.ts (2)
17-17: Good addition:hyphenateimport is appropriately scoped to the new lookup.
56-58: Slot resolution behavior looks correct (exact name first, kebab-case fallback only in browser).
This matches in-DOM template constraints while keeping SSR unaffected.packages/runtime-core/__tests__/componentSlots.spec.ts (1)
14-15: Nice: tests explicitly coverrenderSlot+ rendering-instance wiring.packages/runtime-core/src/component.ts (2)
69-70:hyphenateimport is a good fit for the intended in-DOM kebab-case behavior.
1166-1173: Wiring looks good: consistent slots proxying in dev, and browser-only in prod to minimize overhead.Also applies to: 1181-1187
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/runtime-core/src/component.ts (1)
1114-1127: Well-structured kebab-case slot resolution with correct priority.The proxy handler correctly:
- Tracks slot access in dev mode
- Prioritizes exact matches over kebab-case fallback
- Guards hyphenation with
__BROWSER__and string type check- Centralizes slot proxy behavior for maintainability
♻️ Optional: Make symbol key handling more explicit
The current logic using
||could returnfalseinstead ofundefinedwhen accessing a non-existent slot with a symbol key (an edge case since slots use string names). For clarity:const createSlotsProxyHandlers = ( instance: ComponentInternalInstance, ): ProxyHandler<InternalSlots> => ({ get(target, key: string | symbol) { if (__DEV__) { track(instance, TrackOpTypes.GET, '$slots') } - // in-DOM templates use kebab-case slot names, only relevant in browser - return ( - target[key as string] || - (__BROWSER__ && typeof key === 'string' && target[hyphenate(key)]) - ) + // in-DOM templates use kebab-case slot names, only relevant in browser + return ( + target[key as string] ?? + (__BROWSER__ && typeof key === 'string' ? target[hyphenate(key)] : undefined) + ) }, })Using
??(nullish coalescing) ensuresundefinedis returned for missing slots rather thanfalse. However, this is unlikely to affect real usage since slots use string keys.
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
packages/runtime-core/src/component.ts
🔇 Additional comments (3)
packages/runtime-core/src/component.ts (3)
69-69: LGTM: Import is necessary for kebab-case resolution.The
hyphenateimport from@vue/sharedis correctly added to support converting camelCase slot names to kebab-case for in-DOM template slot resolution.
1169-1175: LGTM: Dev mode slots proxy correctly uses the new handler.The lazy initialization pattern is consistent with
attrsProxy, and the use ofcreateSlotsProxyHandlersensures both slot access tracking and kebab-case resolution (when__BROWSER__is true) work correctly in development mode.
1185-1187: LGTM: Production mode efficiently handles browser-specific slot resolution.The conditional proxy creation is performance-conscious:
- Browser contexts get kebab-case resolution via proxy (no tracking overhead since
__DEV__is false)- SSR contexts avoid proxy overhead entirely by using plain
instance.slotsThis aligns perfectly with the fix objective—handling browser HTML attribute lowercasing while keeping SSR efficient.
There was a problem hiding this comment.
Actionable comments posted: 0
🧹 Nitpick comments (1)
packages/runtime-core/src/component.ts (1)
1114-1127: Consider improving type safety in the proxy get handler.The proxy handler receives
key: string | symbolbut the first access pathtarget[key as string]doesn't guard against symbols. WhileInternalSlotsis typed as{ [name: string]: Slot | undefined }(so symbols would return undefined), it's clearer to be explicit.Additionally, this logic duplicates the kebab-case resolution from
renderSlot.ts. If the logic needs to change in the future, both locations must be updated.Optional: More explicit type handling
const createSlotsProxyHandlers = ( instance: ComponentInternalInstance, ): ProxyHandler<InternalSlots> => ({ get(target, key: string | symbol) { if (__DEV__) { track(instance, TrackOpTypes.GET, '$slots') } + if (typeof key !== 'string') { + return target[key as any] + } // in-DOM templates use kebab-case slot names, only relevant in global builds return ( - target[key as string] || - (__GLOBAL__ && typeof key === 'string' && target[hyphenate(key)]) + target[key] || + (__GLOBAL__ && target[hyphenate(key)]) ) }, })
📜 Review details
Configuration used: defaults
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
packages/runtime-core/__tests__/componentSlots.spec.tspackages/runtime-core/src/component.tspackages/runtime-core/src/helpers/renderSlot.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- packages/runtime-core/tests/componentSlots.spec.ts
🧰 Additional context used
🧬 Code graph analysis (2)
packages/runtime-core/src/helpers/renderSlot.ts (2)
packages/runtime-core/src/component.ts (1)
slots(1168-1176)packages/shared/src/general.ts (1)
hyphenate(118-120)
packages/runtime-core/src/component.ts (2)
packages/runtime-core/src/componentSlots.ts (1)
InternalSlots(34-36)packages/shared/src/general.ts (1)
hyphenate(118-120)
🔇 Additional comments (5)
packages/runtime-core/src/helpers/renderSlot.ts (2)
17-17: LGTM: Import addition is appropriate.The
hyphenateimport is necessary for the kebab-case slot name resolution in line 57.
56-57: Edge cases for kebab-case slot name fallback are properly handled.The code correctly prioritizes exact matches before the kebab-case fallback, and the
__GLOBAL__guard appropriately limits this feature to global builds. Thehyphenate()function properly handles all edge cases:
- Empty strings, special characters, and already-kebab-cased names are handled correctly by the regex pattern
/\B([A-Z])/g- When both
fooBarandfoo-barslots are defined, exact match (fooBar) takes precedence, as confirmed by existing testspackages/runtime-core/src/component.ts (3)
69-69: LGTM: Import addition is appropriate.The
hyphenateimport is required for the newcreateSlotsProxyHandlersfunction.
1169-1175: LGTM: Dev mode slots proxy implementation.The lazy initialization pattern with caching is appropriate for development mode, and using the centralized
createSlotsProxyHandlersensures consistent behavior.
1185-1187: Good optimization: Conditional proxy in production.The conditional proxy based on
__GLOBAL__is a smart optimization—non-global builds (ESM, bundler) skip the proxy overhead since kebab-case resolution is only needed for in-DOM templates in global/browser builds.Note: Dev mode always uses the proxy (see lines 1169-1175), but the hyphenation logic is still gated by
__GLOBAL__inside the handler, maintaining consistent behavior across modes.
edison1105
added
ready to merge
skirtles-code
left a comment
skirtles-code
left a comment
There was a problem hiding this comment.
I'm not necessarily opposed to this, but I think the decision to support case conversion for slot names is more than just a minor fix. I'm not sure exactly what ready to merge indicates, but personally I feel this needs more scrutiny from the team before committing to supporting this.
This isn't a new problem. We've definitely discussed this several times on Vue Land, usually when someone asks about the best naming convention for slots. e.g.:
I haven't been able to find any previous discussion of it on GitHub, but I'd be surprised if some form of case conversion or normalisation hasn't been considered before.
Adding partial support for this could be the thin end of the wedge and alternative approaches are available.
This reverts commit bcbb196.
edison1105
added
ready for review
|
@skirtles-code First, to clarify, this problem only occurs in the in-DOM template scenario. As we know, in the browser environment, prop names and v-on event names must use kebab-case—this is an established Vue convention (see the docs). While not explicitly stated, slot names, as part of template attributes, should logically follow the same convention. Specifically regarding issue #14300, it's essentially the same problem discussed on Discord(you provided above): The fix I proposed is actually quite straightforward: when My intention was not to rush this through, but rather to provide a concrete, low-risk solution for discussion. I'm more than happy to revert the status and assist with a more in-depth exploration. |
|
Since users are allowed to define slots using camel case, there should at least be a way for them to work correctly in the dom. Are there any discussions about not converting slot names from kebab case? |
3 participants
closes #14300
Summary by CodeRabbit
Bug Fixes
Tests
✏️ Tip: You can customize this high-level summary in your review settings.