mrnocreativity / postcss-critical-split

A PostCSS plugin that takes existing CSS files and splits out the annotated critical styles into a seperate file.
MIT License
89 stars 6 forks source link

Missing critical rules #16

Closed agarzola closed 5 years ago

agarzola commented 5 years ago

Hi! 👋 I noticed some rules missing from my critical CSS files, so I reproduced the issue with a contrived example. Given the following CSS:

/* critical:start */
header {
  background: red;
}

/* critical:end */
p {
  color: orange;
}

/*# sourceMappingURL=test.css.map */
/* v19.04.19 +4 (c00d698d) */

And the following gulp tasks:

const gulp = require('gulp');
const postCSS = require('gulp-postcss');
const criticalSplit = require('postcss-critical-split');
// … etc.

function criticalCss() {
  return gulp.src('./tmp/assets/css/test.css')
    .pipe(postCSS([
      criticalSplit({
        debug: true,
        output: 'critical',
      }),
    ]))
    .pipe(rename(path => path.basename += '-critical'))
    .pipe(gulp.dest('tmp/assets/css'));
}

function nonCriticalCss() {
  return gulp.src('./tmp/assets/css/test.css')
    .pipe(postCSS([
      criticalSplit({
        debug: true,
        output: 'rest',
      }),
    ]))
    .pipe(rename(path => path.basename += '-noncritical'))
    .pipe(gulp.dest('tmp/assets/css'));
}

gulp.task('styles', gulp.series(criticalCss, nonCriticalCss));

I’m getting the following in my test-noncritical.css file:

p {
    color: orange;
}
/* v19.04.19 +4 (c00d698d) */

and an empty test-critical.css.

The debug info for the output: 'critical' config:

----- postcss-critical-split debug info ---------
processTime: 3ms
rules: 8
criticals: 1
criticalPercentage: 12.50%
loops: 2
clones: 2
emtpyClones: 1
compares: 0
appends: 1
getParents: 1
parentRequest: 12
-------------------------------------------------

The debug info for the output: 'rest' config:

----- postcss-critical-split debug info ---------
processTime: 0ms
rules: 8
criticals: 1
criticalPercentage: 12.50%
loops: 2
clones: 2
emtpyClones: 1
compares: 0
appends: 1
getParents: 1
parentRequest: 12
-------------------------------------------------

I’m not sure what constitutes a rule in the context of these debug logs, but I find 8 to be far more than I expect. criticalPercentage seems low, considering what’s inside the critical:start/critical:end comment tags.

I’m using gulp-postcss v6.4, if that means anything. Also, in case it’s useful, the CSS I’m feeding through postcss-critical-split is generated via gulp-sass in a separate task.

Please let me know if I’m doing something wrong here, or if there is any other information I can provide that would be useful to you. Thanks!

mrnocreativity commented 5 years ago

hey @agarzola! I haven't worked on this for a while. But you're right. Something seems off.

Don't bother with the debug logs. I used these back in the days when I had serious trouble debugging complex constructs for testing and validation. These could very well show you some incorrect numbers. I'm not sure about the terminology and what constitutes a rule either. But if I remember correctly, it refers to pretty much 'any line that is not a closing tag or a whiteline'.

But yea, checking out your sample, I'm a bit shocked that this simple example would fail. My first suspicion would be user error but looking through your code, I'd assume it's correct (at least I'd do the same if you'd ask me to do it :D )

I'll try to find some time to verify why this isn't working as expected.

Since it's been a while since I used it myself and I barely remember the options and the defaults: Did you try setting the separators etc using the config? Did that not yield better results?

agarzola commented 5 years ago

Thanks for the prompt reply, @mrnocreativity! I was about to update this with the following comment: After writing this, it occurred to me that I should check against the latest version of gulp-postcss (v7.0.1, as of this comment). This appears to have fixed the issue.

The debug logs are still confusing, but I’ll follow your advice and pay no attention to them. 😄 Thanks again for the blazingly fast response and for putting this tool out there!

mrnocreativity commented 5 years ago

Glad you were able to solve this! Have fun and good luck with your project :)