Skip to content

feat(preset-wind4): improve css variable usage in bracket syntax#5174

Merged
zyyv merged 3 commits intomainfrom
variableInBracket
Apr 8, 2026
Merged

feat(preset-wind4): improve css variable usage in bracket syntax#5174
zyyv merged 3 commits intomainfrom
variableInBracket

Conversation

@zyyv
Copy link
Copy Markdown
Member

@zyyv zyyv commented Apr 8, 2026

related to #4970, close #5151, close #5161

Copilot AI review requested due to automatic review settings April 8, 2026 06:11
@zyyv zyyv requested a review from antfu as a code owner April 8, 2026 06:11
@netlify
Copy link
Copy Markdown

netlify bot commented Apr 8, 2026

Deploy Preview for unocss ready!

Name Link
🔨 Latest commit faf2da3
🔍 Latest deploy log https://app.netlify.com/projects/unocss/deploys/69d5f19cc46d58000814bd14
😎 Deploy Preview https://deploy-preview-5174--unocss.netlify.app
📱 Preview on mobile
Toggle QR Code...

QR Code

Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify project configuration.

@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new bot commented Apr 8, 2026

Open in StackBlitz

commit: faf2da3

@zyyv zyyv merged commit 8930816 into main Apr 8, 2026
16 checks passed
@zyyv zyyv deleted the variableInBracket branch April 8, 2026 06:17
Copy link
Copy Markdown
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

This PR aims to improve preset-wind4 bracket syntax handling so CSS-variable-like tokens (e.g. --spacing(8), fallbacks, and theme(...)) are transformed more reliably inside arbitrary values (notably within calc(...)), addressing reported failures in bracket parsing.

Changes:

  • Add a new cssVarsRE and update bracket handling to rewrite --* tokens (and resolve theme(...)) inside bracket values.
  • Pass theme into the size rule’s value handler so bracket parsing can resolve theme functions/vars.
  • Update test expectations and golden CSS snapshots; adjust some language-server inlined dependency versions/order.

Reviewed changes

Copilot reviewed 6 out of 6 changed files in this pull request and generated 5 comments.

Show a summary per file
File Description
test/preset-wind4.test.ts Extends bracket-syntax test coverage and switches to asserting the default layer output.
test/assets/output/preset-wind4-targets.css Updates the golden CSS output for target fixtures after bracket parsing changes.
packages-presets/preset-wind4/src/utils/handlers/regex.ts Introduces cssVarsRE to detect --* tokens for rewriting.
packages-presets/preset-wind4/src/utils/handlers/handlers.ts Implements theme + CSS-var rewriting logic inside bracket parsing using the new regex.
packages-presets/preset-wind4/src/rules/size.ts Passes theme into the value handler chain to enable theme-aware bracket transforms.
packages-integrations/language-server/package.json Adjusts inlinedDependencies ordering/versions (incl. acorn, ufo).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment on lines +214 to 238
const matches = Array.from(base.matchAll(cssVarsRE))
for (const match of matches) {
const [full, varPaths, _value] = match

if (theme) {
const [key, ...paths] = varPaths.split('.')
const { val, varKey } = processThemeVariable(theme, key, paths, varPaths)

if (val != null) {
const cssVar = `--${varKey.replaceAll('.', '-')}`
// use theme value with multiplier
if (_value && !_value.startsWith(',')) {
base = base.replace(full, `calc(var(${cssVar}) * ${_value.slice(1, -1)})`)
}
// default value
else {
const fallback = _value?.slice(1)
base = base.replace(full, `var(${cssVar}${fallback ? `, ${fallback}` : ''})`)
}
continue
}
}

base = base.replace(full, `var(${full})`)
}
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The matches = Array.from(base.matchAll(cssVarsRE)) + base = base.replace(full, ...) loop is not safe when the same CSS var token occurs multiple times in the bracket value. String.prototype.replace only replaces the first occurrence, and after the first substitution the inserted var(--x) still contains the substring --x, so subsequent iterations can re-wrap the already-replaced occurrence (e.g. producing var(var(--x))) and leave later occurrences unchanged. Consider doing a single base.replace(cssVarsRE, (full, varPaths, value) => ...) callback (or index-based replacement) so each match is replaced exactly once at its original position.

