1857522 - Consider reducing the Stylelint-allowed maximum level of CSS nesting from 8 to... 4?

Status: RESOLVED FIXED

(Developer Infrastructure :: Lint and Formatting, task)

Description

[:Gijs (he/him)]

+++ This bug was initially created as a clone of Bug #1856660 +++

The nesting level was set at 8. That seems high to me, but newtab/messaging-system uses it. It would be interesting to understand why and what people think is a "reasonable" value. Personally, a selector with 8 descendant or child selectors seems unusable to me, but perhaps I'm in the minority.

Comment 1

[Dave Townsend [:mossop]]

Yes when I saw 8 I was surprised. I'd go as far as suggesting that 3 would be enough but I haven't used nesting much yet myself so I don't have a great sense for it. Dão probably has better opinions than me.

Comment 2

[Dão Gottwald [:dao]]

Yeah, 8 is too high and only there because of newtab code, which overuses nesting (and the descendant selector). It's not a model I think we want to follow. We should have newtab override the limit for now, and set the global limit to 4, would be my initial guess based on the nesting we've introduced so far. I feel like urlbarView.css and tabs.css have been good test cases, and I think 4 may be the maximum I've reached, possibly only once: https://searchfox.org/mozilla-central/rev/6602bdf9fff5020fbc8e248c963ddddf09a77b1b/browser/themes/shared/tabs.css#330
Perhaps that's an argument for 3, but I think I'd err on the side of leaving some leeway here. Usability depends a lot on how large your rules are as well. If they all fit on a screen like in the above example, 4 seems okay?

Comment 3

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1857522 - Reduce max nesting depth to 3. r=#desktop-theme-reviewers

That still doesn't trigger on the code in comment 2, so that is a bit
confusing and needs investigation.

Comment 4

[Emilio Cobos Álvarez (:emilio)]

Attachment: Bug 1857522 - Don't ignore pseudo-classes in max-nesting-depth limit. r=#desktop-theme-reviewers

As that allows you to get around the lint with :is() trivially.

Comment 5

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/3a9825468499
Reduce max nesting depth to 3. r=Gijs

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/38314c6dc43e
Don't ignore pseudo-classes in max-nesting-depth limit. r=Gijs

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d63b657f832f
Update autogenerated files.

Comment 8

[Atila Butkovits]

https://hg.mozilla.org/mozilla-central/rev/3a9825468499
https://hg.mozilla.org/mozilla-central/rev/38314c6dc43e
https://hg.mozilla.org/mozilla-central/rev/d63b657f832f