feat(components): style properties support Boolean values#24165
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: defaults Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
🚧 Files skipped from review as they are similar to previous changes (3)
📝 WalkthroughWalkthroughMultiple components' style-related prop validators were changed to accept Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~20 minutes Possibly related PRs
Suggested labels
Suggested reviewers
Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 inconclusive)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ 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 |
commit: |
|
Size Change: +186 B (+0.01%) Total Size: 1.49 MB 📦 View Changed
ℹ️ View Unchanged
|
There was a problem hiding this comment.
Actionable comments posted: 4
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
packages/components/popper/src/content.ts (1)
124-148:⚠️ Potential issue | 🟠 MajorReorder
Booleanto last; also setdefault: undefinedforpopperStyle.Two issues with these changes:
Ordering (both
styleandpopperStyle):[Boolean, String, Array, Object]activates Vue's Boolean casting rule, so an empty-string value or shorthand attribute usage would be coerced totrue(not a validStyleValue). MoveBooleanto the end of the union.Implicit Boolean default on
popperStyle(Line 146): unlikestyle,popperStylewas not given an explicitdefault: undefined. BecauseBooleanis now in the type list, Vue treats an absent prop asfalserather thanundefined, changing the runtime value the component sees when the consumer doesn't passpopperStyle. Settingdefault: undefined(matchingstyleabove andpopperContentPropsDefaults.popperStyleat Line 194) restores prior behavior.🔧 Suggested fix
style: { - type: definePropType<StyleValue>([Boolean, String, Array, Object]), + type: definePropType<StyleValue>([String, Array, Object, Boolean]), default: undefined, }, ... popperStyle: { - type: definePropType<StyleValue>([Boolean, String, Array, Object]), + type: definePropType<StyleValue>([String, Array, Object, Boolean]), + default: undefined, },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/components/popper/src/content.ts` around lines 124 - 148, The prop definitions for style and popperStyle are currently ordering Boolean first and missing an explicit default for popperStyle; update both prop definitions (the style and popperStyle props in this file where definePropType<StyleValue> is used) to put Boolean at the end of the type union (e.g., [String, Array, Object, Boolean]) to avoid Vue's Boolean casting, and add default: undefined to the popperStyle prop to match style and the existing popperContentPropsDefaults.popperStyle so the prop value is undefined when not provided.packages/components/scrollbar/src/scrollbar.ts (1)
120-144:⚠️ Potential issue | 🟠 MajorMove
Booleanto the end of the type unions.Same Vue Boolean-casting concern as elsewhere in this PR — with
Booleanlisted beforeString, any empty-string value or shorthand attribute (<el-scrollbar wrap-style />) onwrapStyle/viewStylewill now be coerced totrue, which is not a validStyleValueand is a behavioral regression relative to before this PR. ReorderingBooleanto the tail of the union keeps the original string behavior while still allowingfalse.Also worth double-checking: these two props keep
default: ''rather than thedefault: undefinedused elsewhere in this PR — that's fine for the casting issue (defaults aren't re-cast), but the ordering still matters for explicit consumer-supplied values.🔧 Suggested ordering
wrapStyle: { - type: definePropType<StyleValue>([Boolean, String, Object, Array]), + type: definePropType<StyleValue>([String, Object, Array, Boolean]), default: '', }, ... viewStyle: { - type: [Boolean, String, Array, Object], + type: [String, Array, Object, Boolean], default: '', },🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/components/scrollbar/src/scrollbar.ts` around lines 120 - 144, The prop type unions for wrapStyle and viewStyle currently list Boolean before String which causes Vue to coerce empty-string/shorthand attributes to true; update the type arrays in the wrapStyle and viewStyle prop definitions (the definePropType<StyleValue>(...) and the viewStyle prop's type) to place Boolean at the end of the union (e.g., [String, Object, Array, Boolean] or [Boolean, String, Array, Object] → move Boolean last) while keeping the existing default: '' so explicit empty-string behavior is preserved.
🧹 Nitpick comments (1)
packages/components/space/src/space.ts (1)
56-59: Optional: consider unifying default values for style props across the PR.Across this PR, the new/updated style props use a mix of
default: '',default: () => ({}),default: () => mutable({} as const), anddefault: undefined. Functionally all are validStyleValues, but a single convention (likelydefault: undefined, as adopted inbadge,virtual-list,statistic, andcolor-picker-panel) would be easier to reason about and reduce churn for downstreamExtractPublicPropTypesconsumers. Non-blocking; defer if backward-compat is a concern.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@packages/components/space/src/space.ts` around lines 56 - 59, The style prop in the Space component (property name: style, defined with definePropType<StyleValue>) currently uses default: '' — change its default to undefined to match the convention used elsewhere (e.g., badge, virtual-list, statistic, color-picker-panel) so ExtractPublicPropTypes consumers see a consistent default for StyleValue; update the prop definition for style in space.ts accordingly.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@packages/components/avatar/src/avatar-group-props.ts`:
- Around line 71-74: The collapseStyle prop's type union currently lists Boolean
before String which causes Vue to coerce empty/shorthand attributes into true;
update the type array for collapseStyle in avatar-group-props.ts (the
definePropType<StyleValue> for collapseStyle) so Boolean is placed last (e.g.,
[String, Array, Object, Boolean]) to preserve normal string handling and prevent
unwanted boolean coercion.
In `@packages/components/countdown/src/countdown.ts`:
- Around line 68-71: The prop definition for valueStyle currently uses
definePropType<StyleValue>([Boolean, String, Object, Array]) which causes Vue to
cast empty-string props to true; update the union order for valueStyle to put
Boolean last (e.g. [String, Object, Array, Boolean]) so string values (including
empty string) are preserved while still allowing false, adjusting the valueStyle
prop definition in the countdown component accordingly.
In `@packages/components/roving-focus-group/src/roving-focus-group.ts`:
- Around line 12-15: The prop declaration for style uses
definePropType<StyleValue> with types [Boolean, String, Array, Object] which
causes Vue to coerce string values to Boolean; change the type order in the
style prop so Boolean is last (e.g., [String, Array, Object, Boolean]) to avoid
unintended Boolean casting; update the style prop definition where
definePropType<StyleValue> is used to reflect this new ordering.
In `@packages/components/table-v2/src/cell.ts`:
- Around line 13-16: The style prop definition uses
definePropType<StyleValue>([Boolean, String, Array, Object]) which causes
empty-string/shorthand attributes to be cast to true; update the union ordering
so Boolean is last (e.g., [String, Array, Object, Boolean]) in the
definePropType call for the style prop to prevent unintended boolean casting
while keeping StyleValue typings intact.
---
Outside diff comments:
In `@packages/components/popper/src/content.ts`:
- Around line 124-148: The prop definitions for style and popperStyle are
currently ordering Boolean first and missing an explicit default for
popperStyle; update both prop definitions (the style and popperStyle props in
this file where definePropType<StyleValue> is used) to put Boolean at the end of
the type union (e.g., [String, Array, Object, Boolean]) to avoid Vue's Boolean
casting, and add default: undefined to the popperStyle prop to match style and
the existing popperContentPropsDefaults.popperStyle so the prop value is
undefined when not provided.
In `@packages/components/scrollbar/src/scrollbar.ts`:
- Around line 120-144: The prop type unions for wrapStyle and viewStyle
currently list Boolean before String which causes Vue to coerce
empty-string/shorthand attributes to true; update the type arrays in the
wrapStyle and viewStyle prop definitions (the definePropType<StyleValue>(...)
and the viewStyle prop's type) to place Boolean at the end of the union (e.g.,
[String, Object, Array, Boolean] or [Boolean, String, Array, Object] → move
Boolean last) while keeping the existing default: '' so explicit empty-string
behavior is preserved.
---
Nitpick comments:
In `@packages/components/space/src/space.ts`:
- Around line 56-59: The style prop in the Space component (property name:
style, defined with definePropType<StyleValue>) currently uses default: '' —
change its default to undefined to match the convention used elsewhere (e.g.,
badge, virtual-list, statistic, color-picker-panel) so ExtractPublicPropTypes
consumers see a consistent default for StyleValue; update the prop definition
for style in space.ts accordingly.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 07c1c392-6d65-42c1-8245-f78d64b24d08
📒 Files selected for processing (14)
packages/components/avatar/src/avatar-group-props.tspackages/components/badge/src/badge.tspackages/components/card/src/card.tspackages/components/color-picker-panel/src/color-picker-panel.tspackages/components/countdown/src/countdown.tspackages/components/input/src/input.tspackages/components/popper/src/content.tspackages/components/roving-focus-group/src/roving-focus-group.tspackages/components/scrollbar/src/scrollbar.tspackages/components/space/src/space.tspackages/components/statistic/src/statistic.tspackages/components/table-v2/src/cell.tspackages/components/table/src/table/defaults.tspackages/components/virtual-list/src/props.ts

fix #24102
Please make sure these boxes are checked before submitting your PR, thank you!
devbranch.Summary by CodeRabbit