fix(theme): blank top-level base in applyUnstyled - #6825
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
📝 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)
Warning There were issues while running some tools. Please review the errors and either fix the tool's configuration or disable the tool if it's a critical failure. 🔧 ESLint
src/utils/theme.tsParsing error: Unexpected token : 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 |
Merging this PR will not alter performance
Comparing |
commit: |
🔗 Linked issue
Relates to #6551
❓ Type of change
📚 Description
applyUnstyledonly blanksslots,variantsandcompoundVariants. Themes come in two shapes: multi-element components put their classes underslots, single-element ones are a top-levelbasestring (or array). There's nobasebranch, so withtheme.unstyledenabled the 37 base-shaped themes (skeleton,kbd,container,main, the prose elements, ...) come out identical to the original and keep every class, while everything slots-shaped is blanked. The docs promise the option removes the default classes from all components at once, so this is a plain hole.While in there,
applyUnstyledno longer mutates the theme it receives: object-shaped themes are the shared module export (function-shaped ones produce a fresh object per call), so blanking in place corrupts every later read within the same process.Covered by unit tests for both fixes. #6731 blanks unused component themes through this same function, so it picks this up on its next
v4merge.📝 Checklist