Skip to content
New issue

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

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

Already on GitHub? # to your account

CSS transforms can break nested selectors #3620

Closed
nstepien opened this issue Jan 31, 2024 · 7 comments
Closed

CSS transforms can break nested selectors #3620

nstepien opened this issue Jan 31, 2024 · 7 comments
Labels

Comments

@nstepien
Copy link

nstepien commented Jan 31, 2024

https://esbuild.github.io/try/#YgAwLjIwLjAALS1zdXBwb3J0ZWQ6bmVzdGluZz1mYWxzZQBlAGVudHJ5LmNzcwBkaXYgewogID4gaW5wdXQsCiAgPiBsYWJlbCA+IGlucHV0IHsKICAgIG1hcmdpbjogMWVtOwogIH0KfQ

Esbuild transforms this:

div {
  > input,
  > label > input {

into this:

div > :is(input, label > input) {

Looks good right? No!
The following

div > :is(label > input)

actually means "select an input that is an immediate child of label AND is also an immediate child of div", which is just not possible.

I use esbuild through Vite, but I couldn't figure out a way to avoid this issue without disabling build.cssMinify.
Is there an esbuild option to never transform nested selectors?

In Vite I've tried

  build: {
    target: ['chrome121'],
  },
  esbuild: {
    minifyIdentifiers: false,
    minifySyntax: false,
    minifyWhitespace: false,
    supported: {
      nesting: true
    }
  },

without success, the broken selectors are still using :is().

@nstepien nstepien changed the title CSS minification can break nested selectors CSS transforms can break nested selectors Jan 31, 2024
@nstepien
Copy link
Author

nstepien commented Jan 31, 2024

I found that build.cssTarget: [] prevents nested selectors from being transformed!

Edit: updating all our dependencies also made esbuild stop transforming nested selectors without needing a workaround.

@evanw evanw added the css label May 25, 2024
@evanw
Copy link
Owner

evanw commented May 25, 2024

https://esbuild.github.io/try/#YgAwLjIwLjAALS1zdXBwb3J0ZWQ6bmVzdGluZz1mYWxzZQBlAGVudHJ5LmNzcwA6Oi13ZWJraXQtc2Nyb2xsYmFyLXRodW1iIHsKICAmOndpbmRvdy1pbmFjdGl2ZSB7CiAgICBiYWNrZ3JvdW5kOiByZWQ7CiAgfQogICY6aG92ZXIgewogICAgYmFja2dyb3VuZDogYmx1ZTsKICB9IAp9

This also looks broken.

I don't think this case is supposed to do anything because the nesting selector cannot represent pseudo-elements. I believe this is because the behavior of the nesting selector is defined to be the same as the behavior of :is(), which also doesn't apply to pseudo-elements (see also w3c/csswg-drafts#7433). Here's a full example:

<!DOCTYPE html>
<style>
  section {
    width: 300px;
    height: 300px;
    overflow-y: scroll;
    display: inline-block;
  }
  div {
    height: 900px;
  }
  section::-webkit-scrollbar {
    background-color: yellow;
  }

  /* Styling the scrollbar with non-nested CSS works */
  #a::-webkit-scrollbar-thumb {
    background: green;
  }
  #a::-webkit-scrollbar-thumb:window-inactive {
    background: red;
  }
  #a::-webkit-scrollbar-thumb:hover {
    background: blue;
  }

  /* Attempting to style the scrollbar with nested CSS doesn't work */
  #b::-webkit-scrollbar-thumb {
    & {
      background: green;
    }
    &:window-inactive {
      background: red;
    }
    &:hover {
      background: blue;
    }
  }

</style>
<section id="a"><div></div></section>
<section id="b"><div></div></section>

If you try that in a browser, the example on the left that doesn't use CSS nesting works but the example on the right that uses CSS nesting doesn't work. So it doesn't necessarily seem like a bug if esbuild is transforming nested CSS into non-nested CSS that has the same effect (which is no effect, since the input CSS is meaningless).

@tim-we
Copy link

tim-we commented Aug 6, 2024

@evanw what about the first post though?

Esbuild transforms this:

div {
  > input,
  > label > input {

into this:

div > :is(input, label > input) {

Looks good right? No! The following

div > :is(label > input)

actually means "select an input that is an immediate child of label AND is also an immediate child of div", which is just not possible.

This is a bug and I have run into it because unfortunately we have to support Chrome 117 in a project I'm working on. This means CSS nesting is not supported and esbuild will transform it causing some rules not to apply anymore, due to this bug.

Are you planning on fixing this? Asking because its been open for a while. If I find the time I will look into this a bit more and try to find the code causing this. Would you accept a PR for this?

In the meantime my workaround is manually expanding those CSS rules. I use this RegExp to find occurrences in the esbuild output: > :is\(.+ > .+\)

@tim-we
Copy link

tim-we commented Aug 11, 2024

I think this:

.parent {
  > .a,
  > .b1 > .b2 {
    /* ... */
  }
}

should be transformed into this instead:

.parent > .a,
.parent > .b1 > .b2 {
    /* ... */
 }

as I don't think there is a way to make this more compact with the :is() pseudo class (correct me if I'm wrong).
The transformation happens in the method multipleComplexSelectorsToSingleComplexSelector which is called here:

// Avoid generating ":is" if it's not supported
if p.options.unsupportedCSSFeatures.Has(compat.IsPseudoClass) && len(r.Selectors) > 1 {
canUseGroupDescendantCombinator = false
canUseGroupSubSelector = false
}
// Try to apply simplifications for shorter output
if canUseGroupDescendantCombinator {
// "& a, & b {}" => "& :is(a, b) {}"
// "& > a, & > b {}" => "& > :is(a, b) {}"
nestingSelectorLoc := r.Selectors[0].Selectors[0].NestingSelectorLoc
for i := range r.Selectors {
sel := &r.Selectors[i]
sel.Selectors = sel.Selectors[1:]
}
merged := p.multipleComplexSelectorsToSingleComplexSelector(r.Selectors)(rule.Loc)
merged.Selectors = append([]css_ast.CompoundSelector{{NestingSelectorLoc: nestingSelectorLoc}}, merged.Selectors...)
r.Selectors = []css_ast.ComplexSelector{merged}
} else if canUseGroupSubSelector {
// "&a, &b {}" => "&:is(a, b) {}"
// "> &a, > &b {}" => "> &:is(a, b) {}"
nestingSelectorLoc := r.Selectors[0].Selectors[0].NestingSelectorLoc
for i := range r.Selectors {
sel := &r.Selectors[i]
sel.Selectors[0].NestingSelectorLoc = ast.Index32{}
}
merged := p.multipleComplexSelectorsToSingleComplexSelector(r.Selectors)(rule.Loc)
merged.Selectors[0].NestingSelectorLoc = nestingSelectorLoc
r.Selectors = []css_ast.ComplexSelector{merged}
}

So I think part of the fix would be to detect the > combinator in the nested selector and avoid calling multipleComplexSelectorsToSingleComplexSelector.

@Teisi
Copy link

Teisi commented Aug 26, 2024

Same problem here.

For illustration:
example

https://jsfiddle.net/yn3ecod0/

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
Projects
None yet
Development

No branches or pull requests

4 participants