[Bug] : CSSNano has problems with postcss-nested

[Chris2011]

Describe the bug
I think it is a bug but at the end I can understand, that this is a missing feature maybe.

I have a piece of code, in which I have two properties with the same value margin-top: 60px in a p and in an id selector. CSSNano can handle this and put this together to #app,p{margin-top: 60px}. It will remove the separated p and the property inside of the #app selector and merge them together.

When I use the plugin postcss-nested and I put the div within the #app selector, cssnano will not remove the duplicated value.
It will just put the p on the same level, which is correct #app p{margin-top:60px} but it will leave the margin-top also inside of the app. For me, I expect to have this #app,#app p{margin-top:60px}. Everything what I described, you can find here: https://github.com/Chris2011/cssnano-postcss-nested-problem

To Reproduce
Steps to reproduce the behavior:

1. Clone repo: https://github.com/Chris2011/cssnano-postcss-nested-problem
2. Do a yarn
3. Run yarn build
4. Go to the dist\assets folder and open the css file.
5. Result is: #app{margin-top:60px}#app p{margin-top:60px}

Expected behavior
Result should be: #app,#app p{margin-top:60px}

Desktop (please complete the following information):

• CSSNANO Version [e.g. 22]: 4.1.10
• Plugins/presets versions: plugin: postcss-nested - 5.0.3, preset: default
• Add the output of npx envinfo && npm ls cssnano

  System:
    OS: Windows 10 10.0.19042
    CPU: (8) x64 Intel(R) Core(TM) i7-1065G7 CPU @ 1.30GHz
    Memory: 2.58 GB / 15.60 GB
  Binaries:
    Node: 12.16.3 - C:\Program Files\nodejs\node.EXE
    Yarn: 1.22.10 - ~\AppData\Roaming\npm\yarn.CMD
    npm: 7.5.2 - C:\Program Files\nodejs\npm.CMD
  Managers:
    Gradle: 6.5 - C:\Gradle\gradle-6.5\bin\gradle.BAT
    pip3: 19.2.3 - C:\Python38\Scripts\pip3.EXE
  Utilities:
    Git: 2.30.1.
  Virtualization:
    Docker: 20.10.2 - C:\ProgramData\DockerDesktop\version-bin\docker.EXE
  IDEs:
    VSCode: 1.53.2 - C:\Users\Chris\AppData\Local\Programs\Microsoft VS Code\bin\code.CMD
    Visual Studio: 16.8.30711.63 (Visual Studio Community 2019)
  Languages:
    Bash: 4.4.20 - C:\Windows\System32\bash.EXE
    Go: 1.14.4 - C:\Go\bin\go.EXE
    Python: 3.8.2
  Databases:
    SQLite: 3.32.2 - C:\Users\Chris\AppData\Local\Android\Sdk\platform-tools\sqlite3.EXE
  Browsers:
    Chrome: 88.0.4324.182
    Edge: Spartan (44.19041.423.0), Chromium (88.0.705.74)
    Internet Explorer: 11.0.19041.1

cssnano-postcss-nested-problem@0.0.0 C:\Projekte\Others\vite-project
-- cssnano@4.1.10

[ludofischer]

Hi, thank you for the detailed reproduction! I have managed to reproduce the bug and I also believe it is fixed in the cssnano 5.0.0 release candidate. Could you tell me if the release candidate fixes the issue on your end too?
To test, set the cssnano version in package.json to

"cssnano": "^5.0.0-rc.1",

[Chris2011]

Hey, great to hear it. I tested it, and it seems to work. thx :)

[Chris2011]

Unfortunately, I have a more detailed example:

#app {
  font-family: Avenir, Helvetica, Arial, sans-serif;
  -webkit-font-smoothing: antialiased;
  -moz-osx-font-smoothing: grayscale;
  text-align: center;
  color: #2c3e50;
  margin-top: 60px;
  transform: scale(0.5);
  transition: all 400ms ease-in;

  p {
    margin-top: 60px;
    width: 100%;
    height: 50px;
    display: block;
    background-color: yellow;
    border: 1px solid black;

    &.t {
      color: green;
    }
  }
}

div {
  margin-top: 60px;
}

