Comments cause parent selector output repetition

[argyleink]

Bug description

Source CSS

progress {
  --_track: hsl(228 100% 90%);
  --_track-size: min(10px, 1ex);
  --_progress: hsl(228 100% 50%);
  --_radius: 1e3px;
  --_indeterminate-track: linear-gradient(to right,
    var(--_track) 45%,
    var(--_progress) 0%,
    var(--_progress) 55%,
    var(--_track) 0%
  );
  --_indeterminate-track-size: 225% 100%;
  --_indeterminate-track-animation: progress-loading 2s infinite ease;
  
  /*  reset  */
  appearance: none;
  border: none;
  
  /*  custom style  */
  position: relative;
  height: var(--_track-size);
  border-radius: var(--_radius);
  overflow: hidden;

  @media (prefers-color-scheme: dark) {
    --_track: hsl(228 20% 30%);
    --_progress: hsl(228 100% 75%);
  }

  &:focus-visible {
    outline-color: var(--_progress);
  }
  
  /*  Safari/Chromium  */
  &[value]::-webkit-progress-bar {
    background-color: var(--_track);
  }
  
  &[value]::-webkit-progress-value {
    background-color: var(--_progress);
    transition: inline-size .25s ease-out;
  }
  
  /*  Firefox  */
  &[value]::-moz-progress-bar {
    background-color: var(--_progress);
  }
  
  /*  indeterminate  */
  &:indeterminate::after {
    content: "";
    inset: 0;
    position: absolute;
    background: var(--_indeterminate-track);
    background-size: var(--_indeterminate-track-size);
    background-position: right; 
    animation: var(--_indeterminate-track-animation);
  }
  
  /*  indeterminate Safari  */
  &:indeterminate::-webkit-progress-bar {
    background: var(--_indeterminate-track);
    background-size: var(--_indeterminate-track-size);
    background-position: right; 
    animation: var(--_indeterminate-track-animation);
  }
  
  /*  indeterminate Firefox  */
  &:indeterminate::-moz-progress-bar {
    background: var(--_indeterminate-track);
    background-size: var(--_indeterminate-track-size);
    background-position: right; 
    animation: var(--_indeterminate-track-animation);
  }
  
  /*  complete  */
  &:not([max])[value="1"]::before,
  &[max="100"][value="100"]::before {
    content: "✓";
    
    position: absolute;
    inset-block: 0;
    inset-inline: auto 0;
    display: flex;
    align-items: center;
    padding-inline-end: max(calc(var(--_track-size) / 4), 3px);

    color: white;
    font-size: calc(var(--_track-size) / 1.25);
  }
}

Expected CSS

progress {
  --_track: hsl(228 100% 90%);
  --_track-size: min(10px, 1ex);
  --_progress: hsl(228 100% 50%);
  --_radius: 1e3px;
  --_indeterminate-track: linear-gradient(to right,
    var(--_track) 45%,
    var(--_progress) 0%,
    var(--_progress) 55%,
    var(--_track) 0%
  );
  --_indeterminate-track-size: 225% 100%;
  --_indeterminate-track-animation: progress-loading 2s infinite ease;
  
  /*  reset  */
  -webkit-appearance: none;
     -moz-appearance: none;
          appearance: none;
  border: none;
  
  /*  custom style  */
  position: relative;
  height: var(--_track-size);
  border-radius: var(--_radius);
  overflow: hidden
}

@media (prefers-color-scheme: dark) {
progress {
    --_track: hsl(228 20% 30%);
    --_progress: hsl(228 100% 75%)
}}
progress:focus-visible {
    outline-color: var(--_progress);
  }
  /*  Safari/Chromium  */
progress[value]::-webkit-progress-bar {
    background-color: var(--_track);
  }
progress[value]::-webkit-progress-value {
    background-color: var(--_progress);
    -webkit-transition: inline-size .25s ease-out;
    transition: inline-size .25s ease-out;
  }
progress {
  
  /*  Firefox  */
}progress[value]::-moz-progress-bar {
    background-color: var(--_progress);
  }progress {
  
  /*  indeterminate  */
}progress:indeterminate::after {
    content: "";
    inset: 0;
    position: absolute;
    background: var(--_indeterminate-track);
    background-size: var(--_indeterminate-track-size);
    background-position: right; 
    -webkit-animation: var(--_indeterminate-track-animation); 
            animation: var(--_indeterminate-track-animation);
  }

  /*  indeterminate Safari  */
progress:indeterminate::-webkit-progress-bar {
    background: var(--_indeterminate-track);
    background-size: var(--_indeterminate-track-size);
    background-position: right; 
    -webkit-animation: var(--_indeterminate-track-animation); 
            animation: var(--_indeterminate-track-animation);
  }
  
  /*  indeterminate Firefox  */
