Skip to content

Nested @media transformed incorrectly for pseudo elements #4164

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
Open
EECOLOR opened this issue Apr 30, 2025 · 3 comments · May be fixed by #4177
Open

Nested @media transformed incorrectly for pseudo elements #4164

EECOLOR opened this issue Apr 30, 2025 · 3 comments · May be fixed by #4177

Comments

@EECOLOR
Copy link

EECOLOR commented Apr 30, 2025

I have the following css:

.this-works1.test {
  background-color: hotpink;

  @media screen and (min-width: 768px) {
    background-color: yellow;
  }
}

.this-works2 {
  &.test {
    background-color: hotpink;

    @media screen and (min-width: 768px) {
      background-color: yellow;
    }
  }
}

.this-does-not-work1::before {
  background-color: hotpink;

  @media screen and (min-width: 768px) {
    background-color: yellow;
  }
}

.this-does-not-work2 {

  &::before {
    background-color: hotpink;

    @media screen and (min-width: 768px) {
      background-color: yellow;
    }
  }
}

I am using the following code to transform it:

const { code, map, warnings } = await esbuild.transform(cssSource, {
        sourcefile: `${prefix}.css`,
        sourcemap: 'external',
        loader: 'local-css',
        target: ['chrome94', 'opera79', 'edge94', 'firefox92', 'safari15']
      })

The result is the following:

.prefix_this-works1.prefix_test {
  background-color: hotpink;
}
@media screen and (min-width: 768px) {
  .prefix_this-works1.prefix_test {
    background-color: yellow;
  }
}
.prefix_this-works2.prefix_test {
  background-color: hotpink;
}
@media screen and (min-width: 768px) {
  .prefix_this-works2.prefix_test {
    background-color: yellow;
  }
}
.prefix_this-does-not-work1::before {
  background-color: hotpink;
}
@media screen and (min-width: 768px) {
   {
    background-color: yellow;
  }
}
.prefix_this-does-not-work2::before {
  background-color: hotpink;
}
@media screen and (min-width: 768px) {
   {
    background-color: yellow;
  }
}

As you can see it created these rules:

@media screen and (min-width: 768px) {
   {
    background-color: yellow;
  }
}

Here the selector inside of @media is missing.

I was expecting something more like this:

@media screen and (min-width: 768px) {
  .prefix_this-does-not-work2::before {
    background-color: yellow;
  }
}

I am not sure if this is an easy fix. I did not dig into the source code too much (and have no experience in Go). On top of that I am not sure if we can simply switch the target to browsers that support nesting.

In any case, if it is an easy fix all good. If it is not easy, please give me a few pointers that would allow me to do the work and submit a pull request (if we need to actually support older browsers).
@EECOLOR
Copy link
Author

EECOLOR commented May 6, 2025

Initially it seems that disabling the transformation (by using the versions listed here: https://developer.mozilla.org/en-US/docs/Web/CSS/Nesting_selector#browser_compatibility) would work for us. We however found out that these numbers are incorrect (at least for Safari). The 17.3 version does not handle nesting correctly in some cases.

@EECOLOR
Copy link
Author

EECOLOR commented May 6, 2025

@evanw Would you accept a pull request for this? If so, if you have any pointers on where to start, that would greatly help :-D

@sapphi-red sapphi-red mentioned this issue May 7, 2025
Open
7 tasks
@EECOLOR
Copy link
Author

EECOLOR commented May 7, 2025 •
edited
Loading

Removing the if-statement in css_nesting.go solves this error, but introduces others:

// if !sel.UsesPseudoElement() {
  parentSelectors = append(parentSelectors, css_ast.ComplexSelector{Selectors: substituted})
// }

It's great to see this is covered by the tests. I'll have to trace the flow of the application to see if we can easily distinguish between the different scenario's.

@EECOLOR EECOLOR linked a pull request May 8, 2025 that will close this issue
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Allow media queries nested in pseudo element selectors EECOLOR/esbuild
1 participant
@EECOLOR