Skip to content

(CSS) Fix nesting lowering with complex sub selectors #4037

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from

Conversation

tim-we
Copy link

@tim-we tim-we commented Jan 13, 2025

This PR fixes the issue described in #3620.

When nesting is lowered (for example when the target is Chrome 117) the current version of esbuild will transform this:

.parent {
  > .a,
  > .b1 > .b2 {
    color: red;
  }
}

into this (playground link)

.parent > :is(.a, .b1 > .b2) {
  color: red;
}

which is wrong because .parent > :is(.b1 > .b2) is semantically different from .parent > .b1 > .b2.

With the change in this PR the output will instead be this:

.parent > .a,
.parent > .b1 > .b2 {
  color: red;
}

I have added a test for this as well. The fix is in the first pass of the lowerNestingInRuleWithContext method. When a nested complex selector is sufficiently complex (has another combinator) the transformation will be disabled.

@tim-we tim-we changed the title Fix nesting lowering with complex sub selectors (CSS) Fix nesting lowering with complex sub selectors Jan 13, 2025
@evanw evanw closed this in 31e7b4d Feb 4, 2025
@sanoojes
Copy link

I think its not fixed

config

{
  bundle: true,
  platform: "browser",
  globalName: "Lucid",
  sourcemap: false,
  minify: true,
  entryPoints: [ "styles/app.css" ],
  outfile: "src/user.css",
  supported: { nesting: false },
  plugins: []
}

After build
image

Source
image

@tim-we
Copy link
Author

tim-we commented Mar 15, 2025

Hello @sanoojes, this is PR that did not get accepted, the issue was solved in a different way. If you think this is not fixed either comment on #3620 or create a new issue.
However looking at the code you posted I don't understand the issue, the transformation looks ok? Please state clearly what you think is wrong, it is very much not obvious from those screenshots.

@sanoojes
Copy link

image

ohh sorry to comment here
also the transformation i want is something like in the screenshot above as the sass complies
i do not want that :is() selectors any way to fix it ?
i comment on issue #3620 when am free
i will provide details and issues soon as i get free

also apologies if i made mistakes
English its not my first language

@tim-we
Copy link
Author

tim-we commented Mar 16, 2025

@sanoojes this PR and the issue #3620 is about the correctness of the transformation. If I understand you correctly you just don't want any :is() pseudo-class function in your output. This is unrelated to this PR and the issue.

I wonder why you want that, as :is() seems to be supported by all major browser for more than 10 years. To disable that transformation you just have to pick a very old browser as a target, like Firefox 77 or Chrome 87 (those are from 2011).

If you change the browser version you can see the effect:
https://esbuild.github.io/try/#dAAwLjI1LjEALS10YXJnZXQ9ZmlyZWZveDc3IC0tbG9hZGVyPWNzcwAuYSwgLmIgewogIC5jIHsKICAgIGNvbG9yOiByZWQ7CiAgfQp9

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.

2 participants