progress:indeterminate::-moz-progress-bar {
    background: var(--_indeterminate-track);
    background-size: var(--_indeterminate-track-size);
    background-position: right; 
    animation: var(--_indeterminate-track-animation);
  }
  
  /*  complete  */
progress[value="1"]:not([max])::before,
  progress[max="100"][value="100"]::before {
    content: "✓";
    
    position: absolute;
    inset-block: 0;
    inset-inline: auto 0;
    display: flex;
    align-items: center;
    -webkit-padding-end: max(calc(var(--_track-size) / 4), 3px);
            padding-inline-end: max(calc(var(--_track-size) / 4), 3px);

    color: white;
    font-size: calc(var(--_track-size) / 1.25);
  }@-webkit-keyframes progress-loading {
  50% {
    background-position: left; 
  }
}@keyframes progress-loading {
  50% {
    background-position: left; 
  }
}

Actual CSS

progress {
  --_track: hsl(228 100% 90%);
  --_track-size: min(10px, 1ex);
  --_progress: hsl(228 100% 50%);
  --_radius: 1e3px;
  --_indeterminate-track: linear-gradient(to right,
    var(--_track) 45%,
    var(--_progress) 0%,
    var(--_progress) 55%,
    var(--_track) 0%
  );
  --_indeterminate-track-size: 225% 100%;
  --_indeterminate-track-animation: progress-loading 2s infinite ease;
  
  /*  reset  */
  -webkit-appearance: none;
     -moz-appearance: none;
          appearance: none;
  border: none;
  
  /*  custom style  */
  position: relative;
  height: var(--_track-size);
  border-radius: var(--_radius);
  overflow: hidden
}@media (prefers-color-scheme: dark) {progress {
    --_track: hsl(228 20% 30%);
    --_progress: hsl(228 100% 75%)
}
  }progress:focus-visible {
    outline-color: var(--_progress);
  }progress {
  
  /*  Safari/Chromium  */
}progress[value]::-webkit-progress-bar {
    background-color: var(--_track);
  }progress[value]::-webkit-progress-value {
    background-color: var(--_progress);
    -webkit-transition: inline-size .25s ease-out;
    transition: inline-size .25s ease-out;
  }progress {
  
  /*  Firefox  */
}progress[value]::-moz-progress-bar {
    background-color: var(--_progress);
  }progress {
  
  /*  indeterminate  */
}progress:indeterminate::after {
    content: "";
    inset: 0;
    position: absolute;
    background: var(--_indeterminate-track);
    background-size: var(--_indeterminate-track-size);
    background-position: right; 
    -webkit-animation: var(--_indeterminate-track-animation); 
            animation: var(--_indeterminate-track-animation);
  }progress {
  
  /*  indeterminate Safari  */
}progress:indeterminate::-webkit-progress-bar {
    background: var(--_indeterminate-track);
    background-size: var(--_indeterminate-track-size);
    background-position: right; 
    -webkit-animation: var(--_indeterminate-track-animation); 
            animation: var(--_indeterminate-track-animation);
  }progress {
  
  /*  indeterminate Firefox  */
}progress:indeterminate::-moz-progress-bar {
    background: var(--_indeterminate-track);
    background-size: var(--_indeterminate-track-size);
    background-position: right; 
    animation: var(--_indeterminate-track-animation);
  }progress {
  
  /*  complete  */
}progress[value="1"]:not([max])::before,
  progress[max="100"][value="100"]::before {
    content: "✓";
    
    position: absolute;
    inset-block: 0;
    inset-inline: auto 0;
    display: flex;
    align-items: center;
    -webkit-padding-end: max(calc(var(--_track-size) / 4), 3px);
            padding-inline-end: max(calc(var(--_track-size) / 4), 3px);

    color: white;
    font-size: calc(var(--_track-size) / 1.25);
  }@-webkit-keyframes progress-loading {
  50% {
    background-position: left; 
  }
}@keyframes progress-loading {
  50% {
    background-position: left; 
  }
}.button-group {
  display: flex;
  flex-flow: row wrap;
  gap: var(--size-3);
}label {
  display: flex;
  align-items: baseline;
  gap: 1ch;
}main {
  display: grid;
  gap: var(--size-3);
  padding: var(--size-3);
  max-inline-size: 80vw;
}body {
  display: grid;
  align-content: center;
  justify-content: center;
  place-content: center;
}

Does it happen with npx @csstools/csstools-cli <plugin-name> minimal-example.css?

No response

Debug output

No response

Extra config

No response

What plugin are you experiencing this issue on?

PostCSS Nesting

Plugin version

7.4.2

What OS are you experiencing this on?

No response

Node Version

17.1.0

Validations

• Follow our Code of Conduct
• Check that there isn't already an issue that request the same feature to avoid creating a duplicate.

Would you like to open a PR for this bug?

• I'm willing to open a PR

[Antonio-Laguna]

@argyleink thanks for reporting this!

I've narrowed it down to this minimum case:

