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

Issue with prepending selectors (separated by whitespace) #130

Closed
saulhardman opened this issue Apr 9, 2018 · 10 comments
Closed

Issue with prepending selectors (separated by whitespace) #130

saulhardman opened this issue Apr 9, 2018 · 10 comments

Comments

@saulhardman
Copy link

saulhardman commented Apr 9, 2018

Hi @simonsmith, first of all, thanks a lot for maintaining this project 🙇

We're using SCSS syntax on the project in question and I'm attempting to extend the componentSelectors RegExp to allow for 'global state' classes to be prepended. E.g. .no-js .c-responsive-image etc.

The regular expression that I've conjured up appears to do the job when run separately, but it's outputting warnings when used with BEM Linter.

A (relatively) reduced test case can be found here on Glitch.

/** @define responsive-image */
.c-responsive-image {
  display: block;

  .no-js & {
    display: none;
  }
}

/* without `&` */
.no-js .c-responsive-image {
  display: none;
}
const { join } = require('path');
const { promisify } = require('util');
const readFile = promisify(require('fs').readFile)

const postcss = require('postcss');
const bemLinter = require('postcss-bem-linter');
const reporter = require('postcss-reporter');

const kebabCaseString = '[a-z]+[-[a-z]+]*';
const kebabCaseStringWithNumbers = '[a-z]+[-[a-z0-9]+]*';
const globalStateClassNames = `\\.(no|has|supports|t)-${kebabCaseString}`;
const localStateClassNames = `\\.(is|has)-${kebabCaseString}`;

const componentName = `^${kebabCaseString}$`;
const componentSelectors = name => new RegExp(`^(${globalStateClassNames}\\s)?\\.[co]-${name}(_{2}${kebabCaseString})?(-{2}${kebabCaseString})?(${localStateClassNames})?$`);
const utilitySelectors = `^\\.u-${kebabCaseStringWithNumbers}$`;

const bemLinterOptions = {
  componentName,
  componentSelectors,
  utilitySelectors,
};

console.log('Standalone RegExp test:', componentSelectors('responsive-image').test('.no-js .c-responsive-image'));
            
module.exports = (async () => {
  const scss = await readFile(join(process.cwd(), 'index.scss'), 'utf8');

  await postcss()
    .use(bemLinter(bemLinterOptions))
    .use(reporter())
    .process(scss)
})();

Can you spot anything obvious that I'm doing wrong? If this is in fact a bug then I'm more than willing to contribute to the fix, but I'll need a little guidance.

Edit:

# error output
6:3	⚠  Invalid component selector ".no-js .c-responsive-image" [postcss-bem-linter]
5:50 PM
12:1	⚠  Invalid component selector ".no-js .c-responsive-image" [postcss-bem-linter]
@simonsmith
Copy link
Collaborator

Thanks for the detailed report!

The regular expression that I've conjured up appears to do the job when run separately, but it's outputting warnings when used with BEM Linter.

Are you saying the plugin appears to be ignoring your regex?

@saulhardman
Copy link
Author

Thanks for the quick response!

It's not that the plugin is ignoring it, more that it appears to be giving me a false negative.

@simonsmith
Copy link
Collaborator

I'll try your example later and report back

@saulhardman
Copy link
Author

Thanks @simonsmith 👍

@simonsmith
Copy link
Collaborator

@saulhardman Do I need to join the Glitch project to access the console?

@saulhardman
Copy link
Author

@simonsmith unsure – it's the first time I've taken it for a spin. I've accepted your request to join.

@simonsmith
Copy link
Collaborator

simonsmith commented Apr 10, 2018

I've had a look at the source code with your example (I haven't looked for a while!):

The exported function in validate-selectors.js is where your componentSelectors function ends up as selectorPattern. That is used to generate the initialPattern regex which looks like:

^(\.(no|has|supports|t)-[a-z]+[-[a-z]+]*\s)?\.[co]-responsive-image(_{2}[a-z]+[-[a-z]+]*)?(-{2}[a-z]+[-[a-z]+]*)?(\.(is|has)-[a-z]+[-[a-z]+]*)?$

Line 54 Runs your regex against each 'sequence' in the selector. In this case the sequences are:

[ '.no-js', '.c-responsive-image' ]

If I run the generated regex against both of those sequences it only passes for .c-responsive-image as seen here - https://regex101.com/r/SmL12u/2

The README.md says

initial describes valid initial selector sequences — those occurring at the beginning of a selector, before any combinators.

So perhaps try to adjust your regex accordingly to be able to run against each sequence. I admit the plugin itself could make this far clearer!

Hope that helps. We can always improve things if you think this is not working as you expected or not very clear

@saulhardman
Copy link
Author

Hey @simonsmith, thanks very much for looking into this 👍

It just shows my ignorance with respect to the meaning behind and function of the 'initial' and 'combined' selectors. The terms are pretty confusing, so perhaps walkthrough example would be helpful?

I've separated the 2 regexes now and appear to have gotten to a satisfactory point.

For posterity, the following hastily constructed and thoroughly untested RegExp allows the following selector format:

[.no-js.supports-intersection-observer] .c-card[__body][--large][.is-active.is-highlighted]
const { oneLineTrim } = require('common-tags');

const kebabCaseString = '[a-z]+(?:-[a-z]+)*';
const kebabCaseStringWithNumbers = '[a-z]+(?:-[a-z0-9]+)*';
const globalStateClassNames = `(?:\\.(?:no|has|supports|t)-${kebabCaseString})+`;
const localStateClassNames = `(?:\\.(?:is|has)-${kebabCaseString})+`;
const componentSelector = name => oneLineTrim`
  \\.[co]-${name}
  (?:_{2}${kebabCaseString})?
  (?:-{2}${kebabCaseString})?
  (?:${localStateClassNames})?
`;

const componentName = `^${kebabCaseString}$`;
const componentSelectors = {
  initial: name => new RegExp(oneLineTrim`^
    (?:${globalStateClassNames})?
    (?:${componentSelector(name)})?
  $`),
  combined: name => new RegExp(oneLineTrim`^
    (?:${globalStateClassNames}\\s)*
    ${componentSelector(name)}
  $`),
};
const utilitySelectors = `^\\.u-${kebabCaseStringWithNumbers}$`;

// .stylelintrc.js
module.exports = {
  plugins: [
    'stylelint-selector-bem-pattern',
  ],

  rules: {
    'plugin/selector-bem-pattern': {
      componentName,
      componentSelectors,
      utilitySelectors,
    },
  },
};

// using PostCSS
postcss()
  .use(bemLinter({
    componentName,
    componentSelectors,
    utilitySelectors,
  }))
  .use(reporter())

@simonsmith
Copy link
Collaborator

@saulhardman Good to hear! Glad it works

I agree the documentation is very light on initial vs combined. I think some code examples are needed there.

If you're happy this is resolved we can close this and I'll track the documentation update in a different issue

@saulhardman
Copy link
Author

Yes, most definitely, thanks again for the help Simon 🙇 👏 👍

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

No branches or pull requests

2 participants