This repository was archived by the owner on Jan 8, 2026. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 0
Disable NestingDepth rule #32
Open
alvinsiu
wants to merge
3
commits into
master
Choose a base branch
from
sass
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
How far down do you need to go? I think turning this off is a little heavy-handed.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
My opinion is that there really isn't necessarily a max depth we should use, but this also really depends on what paradigm FF will be using for styling (which I don't think there necessarily is one). If we're using BEM, then I would actually not change this linting rule at all since it allows for flat css structure with a lot less risk of unintentional styling overlaps. But if we're not using BEM (which I don't really see true BEM in the few codebases I looked in), then nesting is critical, especially with such the frontend codebases being so fragmented (a CSS rule in component library can affect a different app, and that most likely will not be caught in QA).
And from my personal experience, most developers aren't super aware of BEM, and even if they are, there's quite a few rules you have to follow fairly strictly to take full advantage of it. This makes me generally avoid requiring the use of BEM by all engineers and instead encourage the use of "componentizing" styling rules by nesting. Does it make the html/styling less maintainable? Probably a little bit, but I think given the primary concern of our software is backend, so I'd rather not add more overhead for people developing HTML/CSS. If our business switches focus to frontend a bit more where we are prioritizing a more consistent styling across our frontends, then I think that's a good time to revisit.
So for me, it's kind of all or nothing - keep the rule and require BEM (or similar technique), or get rid of the rule and encourage componentizing through nesting for custom styling.