.foo {
	/*  Baz  */
	&.bar {
		background-color: red;
	}
}

Which produces:

.foo {
	/*  Baz  */
}
.foo.bar {
		background-color: red;
	}

This is mostly due to cleanup-parent not cleaning up given it contains a comment node. My initial instinct was to move the comment along with the new declaration. @romainmenke what do you think about this?

[romainmenke]

Tricky one 🤔

Technically it is correct and some authors might even expect exactly the current behaviour.
The comment is not located within the nested rule but in the parent and the parent should not be modified by postcss-nesting.

The tricky bit is that we can't differentiate between a leading or a trailing comment :

.foo {
	/* #region: Base styles */
	color: red;
	width: 20px;
	/* #endregion: Base styles  */
	&.bar {
		/* #region: Specific styles */
		color: red;
		width: 20px;
		/* #endregion: Specific styles  */
	}
}

Would become :

.foo {
	/* #region: Base styles */
	color: red;
	width: 20px;
}

.foo.bar {
	/* #endregion: Base styles  */
	/* #region: Specific styles */
	color: red;
	width: 20px;
	/* #endregion: Specific styles  */
}

[romainmenke]

@argyleink Is this a blocking issue for you?
Currently I am leaning towards not fixing this.

• it's technically correct
• a CSS minifier would strip these

I can be convinced to fix this if a well defined heuristic can be found.

[argyleink]

not blocking, just noise. i'm commenting out my comments during dev so it's not noise in devtools..

can efforts be made to avoid producing empty selectors? i'm not worried about where the comments go, it's more the side effects and cruft produced by using comments inside nesting. some part of the plugin script is generating a new selector for the parent and then not putting anything into it, then inserting it into the stylesheet. could the selector be verified to contain rules before insertion? i get leaning onto minifiers, but it'd be cleaner if the plugin didnt produce them at all.

[romainmenke]

can efforts be made to avoid producing empty selectors?

This is done for truly empty selectors, but selectors with a comment node are not empty for PostCSS.

Some plugins also listen to comments to change behaviour half way through a file : https://github.com/postcss/autoprefixer#control-comments

---

some part of the plugin script is generating a new selector

The order is preserved as this is important.
For example :

.foo {
	color: red;

	&:where(:hover) {
		width:10px;
	}

	width: 20px;
}

width: 20px can not be moved up as then it wouldn't override width: 10px;.
This CSS is non-sensical, but there will be valid use cases that will break if we change the order.

To preserve the order we must insert duplicates of the parent selector.
If the parent is empty this is skipped. But a comment counts as "not empty".

Side note : We tried to enforce the spec here and disallowed declarations after nested rules. This backfired as there is too much sugary syntax that is a declaration in PostCSS first but then expanded to a rule later. We reverted this enforcement.

If we had been able to add this enforcement it would be trivial to count all comments of this kind as belonging to the nested selector.

more reading on this : csstools/postcss-nesting#95 (comment)

---

i get leaning onto minifiers, but it'd be cleaner if the plugin didnt produce them at all.

If we can define a good heuristic we can do this.
But currently I see no way to differentiate trailing and leading comments as illustrated in my example above :/

[argyleink]

We tried to enforce the spec here and disallowed declarations after nested rules. This backfired as there is too much sugary syntax that is a declaration in PostCSS first but then expanded to a rule later. We reverted this enforcement.

yeah, i expect this to be a gotcha once folks switch from scss/postcss nesting to built-in. totally coo if you kept it 👍🏻

To preserve the order we must insert duplicates of the parent selector.
If the parent is empty this is skipped. But a comment counts as "not empty".

proposal: if a selector is only containing a comment, replace the selector and comment with just the comment?

progress {
  
  /*  indeterminate Firefox  */
}progress:indeterminate::-moz-progress-bar {
    background: var(--_indeterminate-track);
    background-size: var(--_indeterminate-track-size);
    background-position: right; 
    animation: var(--_indeterminate-track-animation);
  }progress {
  
  /*  complete  */
}

becomes

 /*  indeterminate Firefox  */progress:indeterminate::-moz-progress-bar {
    background: var(--_indeterminate-track);
    background-size: var(--_indeterminate-track-size);
    background-position: right; 
    animation: var(--_indeterminate-track-animation);
  }/*  complete  */

hopefully this keeps the order but targets just the problem area (selectors with only comments). that list of empty selectors in devtools looks buggy, this would be hard to explain to someone new who is just trying to diligently comment their code, yet the output bloats.

thanks for looking at this, def not as simple to solve as i'd anticipated!

[romainmenke]

if a selector is only containing a comment, replace the selector and comment with just the comment?

I like this!
Not going to comment on how hard it's going to be, don't want to jinx it :D

[Antonio-Laguna]

Thanks for all of this and I'm glad it was something that could get fixed!

@argyleink this got just released as 10.1.3!