search

Netcentric / stylelint-config

Cognizant Netcentric's coding and style rules for Stylelint
Apache License 2.0
4 stars 1 forks source link

Add bem selector-class-pattern stylelint rule #8

Open juantxokupul opened 2 years ago

quicoto commented 2 years ago

@juantxokupul Sweet, can we add a test for something like:

.block__elementSomething

Which I've seen in many places. See what happens, or what do we think should happen.

Thank you

raohmaru commented 2 years ago

Following contribution guidelines, please fill an issue and link to this PR. This way we can track and discuss this change.

raohmaru commented 2 years ago

In my opinion, we shouldn't force a naming convention. This project is open source and hence potentially it can be used by anyone and not only by Netcentric. And as much as at Netcentric we love BEM, we cannot force it to any developer that wants to use this stylelint configuration.

quicoto commented 2 years ago

@raohmaru developers can always overwrite this rule in their stylelint.config (which they'll need anyway to add the extend rule with our package)

Considering this should be one of the standard packages we use for all our new projects, having to copy and paste this long regex every time to follow the NC Code Style seems overkill. Seems more optimal to have it here in the package and remove it in the few cases where it's not wanted.

quicoto commented 2 years ago

I liked it when it was a 1 liner in the config.

I'm not a fan of adding a new dependency, which can not be taken out via config when someone includes this package.

juantxokupul commented 2 years ago

Issue created by @prototowb, thank you!

@raohmaru This PR was created since kebab-case was the default class naming convention, and we thought that bem would be better. I think this repository is intended to establish some default configs. As @quicoto says, you can always override.

@quicoto yes, I also don't like to add a dependency, but you need a css post-processor to support scss nested selectors.

quicoto commented 2 years ago

I'm retracting my enthusiasm with this change. I don't think we should go forward with it.

juantxokupul commented 2 years ago

NOTE: you can take out this config setting the rule to null

"plugin/selector-bem-pattern": null

(EDIT) you also need to define the component in order to have this working. Without that, this will do nothing -> no need to override anything.

juantxokupul commented 2 years ago

PR updated

I realized that stylelint is already a post processor and the only thing needed was to set resolveNestedSelectors option to true

The only advantage I see with the plugin, is that you can apply bem naming convention only in certain css files/sections

juantxokupul commented 2 years ago

@juantxokupul Sweet, can we add a test for something like:

.block__elementSomething

Which I've seen in many places. See what happens, or what do we think should happen.

Thank you

this is allowed and I think it should I searched in NC wiki and found this https://projects.netcentric.biz/wiki/pages/viewpage.action?spaceKey=~natalia.venditto&title=Code+Style+-+CSS+Guidelines#CodeStyleCSSGuidelines-Namingrules

The page is under 'Archive OLD' itself but seems like a good reference to me

quicoto commented 2 years ago

@juantxokupul Please have a look at the updated page (subpages) of https://projects.netcentric.biz/wiki/pages/viewpage.action?pageId=48141171

Which also needs to be updated... Archive pages should not be used.

juantxokupul commented 2 years ago

@quicoto thank you!

Fortunately, it seems the 6. Classes (.name) section is a copy/paste from the old :)