镜像站点 · 本页由第三方 GitHub 只读镜像提供,非 GitHub 官方站点,不接受任何登录或凭据输入。前往 github.com
Skip to content

Do not extract candidates containing JS string interpolation pattern ${ - #17142

Merged
RobinMalfait merged 3 commits into
mainfrom
fix/issue-17054
Mar 12, 2025
Merged

RobinMalfait merged 3 commits into
mainfrom
fix/issue-17054

Conversation

@RobinMalfait

@RobinMalfait RobinMalfait commented Mar 12, 2025 •

Copy link
Copy Markdown
Contributor

This PR fixes an issue where often people run into issues where they try to use string interpolation and it doesn't work. Even worse, it could result in crashes because we will actually generate CSS. This fix only filters out candidates with a pattern like ${. If this occurs in a string position it is fine.

Another solution would be to add a pre processor for JS/TS (and all thousand file extension combinations) but the problem is that you can also write JS in HTML files so we would have to pre process HTML as well which would not be ideal.

Test plan

  1. Added tests to prove this works in arbitrary values, arbitrary variables in both utilities and variants.
  2. Existing tests pass.
  3. Some screenshots with before / after situations:

Given this input:

let color = '#0088cc';
let opacity = 0.8;
let name = 'variable-name';
let classes = [
  // Arbitrary Properties
  `[color:${color}]`
  `[${property}:value]`,
  `[--img:url('https://example.com?q=${name}')]`, // WONT WORK BUT VALID CSS

  // Arbitrary Values
  `bg-[${color}]`,

  // Arbitrary Variables
  `bg-(--my-${color})`,
  `bg-(--my-color,${color})`,

  // Arbitrary Modifier
  `bg-red-500/[${opacity}]`,
  `bg-red-500/(--my-${name})`,
  `bg-red-500/(--my-opacity,${opacity})`,

  // Arbitrary Variant
  `data-[state=${name}]:flex`,
  `supports-(--my-${name}):flex`,
  `[@media(width>=${value})]:flex`,
];

This is the result:

Before After
image image

Fixes: #17054
Fixes: #15853

If this occurs inside of strings then it's fine but everywhere else it's
thrown out.

Technically you can set it in the value of a CSS variable, but I think
guarding against that is only needed if somebody actually runs into
problems with this.
@RobinMalfait
RobinMalfait marked this pull request as ready for review March 12, 2025 11:04
@RobinMalfait
RobinMalfait requested a review from a team as a code owner March 12, 2025 11:04

@philipp-spiess philipp-spiess left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Nice one!

@philipp-spiess

Copy link
Copy Markdown
Contributor

@RobinMalfait Here's another issue that is resolved by this I think: #15853

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.

Bug: Detecting classes in source files causes crash v4. build fails with dynamically created classnames + at-rules

3 participants