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.

Sign up for GitHub

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

Jump to bottom

BEM + pseudo trips up custom var extraction: .a--b:hover{} #31

Closed
saltymouse opened this issue Jun 2, 2019 · 5 comments
Closed

BEM + pseudo trips up custom var extraction: .a--b:hover{} #31

saltymouse opened this issue Jun 2, 2019 · 5 comments
Labels
bug Something isn't working

Comments

@saltymouse
Copy link

Seems like there's some trouble parsing BEM selectors, namely the --modifier bit.

const dropcss = require('dropcss');

let html = `
    <html>
        <head></head>
        <body>
          <div class="tab-table">
            <div class="tab-table__item">Tab 1</div>
            <div class="tab-table__item tab-table__item--new">Tab 2</div>
          </div>
        </body>
    </html>
`;

let css = `                                                                                           
.tab-table {
  display: flex;
  flex-flow: row nowrap;
  position: relative;
}

.tab-table__item {
  background: #494545;
  border-width: 0 .1rem;
  cursor: pointer;
  flex: 1 1 20%;
  flex-flow: column nowrap;
}

.tab-table__item--new::before {
  align-items: center;
  color: #fff;
  display: flex;
  justify-content: center;
}
`;

let cleaned = dropcss({
    html,
    css, 
});

console.log(cleaned);
/*
{ css: '.tab-table{display: flex;\n  flex-flow: row nowrap;\n  position: relative;}.tab-table__item{background: #494545;\n  border-width: 0 .1rem;\n  cursor: pointer;\n  flex: 1 1 20%;\n  flex-flow: column nowrap;}.tab-table__itemcolor: #fff;\n  display: flex;\n  justify-content: center;}',
  sels: 
   Set {
     '.tab-table',
     '.tab-table__item',
     '.tab-table__item--new::before' } }
*/

Formatted CSS to more clearly see what's going on:

.tab-table {
  display: flex;
  flex-flow: row nowrap;
  position: relative;
}

.tab-table__item {
  background: #494545;
  border-width: 0 .1rem;
  cursor: pointer;
  flex: 1 1 20%;
  flex-flow: column nowrap;
}

.tab-table__itemcolor: #fff; /* should be `.tab-table__item--new::before {` */
display: flex;
justify-content: center;
}

dropcss version: 1.0.12
node version: 10.16.0
@saltymouse
Copy link
Author

saltymouse commented Jun 2, 2019 •
edited
Loading

Update: it seems the problem isn't BEM alone. The block--modifier::before (BEM + pseudo-element) pattern is getting captured by the regex for processing CSS variables (e.g. --css-var-color: brown;) in the output function here: CUSTOM_PROP_DEF

Here's another test:

const dropcss = require('dropcss');                                                                   

let html = `
    <html>
        <head></head>
        <body>
          <div class="anything">anything 1</div>
          <div class="anything anything--modified">anything 2</div>
        </body>
    </html>
`;

let css = `
:root {
  --main-color: deeppink;
}

.anything {
  display: inline-flex;
  background: var(--main-color);
}

.anything::before {
  color: deepskyblue;
}

.anything--modified {
  color: crimson;
}

.anything--modified::before {
  color: tomato;
}

.anything--modified::after {
  color: papayawhip;
}
`;

let cleaned = dropcss({
    html,
    css, 
});

console.log(cleaned);
/*
{ css:
   ':root{--main-color: deeppink;}.anything{display: inline-flex;\n  background: var(--main-color);}.anything::before{color: deepskyblue;}.anything--modified{color: crimson;}.anything}.anything}',
  sels:
   Set {
     ':root',     '.anything',
     '.anything::before',
     '.anything--modified',
     '.anything--modified::before',
     '.anything--modified::after' } }
*/

Again, the final .anything--modified::before {...} rule is truncated to .anything} because of the CUSTOM_PROP_DEF regex.

I'm not sure if this is the appropriate fix or not, but adding a negative-lookbehind (how to do this in JS?) for any word-char seems hone the regex to only CSS vars (because CSS vars should always begin with -- without anything before, unlike a BEM modifier which has the ELEMENT preceding the -- bit).

Original CUSTOM_PROP_DEF regex: https://regex101.com/r/0sb17w/2
Modified CUSTOM_PROP_DEF regex: https://regex101.com/r/0sb17w/3 (possible in JS?)

@leeoniya
Copy link
Owner

leeoniya commented Jun 2, 2019 •
edited
Loading

hey @saltymouse

thanks for the repro and diagnosis.

I'm not sure if this is the appropriate fix or not, but adding a negative-lookbehind (how to do this in JS?)

lookbehinds are pretty new in js: https://caniuse.com/#feat=js-regexp-lookbehind, so i'd prefer not to use them.

we can probably get away with just capturing any non-word char at the start of the match via ([^\w]) and tweak some of the replace() bits: https://regex101.com/r/0sb17w/5

wanna give it a go?

@leeoniya leeoniya added the bug Something isn't working label Jun 2, 2019
@leeoniya leeoniya changed the title Support BEM selectors BEM + pseudo trips up custom var extraction: .a--b:hover{} Jun 2, 2019
@leeoniya
Copy link
Owner

leeoniya commented Jun 2, 2019

actually \w might be too loose, eg: .--foo:hover{} is a valid class. maybe ([{};])\s* would be better.

https://regex101.com/r/0sb17w/6

@leeoniya
Copy link
Owner

leeoniya commented Jun 3, 2019

ok, this should be fixed now. i tried both of your cases in https://codepen.io/leeoniya/pen/LvbRyq and they work as expected.

@saltymouse
Copy link
Author

Awesome—thanks for the quick response. I upgraded to the 1.0.13 version you just pushed and the previous (above) errors in my production codebase are also no more! Confirmed fix on my end too.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
bug Something isn't working
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
2 participants
@leeoniya @saltymouse