evanw / esbuild

An extremely fast bundler for the web
https://esbuild.github.io/
MIT License
37.92k stars 1.13k forks source link

CSS transforms can break nested selectors #3620

Open nstepien opened 7 months ago

nstepien commented 7 months ago

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 commented 7 months ago

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.

nstepien commented 7 months ago

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

This also looks broken.

nstepien commented 7 months ago

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

This looks broken too, a > is missing in :has()

evanw commented 3 months ago

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 https://github.com/w3c/csswg-drafts/issues/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 commented 1 month ago

@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 commented 1 month ago

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: https://github.com/evanw/esbuild/blob/9c13ae1f06dfa909eb4a53882e3b7e4216a503fe/internal/css_parser/css_nesting.go#L162-L191

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

Teisi commented 2 weeks ago

Same problem here.

For illustration: example

https://jsfiddle.net/yn3ecod0/