Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix css properties check in bem mod files #172

Merged
merged 1 commit into from
Jun 27, 2023

Conversation

andieelmes
Copy link
Contributor

@andieelmes andieelmes commented Jun 19, 2023

When valid custom css property name is redefined in the modifier file, error about invalid property name is thrown.

For example, we have three files:

// component.css

.component {
  --component-color: red;
}

// component_mod.css

.component_mod {
  --component-color: green;
}

Second file is currently not considered valid:

component_mod.css
 2:3  ✖  Invalid custom property name "--component-color": a component's custom properties must start with the component name  plugin/selector-bem-pattern

While the third file is okay:

// component_mod2.css

.component_mod2 {
  --component_mod2--color: purple;
}

Test repo: https://github.com/andieelmes/test-postcss-bem-linter
Lint job: https://github.com/andieelmes/test-postcss-bem-linter/actions/runs/5314755305/jobs/9622321460

for (let i = 0; i < filename.length; i++) {
const part = filename.slice(0, -i);
if (componentNamePattern.test(part)) return part;
}
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It looks really clunky, but I'm not sure how to determine the implicit component name without adding another regex for that in the config.

index.js Outdated
@@ -80,6 +81,7 @@ module.exports = postcss.plugin(
validateCustomProperties({
rule,
componentName: range.defined,
selectorPattern: patterns.componentSelectors,
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

config.selectorPattern is unused inside of validateCustomProperties, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

removed, thanks

@simonsmith
Copy link
Collaborator

simonsmith commented Jun 20, 2023

Looks like a good fix to me. I'm not familiar with having modifiers across multiple files but I'm a bit out of touch with BEM practices these days

I've just merged an outstanding PostCSS 8 update as well, could you rebase on master and make sure this still passes as expected? Thanks a lot

@andieelmes
Copy link
Contributor Author

Sure, thanks

We encountered a bug related to the fix in one of our projects, I'll try fixing that and then will rebase

@andieelmes
Copy link
Contributor Author

Just rebased, thanks

Tests still pass

@andieelmes
Copy link
Contributor Author

@simonsmith, could you please have a look?

@simonsmith
Copy link
Collaborator

It looks good to me, thanks for this!

@simonsmith simonsmith merged commit 9a4e4f9 into postcss:master Jun 27, 2023
@simonsmith
Copy link
Collaborator

@andieelmes
Copy link
Contributor Author

Thank you)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants