fix: updateCSSVariable inside scoped theme#502
Conversation
📝 WalkthroughWalkthroughThe PR refactors CSS variable management from in-memory storage to DOM-backed CSS rules, introducing a dynamic stylesheet approach that ensures ScopedTheme components and non-global theme overrides reliably reflect runtime variable updates across native and web platforms. Changes
Sequence DiagramsequenceDiagram
participant App as Application
participant Uniwind as Uniwind.updateCSSVariables()
participant DOMRules as Dynamic CSS Rules<br/>(DOM-backed)
participant Listener as UniwindListener
participant Component as ScopedTheme Component
App->>Uniwind: updateCSSVariables(theme, vars)
Uniwind->>DOMRules: getUniwindDynamicCSSRules() if needed
DOMRules->>DOMRules: Inject `#uniwind-dynamic-styles` element<br/>with theme selectors (.light, .dark, etc)
Uniwind->>DOMRules: For each cached rule, call<br/>rule.style.setProperty(varName, value)<br/>for matching theme
Uniwind->>Listener: notify([StyleDependency.Variables])
Listener->>Component: Trigger component subscribed<br/>to Variables dependency
Component->>Component: Re-render with updated<br/>CSS variables from DOM
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/uniwind/src/core/config/config.ts`:
- Around line 78-91: The code builds CSS selectors from theme names without
escaping, causing invalid selectors for names like "2xl" or "brand:premium"; fix
by using CSS.escape when generating selectors in styleElement.innerText (i.e.,
replace `.${theme}{}` with `.${CSS.escape(theme)}{}`) and change the cssRules
mapping to derive the theme from the original themes array by index (use the
same order as this.themes) rather than parsing selectorText; keep references to
styleElement, this.themes, UniwindCSSRule and ensure updateCSSVariables() will
receive the correct unescaped theme names.
🪄 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: 8aacfd5e-f8ed-4228-9d23-d38037de0794
📒 Files selected for processing (7)
packages/uniwind/src/common/utils.tspackages/uniwind/src/core/config/config.native.tspackages/uniwind/src/core/config/config.tspackages/uniwind/src/core/web/cssListener.tspackages/uniwind/src/hooks/useCSSVariable/useCSSVariable.tspackages/uniwind/tests/native/components/scoped-theme.test.tsxpackages/uniwind/tests/web/core/config.test.ts
|
🚀 This pull request is included in v1.6.3. See Release v1.6.3 for release notes. |
fixes #499
Summary by CodeRabbit
Release Notes
Bug Fixes
Tests