search

leeoniya / dropcss

An exceptionally fast, thorough and tiny unused-CSS cleaner
MIT License
2.13k stars 68 forks source link

Another infinite loop #19

Closed heygrady closed 5 years ago

heygrady commented 5 years ago

Related #16

Here's an example: https://repl.it/@heygrady/dropcss-example

It would be nice if it would throw instead of infinite looping. It doesn't matter much to me if this throws but infinite loops are a pretty big deal. Is it possible to put some runaway loop protection in place?

Regarding https://github.com/leeoniya/dropcss/issues/16#issuecomment-484268826

https://github.com/leeoniya/dropcss/blob/master/src/html.js#L66-L67 https://github.com/leeoniya/dropcss/blob/master/src/css.js#L159-L160

Would this basically fix it?

let prevPos = pos
while (pos < html.length) {
    next();
    // is the problem that we stop advancing in some cases?
    if (prevPos === pos) {
        throw new Error('tokenizer did not advance');
        break;
    }
    prevPos = pos;
}
leeoniya commented 5 years ago

yeah that's the general idea. it would be useful to add some context to the error like pos, previous token (or prev x chars) and next x chars.

you can try temporarily undoing the <br/> fix with these guards in place and see if it makes the problem diagnosis trivial, too.

heygrady commented 5 years ago

BTW, the repl.it link is the same but the html/css it's testing has changed. I think that wasn't clear.

Anyway, not sure why but the issue is that find is looping forever.

https://github.com/leeoniya/dropcss/blob/master/src/find.js#L186-L187

            // if you count down the idx by default it will finish
            default:
                ctx.idx--;

m looks like this:

["body","_"," ","input","_",["'color'","=","type","["],["'date'","=","type","["],"not",":",["'week'","=","type","["],"not",":",["'range'","=","type","["],"not",":",["'radio'","=","type","["],"not",":",["'month'","=","type","["],"not",":",["'checkbox'","=","type","["],"not",":","not",":"]
leeoniya commented 5 years ago

reduced initial testcase for this to:

<body><input></body>
body input:not([type='color']):not([type='checkbox']) {}

:not at least 2 times on a descendant element appears to be the necessary pre-condition to trigger this.

leeoniya commented 5 years ago

yeah, so this was a bug in the selector tokenizer. it was splicing the pseudos into the wrong position. that m you posted above definitely looked wrong. the specific fix was line 39. that commit also includes a tweak to the attr regex to ignore surrounding single quotes around values (for normalization); before, only double quotes were ignored.

thanks!