Fix theme() in JS plugins returning unresolved object instead of DEFAULT value#20299
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (3)
✅ Files skipped from review due to trivial changes (1)
🚧 Files skipped from review as they are similar to previous changes (2)
WalkthroughThis change updates Changes
Sequence Diagram(s)sequenceDiagram
participant Plugin
participant ThemeFn
participant CssResolver
Plugin->>ThemeFn: theme('colors.foo')
ThemeFn->>CssResolver: resolve cssValue
CssResolver-->>ThemeFn: synthetic object with DEFAULT
ThemeFn->>ThemeFn: return cssValue.DEFAULT
ThemeFn-->>Plugin: resolved color value
Related PRs: None specified. Suggested labels: bug, compat Suggested reviewers: None specified. 🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
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.
🧹 Nitpick comments (1)
CHANGELOG.md (1)
14-14: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueMissing PR reference link.
Sibling entries (lines 12-13) include a
([#XXXXX](...))PR link; this new entry doesn't. Worth adding for consistency once the PR number is known.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository UI
Review profile: CHILL
Plan: Pro
Run ID: 8e43882d-33df-4c07-9ea5-dba59ca9c2e7
📒 Files selected for processing (3)
CHANGELOG.mdpackages/tailwindcss/src/compat/plugin-api.test.tspackages/tailwindcss/src/compat/plugin-functions.ts
Confidence Score: 5/5This looks safe to merge.
Reviews (3): Last reviewed commit: "update CHANGELOG" | Re-trigger Greptile |
0d62152 to
2a38bd7
Compare
RobinMalfait
left a comment
There was a problem hiding this comment.
Thanks! Made a few small changes to the comment / changelog entry.
Summary
When a CSS theme key defined via
@theme(or a JS config'sthemeobject) shares a dash-separated prefix with a sibling key — e.g.--color-fooand--color-foo-bar— callingtheme('colors.foo')from inside a JS plugin (addUtilities,addComponents, etc.) does not resolve to thefoovalue. Instead it returns an internal disambiguation object shaped like{ DEFAULT: 'red', bar: 'blue', __CSS_VALUES__: {...} }, because there's no way to tell from CSS custom property names alone whetherfoo-baris a sibling key or a nested sub-key offoo.This same ambiguity was already fixed for the CSS-embedded
theme()function in #19097 (which unwraps to theDEFAULTkey when present), and the changelog entry for that PR states it fixes this "in JS configs and plugins" — but the fix only touchedapply-compat-hooks.ts'sresolveThemeValue, notcreateThemeFn'sthemefunction that's exposed directly to plugins inplugin-functions.ts. This PR closes that gap by applying the same DEFAULT-unwrapping there.Without this fix, passing the raw object into
addUtilities(a very natural thing to do, since a plugin author expects a string) produces broken CSS — the reservedDEFAULTkey gets mangled into a garbage property name (-d-e-f-a-u-l-t) by the kebab-case conversion, and the internal__CSS_VALUES__bookkeeping leaks into the generated stylesheet.Minimal reproduction
Before:
After:
Test plan
packages/tailwindcss/src/compat/plugin-api.test.ts("theme() resolves the DEFAULT value when a bare CSS theme key shares a prefix with a sibling key")pnpm --filter tailwindcss exec vitest runthat this test fails with the exact broken output shown above when the fix is reverted, and passes once it's appliedtailwindcsspackage test suite (pnpm --filter tailwindcss exec vitest run) — 4684 tests passing, no regressionsnpx prettier --check