and the output ignores the div it is still extra at the end of the file, but I expect that it will be merged togehter with the other selectors with margin-top: 60px;

Output:

#app {
  -webkit-font-smoothing: antialiased;
  -moz-osx-font-smoothing: grayscale;
  color: #2c3e50;
  font-family: Avenir, Helvetica, Arial, sans-serif;
  text-align: center;
  transform: scale(0.5);
  transition: all 0.4s ease-in;
}
#app,
#app p {
  margin-top: 60px;
}
#app p {
  background-color: #ff0;
  border: 1px solid #000;
  display: block;
  height: 50px;
  width: 100%;
}
div {
  margin-top: 60px;
}

[ludofischer]

Interesting. My intuition would be that cssnano runs before the rules get unnested, so it does not 'see' them, but that seems unlikely as the latest cssnano uses OnceExit() and should run after postcss-nested which uses Rule

[Chris2011]

Yeah I thought also. I thought I need to change the order, when the stuff should run, but this is not needed as you said?

[Chris2011]

hey, is there anything todo where I can help here?

[ludofischer]

I don't think there is anything to do except diving inside the postcss-merge-rules code and trying to understand where it goes wrong. This issue is not the worst either, there are multiple cases where merging the rules results in incorrect output: #1006, #701, #1000, #999 So I think that plugin is in need of a close look, but I am not familiar with the algorithm it uses.

[Chris2011]

Hey @ludofischer thx for the info. That's is all what I wanted to know. Thx :)

[Chris2011]

If I have time, I can also have a look into it. Maybe adding console.logs into it to see what comes first :D

[Chris2011]

Didn't check the new version yet, will check this first and will let you know :)

[ludofischer]

Didn't check the new version yet, will check this first and will let you know :)

It would be wonderful if you could help improve the situation, a few people already tried but so far I have not heard back from any of them ;-). Last time I checked there are similar issues open in other minifiers too. Merging rules and declarations often yields small size improvements, so it is often handier to disable the optimization completely. For example esbuild managed to become a fairly competitive CSS minifier in a short time, part of the secret is that esbuild just does not attempt merging rules (last time I have checked).

[Chris2011]

Atm I have some other projects todo, that's why I can live with the result for now. I also checked the newest versions and unfortunaltey, same problem. Will check what I can and will let you know when I have time. Maybe we should switch to discussions. Otherwise we can stay here :)

[Chris2011]

An alternative to cssnano is csso. So I tried this code which is the end result of cssnano:

#app {
  -webkit-font-smoothing: antialiased;
  -moz-osx-font-smoothing: grayscale;
  color: #2c3e50;
  font-family: Avenir, Helvetica, Arial, sans-serif;
  text-align: center;
  transform: scale(0.5);
  transition: all 0.4s ease-in;
}
#app,
#app p {
  margin-top: 60px;
}
#app p {
  background-color: #ff0;
  border: 1px solid #000;
  display: block;
  height: 50px;
  width: 100%;
}
#app p.t {
  color: green;
}
div {
  margin-top: 60px;
}

and the output of csso (http://css.github.io/csso/csso.html -beautified) is:

#app {
    -webkit-font-smoothing: antialiased;
    -moz-osx-font-smoothing: grayscale;
    color: #2c3e50;
    font-family: Avenir,Helvetica,Arial,sans-serif;
    text-align: center;
    transform: scale(.5);
    transition: all .4s ease-in
}

#app,#app p,div {
    margin-top: 60px
}

#app p {
    background-color: #ff0;
    border: 1px solid #000;
    display: block;
    height: 50px;
    width: 100%
}

#app p.t {
    color: green
}

as you can see, this is my expected output. When I Take the input code and try it on the cssnano playground, output will be the same as the input. So maybe a look into csso could be good? 😉 🤷

[ludofischer]

This is again caused by the decision in postcss-merge-rules to only merge adjacent rules to avoid cascade problems. I think it does not cause incorrect output, just a missing optimization.

[Chris2011]

I understand the thing. I mean this is why I created this ticket. At the end maybe this is a missing feature so removing the bug is fine for me. I just tested csso and it handles it correct. So I thought it was a bug at first time and I expect cssnano to optimice this too.