Copilot uses AI. Check for mistakes.
Comment on lines +214 to +237
const matches = Array.from(base.matchAll(cssVarsRE))
for (const match of matches) {
const [full, varPaths, _value] = match

if (theme) {
const [key, ...paths] = varPaths.split('.')
const { val, varKey } = processThemeVariable(theme, key, paths, varPaths)

if (val != null) {
const cssVar = `--${varKey.replaceAll('.', '-')}`
// use theme value with multiplier
if (_value && !_value.startsWith(',')) {
base = base.replace(full, `calc(var(${cssVar}) * ${_value.slice(1, -1)})`)
}
// default value
else {
const fallback = _value?.slice(1)
base = base.replace(full, `var(${cssVar}${fallback ? `, ${fallback}` : ''})`)
}
continue
}
}

base = base.replace(full, `var(${full})`)
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This new CSS-var rewrite logic also transforms custom media conditions like (--cssvar) into (var(--cssvar)) (see updated targets snapshot). Custom media queries intentionally use the @media (--name) syntax (not var(--name)), so wrapping --cssvar here breaks valid CSS. Please add a guard to skip rewriting when the bracket content is a custom media query token (e.g. ^\(--[\w-]+\)$), or otherwise constrain cssVarsRE usage to contexts where --foo is actually a CSS custom property reference.

Suggested change
const matches = Array.from(base.matchAll(cssVarsRE))
for (const match of matches) {
const [full, varPaths, _value] = match
if (theme) {
const [key, ...paths] = varPaths.split('.')
const { val, varKey } = processThemeVariable(theme, key, paths, varPaths)
if (val != null) {
const cssVar = `--${varKey.replaceAll('.', '-')}`
// use theme value with multiplier
if (_value && !_value.startsWith(',')) {
base = base.replace(full, `calc(var(${cssVar}) * ${_value.slice(1, -1)})`)
}
// default value
else {
const fallback = _value?.slice(1)
base = base.replace(full, `var(${cssVar}${fallback ? `, ${fallback}` : ''})`)
}
continue
}
}
base = base.replace(full, `var(${full})`)
const isCustomMediaQueryToken = /^\(--[\w-]+\)$/.test(base)
if (!isCustomMediaQueryToken) {
const matches = Array.from(base.matchAll(cssVarsRE))
for (const match of matches) {
const [full, varPaths, _value] = match
if (theme) {
const [key, ...paths] = varPaths.split('.')
const { val, varKey } = processThemeVariable(theme, key, paths, varPaths)
if (val != null) {
const cssVar = `--${varKey.replaceAll('.', '-')}`
// use theme value with multiplier
if (_value && !_value.startsWith(',')) {
base = base.replace(full, `calc(var(${cssVar}) * ${_value.slice(1, -1)})`)
}
// default value
else {
const fallback = _value?.slice(1)
base = base.replace(full, `var(${cssVar}${fallback ? `, ${fallback}` : ''})`)
}
continue
}
}
base = base.replace(full, `var(${full})`)
}

Copilot uses AI. Check for mistakes.
Comment on lines 724 to 742
'text-[--colors.blue,#000]',
'text-[--colors.red.200,#fff]',
'[--foo:--bar(8)]',
`w-[calc(var(--sidebar-width-icon)+--spacing(8))]`,
`w-[--sidebar-width-icon+--spacing(8)+2px)+var(--foo)+theme(spacing.sm)]`,
]

const { css } = await uno.generate(cases)
const { getLayer } = await uno.generate(cases)

expect(css).toMatchInlineSnapshot(`
"/* layer: properties */
@supports ((-webkit-hyphens: none) and (not (margin-trim: inline))) or ((-moz-orient: inline) and (not (color:rgb(from red r g b)))){*, ::before, ::after, ::backdrop{--un-text-opacity:100%;}}
@property --un-text-opacity{syntax:"<percentage>";inherits:false;initial-value:100%;}
/* layer: theme */
:root, :host { --spacing: 0.25rem; --spacing-sm: 0.875rem; --colors-blue-DEFAULT: oklch(70.7% 0.165 254.624); --colors-red-200: oklch(88.5% 0.062 18.334); }
/* layer: default */
expect(getLayer('default')).toMatchInlineSnapshot(`
"/* layer: default */
.text-\\[--colors\\.blue\\,\\#000\\]{color:color-mix(in oklab, var(--colors-blue-DEFAULT, #000) var(--un-text-opacity), transparent);}
.text-\\[--colors\\.red\\.200\\,\\#fff\\]{color:color-mix(in oklab, var(--colors-red-200, #fff) var(--un-text-opacity), transparent);}
.m-\\[--spacing\\(2\\)\\]{margin:calc(var(--spacing) * 2);}
.m-\\[--spacing\\]{margin:var(--spacing);}
.px-\\[--spacing\\.sm\\(2\\.5\\)\\]{padding-inline:calc(var(--spacing-sm) * 2.5);}
.w-\\[--sidebar-width-icon\\+--spacing\\(8\\)\\+2px\\)\\+var\\(--foo\\)\\+theme\\(spacing\\.sm\\)\\]{width:var(--sidebar-width-icon)+calc(var(--spacing) * 8) + 2px) + var(--foo) + 0.875rem;}
.w-\\[calc\\(var\\(--sidebar-width-icon\\)\\+--spacing\\(8\\)\\)\\]{width:calc(var(--sidebar-width-icon) + calc(var(--spacing) * 8));}
.\\[--foo\\:--bar\\(8\\)\\]{--foo:calc(var(--bar) * 8);}"
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The added test case string w-[--sidebar-width-icon+--spacing(8)+2px)+var(--foo)+theme(spacing.sm)] has unbalanced parentheses and the expected CSS output contains arithmetic outside of a calc(...) (e.g. width:var(--sidebar-width-icon)+...), which is not valid CSS for width. If the intent is to verify nested replacements inside expressions, the input should likely be a valid calc(...) expression (and the snapshot should assert a valid width: calc(...) value).

Copilot uses AI. Check for mistakes.
.noscript\:text-red-500,
.scripting-none\:text-red-500{color:color-mix(in srgb, var(--colors-red-500) var(--un-text-opacity), transparent) /* oklch(63.7% 0.237 25.331) */;}
}
@media (var(--cssvar)){
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The snapshot update changes @media (--cssvar) (custom media query) to @media (var(--cssvar)), which is not valid media query syntax and would break custom media usage. This looks like an unintended consequence of the new --* => var(--*) rewrite in bracket parsing; it would be better to keep the custom media form in the generated output (and adjust the handler accordingly) rather than updating the golden file to match an invalid construct.

Suggested change
@media (var(--cssvar)){
@media (--cssvar){

Copilot uses AI. Check for mistakes.
Comment on lines +18 to +19
export const cssVarsRE = /(?<!var\()--([\w.-]+)(\([^)]+\)|,[#.\s\w]+)?/g
// ^ There may not have been any other special cases matched; this needs further improvement.
Copy link

Copilot AI Apr 8, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cssVarsRE currently matches any --foo not preceded by var(, including non-variable contexts (notably custom media queries like (--cssvar)). Given this regex is now used to auto-wrap tokens with var(...), it likely needs additional constraints (or call-site guards) to avoid rewriting syntaxes where --name is not a CSS custom property reference.

Suggested change
export const cssVarsRE = /(?<!var\()--([\w.-]+)(\([^)]+\)|,[#.\s\w]+)?/g
// ^ There may not have been any other special cases matched; this needs further improvement.
export const cssVarsRE = /(?<!var\()(?<![\w(-])--([\w.-]+)(\([^)]+\)|,[#.\s\w]+)?/g
// ^ Avoid matching non-reference syntaxes such as custom media `(--foo)`.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

presetWind4 fails to correctly handle certain usages of --space()

2 participants