Merge pull request #2 from hummal/develop

out of the shadows - RC

README.md

@@ -208,6 +208,7 @@ The CLI usage is not implemented yet :scream:. At the moment there is no need of
 - [ ] :star: cookie includes
 - [ ] :star: wildcards
 - [ ] :grey_question: positioning of critical css rules 
+- [ ] :+1: compress output option
 - [x] :fire: return of the remaining css aswell
 - [x] :grey_question: multi selector partial matches
 - [x] :tea: returning of remaining css aswell (optional)
@@ -219,7 +220,7 @@ None yet
 
 ## Troubleshooting
 
-### WSL / Windows Linux Subsystem Support
+#### WSL / Windows Linux Subsystem Support
 Some unkown reasons prevent puppeteer to run properly in a WSL environment. If you have any errors please try to use your default OS command shell or equivalent. If the error still exists don't hesitate to create an issue ticket.
 
 ## Inspiration

gulpfile.js

@@ -7,7 +7,6 @@ const clean  = require('gulp-clean');
 const srcDir = path.join("./", "src", "**");
 const libDir = path.join("./", "lib");
 
-
 gulp.task('cleanup', () => {
     return gulp.src(libDir, {read: false})
                .pipe(clean());

index.js

@@ -3,7 +3,12 @@
 const log          = require("signale");
 const path         = require("path");
 const NODE_ENV     = process.env.NODE_ENV || "production";
-const pathToCrittr = NODE_ENV === "development" ? "src" : "lib";
+let IS_NPM_PACKAGE = false;
+try {
+    IS_NPM_PACKAGE = !!require.resolve("crittr");
+} catch (e) {}
+
+const pathToCrittr = NODE_ENV === "development" && !IS_NPM_PACKAGE ? "src" : "lib";
 const Crittr       = require(path.join(__dirname, pathToCrittr, 'classes', 'Crittr.class.js'));
 
 /**
@@ -14,8 +19,8 @@ const Crittr       = require(path.join(__dirname, pathToCrittr, 'classes', 'Crit
 module.exports = (options) => {
     return new Promise(async (resolve, reject) => {
         log.time("Crittr Run");
-        const crttr      = new Crittr(options);
-        let resultObj  = {critical: null, rest: null};
+        const crttr   = new Crittr(options);
+        let resultObj = {critical: null, rest: null};
         try {
             (resultObj = await crttr.run());
         } catch (err) {

package.json

@@ -61,7 +61,7 @@
   ],
   "husky": {
     "hooks": {
-      "pre-commit": "npm test"
+      "pre-commit": "npm run build && git add lib/* && npm test"
     }
   }
 }

src/classes/Crittr.class.js

@@ -417,7 +417,7 @@ class Crittr {
                     // Handle sorting by option
                     finalCss = mqpacker.pack(finalCss, {
                         sort: sortCSSmq
-                    });
+                    }).css;
                     debug("getCriticalCssFromUrls - Creating AST Object of atf ruleMap - Finished");
 
                     // Handle restCSS
@@ -432,7 +432,7 @@ class Crittr {
 
                         finalRestCss = mqpacker.pack(finalRestCss, {
                             sort: sortCSSmq
-                        });
+                        }).css;
                     }
 
                     resolve([finalCss, finalRestCss, errors]);

src/classes/CssTransformator.class.js

@@ -28,14 +28,11 @@ class CssTransformator {
             ':first-line'
         ];
 
-        this._TYPES_TO_REMOVE = [
-            "comment",
-            "keyframes",
-            "keyframe"
-        ];
-        this._TYPES_TO_KEEP   = [
+        this.CRITICAL_TYPES_TO_KEEP = [
+            "media",
+            "rule",
             "charset",
-            "font-face"
+            "font-face",
         ];
 
         // detect these selectors regardless of whether one or two semicolons are used
@@ -71,84 +68,6 @@ class CssTransformator {
         })
     }
 
-    /**
-     * Remove all selectors that match one of the removeSelectors.
-     * Mutates the original Object
-     *
-     * @param ast {Object}
-     * @param removeSelectors {Array<String>}
-     * @returns {Object}
-     */
-    filterSelector(ast, removeSelectors) {
-        if (!Array.isArray(removeSelectors)) {
-            log.warn("removeSelectors have to be an array to be processed");
-            return false;
-        }
-
-        let rules = ast;
-
-        // Get Rules of ast object and keep reference
-        if (ast.stylesheet) {
-            rules = ast.stylesheet.rules;
-        } else if (ast.rules) {
-            rules = ast.rules;
-        }
-
-        const compareFn = (a, b) => {
-            return b - a;
-        };
-
-        const removeableRules = [];
-
-        for (const ruleIndex in rules) {
-            if (rules.hasOwnProperty(ruleIndex)) {
-                const rule = rules[ruleIndex];
-
-                if (Rule.isMediaRule(rule)) {
-                    // Recursive check of CSSMediaRule
-                    this.filterSelector(rule, removeSelectors);
-                } else {
-                    //  CSSRule
-                    const selectors           = rule.selectors;
-                    const removeableSelectors = [];
-
-                    for (let selectorIndex in selectors) {
-                        if (selectors.hasOwnProperty(selectorIndex)) {
-                            const selector = selectors[selectorIndex];
-
-                            // TODO: deal with wildcards
-                            if (removeSelectors.includes(selector)) {
-                                // More than one selector in there. Only remove the match and keep the other one.
-                                // If only one selector exists remove the whole rule
-                                if (selectors.length > 1) {
-                                    removeableSelectors.push(selectorIndex);
-                                } else {
-                                    removeableRules.push(ruleIndex);
-                                }
-                            }
-                        }
-                    }
-
-                    // Sort the removeableSelectors DESC to remove them properly from the selectors end to start
-                    removeableSelectors.sort(compareFn);
-                    // Now remove them
-                    for (let selectorIndex of removeableSelectors) {
-                        selectors.splice(selectorIndex, 1);
-                    }
-                }
-            }
-        }
-
-        // Sort the removeableRules DESC to remove them properly from the rules end to start
-        removeableRules.sort(compareFn);
-        // Now remove them
-        for (let ruleIndex of removeableRules) {
-            rules.splice(ruleIndex, 1);
-        }
-
-        return ast;
-    }
-
     /**
      * Filters the AST Object with the selectorMap <Map> containing selectors.
      * Returns a new AST Object without those selectors. Does NOT mutate the AST.
@@ -177,9 +96,9 @@ class CssTransformator {
             return [];
         };
 
-        // Filter rule types we don't want
+        // Filter rule types we don't want in critical
         let newRules = _astRoot.rules.filter(rule => {
-            return !this._TYPES_TO_REMOVE.includes(rule.type);
+            return this.CRITICAL_TYPES_TO_KEEP.includes(rule.type);
         });
 
         // HANDLE CRITICAL CSS
@@ -261,7 +180,7 @@ class CssTransformator {
 
         // Process removeables
         restRules = restRules.filter(rule => {
-            return !(removeableRules.includes(rule) || this._TYPES_TO_KEEP.includes(rule.type));
+            return !(removeableRules.includes(rule));
         });
 
         _astRoot.rules     = newRules;

src/classes/Rule.class.js

@@ -103,7 +103,7 @@ class Rule {
 
     static generateRuleKey(rule, media = "", withKeySeparator = false) {
         const keySeparator = withKeySeparator ? CONSTANTS.RULE_SEPARATOR : "";
-        let ruleStr;
+        let ruleStr = "default";
 
         if (Rule.isRule(rule) && rule.selectors) {
             ruleStr = rule.selectors.join();
@@ -120,8 +120,8 @@ class Rule {
         } else if (Rule.isComment(rule)) {
             return false;
         } else {
-            log.error("Can not generate rule key of rule without selectors! Maybe this is a media query?", rule);
-            return false;
+            //log.error("Can not generate rule key of rule without selectors! Setting default key!", rule);
+            return ruleStr;
         }
 
         return media + keySeparator + ruleStr;

test/basis.test.js

@@ -47,6 +47,13 @@ describe('Basis Test', () => {
                         console.warn("Unkown rule type -> not recognized: ", rule.type);
                     }
                 }
+            } else {
+                if (criticalSelectorRules.has(rule.type)) {
+                    let count = criticalSelectorRules.get(rule.type);
+                    criticalSelectorRules.set(rule.type, ++count);
+                } else {
+                    criticalSelectorRules.set(rule.type, 1);
+                }
             }
         }
 
@@ -270,5 +277,10 @@ describe('Basis Test', () => {
 
             expect(exists).toBeTruthy();
         });
+
+        test("Font-Face should be in critical css", () => {
+            const exists = criticalSelectorRules.has("font-face");
+            expect(exists).toBeTruthy();
+        });
     });
 });

test/data/test.css

@@ -1,5 +1,9 @@
 @charset "UTF-8";
 
+@font-face {
+    font-family: Keep Critical;
+}
+
 .standard-selector {
     color: black;
 }