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.

Sign up for GitHub

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

Jump to bottom
Closed
wants to merge 1 commit into from
Closed

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

wants to merge 1 commit into from

Conversation

tim-we
Copy link

@tim-we tim-we commented Jan 13, 2025 •
edited
Loading

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
@BrewTestBot BrewTestBot mentioned this pull request Feb 8, 2025
Merged
@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 •
edited
Loading

@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

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@tim-we @sanoojes