diff --git a/src/index.js b/src/index.js index 0aca912..fc8f8ae 100644 --- a/src/index.js +++ b/src/index.js @@ -143,14 +143,14 @@ export default class Critters { // `external:false` skips processing of external sheets if (this.options.external !== false) { - const externalSheets = document.querySelectorAll('link[rel="stylesheet"]'); + const externalSheets = [].slice.call(document.querySelectorAll('link[rel="stylesheet"]')); await Promise.all(externalSheets.map( link => this.embedLinkedStylesheet(link, compilation, outputPath) )); } // go through all the style tags in the document and reduce them to only critical CSS - const styles = document.querySelectorAll('style'); + const styles = [].slice.call(document.querySelectorAll('style')); await Promise.all(styles.map( style => this.processStyle(style, document) )); @@ -252,7 +252,7 @@ export default class Critters { const head = document.querySelector('head'); // basically `.textContent` - let sheet = style.childNodes.length > 0 && style.childNodes.map(node => node.nodeValue).join('\n'); + let sheet = style.childNodes.length > 0 && [].slice.call(style.childNodes).map(node => node.nodeValue).join('\n'); // store a reference to the previous serialized stylesheet for reporting stats const before = sheet; @@ -272,8 +272,12 @@ export default class Critters { rule.selectors = rule.selectors.filter(sel => { // Strip pseudo-elements and pseudo-classes, since we only care that their associated elements exist. // This means any selector for a pseudo-element or having a pseudo-class will be inlined if the rest of the selector matches. - sel = sel.replace(/::?(?:[a-z-]+)([.[#~&^:*]|\s|\n|$)/gi, '$1'); - return document.querySelector(sel, document) != null; + sel = sel.replace(/::?(?:[a-z-]+)([.[#~&^:*]|\s|\n|$)/gi, '$1').trim(); + try { + return document.querySelector(sel, document) != null; + } catch (e) { + return null; + } }); // If there are no matched selectors, remove the rule: if (rule.selectors.length === 0) { @@ -353,3 +357,4 @@ export default class Critters { console.log('\u001b[32mCritters: inlined ' + prettyBytes(sheet.length) + ' (' + percent + '% of original ' + prettyBytes(before.length) + ') of ' + name + '.\u001b[39m'); } } + -- 2.51.2 From d8f80ae0d895c24f20b350f63f1b996fc621f9c0 Mon Sep 17 00:00:00 2001 From: Prateek Bhatnagar Date: Fri, 14 Sep 2018 15:53:47 -0700 Subject: [PATCH 2/7] Update dom.js --- src/dom.js | 203 ++--------------------------------------------------- 1 file changed, 6 insertions(+), 197 deletions(-) diff --git a/src/dom.js b/src/dom.js index af18a68..ee04f07 100644 --- a/src/dom.js +++ b/src/dom.js @@ -13,16 +13,7 @@ * License for the specific language governing permissions and limitations under * the License. */ - -import parse5 from 'parse5'; -import nwmatcher from 'nwmatcher'; - -// htmlparser2 has a relatively DOM-like tree format, which we'll massage into a DOM elsewhere -const treeAdapter = parse5.treeAdapters.htmlparser2; - -const PARSE5_OPTS = { - treeAdapter -}; +import {JSDOM} from 'jsdom'; /** * Parse HTML into a mutable, serializable DOM Document. @@ -30,201 +21,19 @@ const PARSE5_OPTS = { * @param {String} html HTML to parse into a Document instance */ export function createDocument (html) { - const document = parse5.parse(html, PARSE5_OPTS); - - defineProperties(document, DocumentExtensions); - // Find the first element within the document - - // Extend Element.prototype with DOM manipulation methods. - // Note: document.$$scratchElement is also used by createTextNode() - const scratch = document.$$scratchElement = document.createElement('div'); - const elementProto = Object.getPrototypeOf(scratch); - defineProperties(elementProto, ElementExtensions); - elementProto.ownerDocument = document; - - // nwmatcher is a selector engine that happens to work with Parse5's htmlparser2 DOM (they form the base of jsdom). - // It is exposed to the document so that it can be used within Element.prototype methods. - document.$match = nwmatcher({ document }); - document.$match.configure({ - CACHING: false, - USE_QSAPI: false, - USE_HTML5: false + const { window } = new JSDOM(html, { + contentType: "text/html", }); + const document = window.document; + return document; } - /** * Serialize a Document to an HTML String * @param {Document} document A Document, such as one created via `createDocument()` */ export function serializeDocument (document) { - return parse5.serialize(document, PARSE5_OPTS); + return document.querySelector('html').innerHTML; } -/** - * Methods and descriptors to mix into Element.prototype - */ -const ElementExtensions = { - /** @extends htmlparser2.Element.prototype */ - - nodeName: { - get () { - return this.tagName.toUpperCase(); - } - }, - - id: reflectedProperty('id'), - - className: reflectedProperty('class'), - - insertBefore (child, referenceNode) { - if (!referenceNode) return this.appendChild(child); - treeAdapter.insertBefore(this, child, referenceNode); - return child; - }, - - appendChild (child) { - treeAdapter.appendChild(this, child); - return child; - }, - - removeChild (child) { - treeAdapter.detachNode(child); - }, - - setAttribute (name, value) { - if (this.attribs == null) this.attribs = {}; - if (value == null) value = ''; - this.attribs[name] = value; - }, - - removeAttribute (name) { - if (this.attribs != null) { - delete this.attribs[name]; - } - }, - - getAttribute (name) { - return this.attribs != null && this.attribs[name]; - }, - - hasAttribute (name) { - return this.attribs != null && this.attribs[name] != null; - }, - - getAttributeNode (name) { - const value = this.getAttribute(name); - if (value != null) return { specified: true, value }; - }, - - getElementsByTagName -}; - -/** - * Methods and descriptors to mix into the global document instance - * @private - */ -const DocumentExtensions = { - /** @extends htmlparser2.Document.prototype */ - - // document is just an Element in htmlparser2, giving it a nodeType of ELEMENT_NODE. - // nwmatcher requires that it at least report a correct nodeType of DOCUMENT_NODE. - nodeType: { - get () { - return 9; - } - }, - - nodeName: { - get () { - return '#document'; - } - }, - - documentElement: { - get () { - // Find the first element within the document - return this.childNodes.filter(child => String(child.tagName).toLowerCase() === 'html')[0]; - } - }, - - body: { - get () { - return this.querySelector('body'); - } - }, - - createElement (name) { - return treeAdapter.createElement(name, null, []); - }, - - createTextNode (text) { - // there is no dedicated createTextNode equivalent in htmlparser2's DOM, so - // we have to insert Text and then remove and return the resulting Text node. - const scratch = this.$$scratchElement; - treeAdapter.insertText(scratch, text); - const node = scratch.lastChild; - treeAdapter.detachNode(node); - return node; - }, - - querySelector (sel) { - return this.$match.first(sel, this.documentElement); - }, - - querySelectorAll (sel) { - return this.$match.select(sel, this.documentElement); - }, - - getElementsByTagName, - - // Bugfix: nwmatcher uses inexistence of `document.addEventListener` to detect IE: - // @see https://github.com/dperini/nwmatcher/blob/3edb471e12ce7f7d46dc1606c7f659ff45675a29/src/nwmatcher.js#L353 - addEventListener: Object -}; - -/** - * Essentially `Object.defineProperties()`, except function values are assigned as value descriptors for convenience. - * @private - */ -function defineProperties (obj, properties) { - for (const i in properties) { - const value = properties[i]; - Object.defineProperty(obj, i, typeof value === 'function' ? { value } : value); - } -} - -/** - * A simple implementation of Element.prototype.getElementsByTagName(). - * This is the only tree traversal method nwmatcher uses to implement its selector engine. - * @private - * @note - * If perf issues arise, 2 faster but more verbose implementations are benchmarked here: - * https://esbench.com/bench/5ac3b647f2949800a0f619e1 - */ -function getElementsByTagName (tagName) { - // Only return Element/Document nodes - if ((this.nodeType !== 1 && this.nodeType !== 9) || this.type === 'directive') return []; - return Array.prototype.concat.apply( - // Add current element if it matches tag - (tagName === '*' || (this.tagName && (this.tagName === tagName || this.nodeName === tagName.toUpperCase()))) ? [this] : [], - // Check children recursively - this.children.map(child => getElementsByTagName.call(child, tagName)) - ); -} - -/** - * Create a property descriptor defining a getter/setter pair alias for a named attribute. - * @private - */ -function reflectedProperty (attributeName) { - return { - get () { - return this.getAttribute(attributeName); - }, - set (value) { - this.setAttribute(attributeName, value); - } - }; -} -- 2.51.2 From a783d3816cd5fdc8ca9721df7770101a2218a9f7 Mon Sep 17 00:00:00 2001 From: Prateek Bhatnagar Date: Fri, 14 Sep 2018 15:54:06 -0700 Subject: [PATCH 3/7] Update package.json --- package.json | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/package.json b/package.json index 6f5b795..a600ded 100644 --- a/package.json +++ b/package.json @@ -60,14 +60,13 @@ "file-loader": "^1.1.11", "html-webpack-plugin": "^3.2.0", "jest": "^22.4.3", - "jsdom": "^11.9.0", + "jsdom": "^11.12.0", "microbundle": "^0.4.4", "mini-css-extract-plugin": "^0.4.0", "webpack": "^4.6.0" }, "dependencies": { "css": "^2.2.1", - "nwmatcher": "^1.4.4", "parse5": "^4.0.0", "pretty-bytes": "^4.0.2", "webpack-sources": "^1.1.0" @@ -94,3 +93,4 @@ } } } + -- 2.51.2 From 54cc5cc2076187c9d6b3c5ecc0de687487a3cec8 Mon Sep 17 00:00:00 2001 From: Jason Miller Date: Sun, 16 Sep 2018 16:46:19 -0400 Subject: [PATCH 4/7] Use jsdom.serialize() --- src/dom.js | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/dom.js b/src/dom.js index ee04f07..740280f 100644 --- a/src/dom.js +++ b/src/dom.js @@ -21,12 +21,12 @@ import {JSDOM} from 'jsdom'; * @param {String} html HTML to parse into a Document instance */ export function createDocument (html) { - const { window } = new JSDOM(html, { + const jsdom = new JSDOM(html, { contentType: "text/html", }); - + const { window } = jsdom; const document = window.document; - + document.$jsdom = jsdom; return document; } /** @@ -34,6 +34,6 @@ export function createDocument (html) { * @param {Document} document A Document, such as one created via `createDocument()` */ export function serializeDocument (document) { - return document.querySelector('html').innerHTML; + return document.$jsdom.serialize(); } -- 2.51.2 From cfa6ce24fc9b27c45e220ed4262ed32845963cd2 Mon Sep 17 00:00:00 2001 From: Jason Miller Date: Sun, 16 Sep 2018 16:47:57 -0400 Subject: [PATCH 5/7] shortcut array clone --- src/index.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index fc8f8ae..d28b685 100644 --- a/src/index.js +++ b/src/index.js @@ -252,7 +252,7 @@ export default class Critters { const head = document.querySelector('head'); // basically `.textContent` - let sheet = style.childNodes.length > 0 && [].slice.call(style.childNodes).map(node => node.nodeValue).join('\n'); + let sheet = style.childNodes.length > 0 && [].map.call(style.childNodes, node => node.nodeValue).join('\n'); // store a reference to the previous serialized stylesheet for reporting stats const before = sheet; -- 2.51.2 From 374f3a98173c798688c1e3786a4489fd6dff11b2 Mon Sep 17 00:00:00 2001 From: Jason Miller Date: Sun, 16 Sep 2018 16:55:24 -0400 Subject: [PATCH 6/7] Add logging for failed CSS selectors --- src/index.js | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/src/index.js b/src/index.js index d28b685..916ce20 100644 --- a/src/index.js +++ b/src/index.js @@ -264,6 +264,8 @@ export default class Critters { // a string to search for font names (very loose) let criticalFonts = ''; + + const failedSelectors = []; // Walk all CSS rules, transforming unused rules to comments (which get removed) walkStyleRules(ast, rule => { @@ -274,8 +276,9 @@ export default class Critters { // This means any selector for a pseudo-element or having a pseudo-class will be inlined if the rest of the selector matches. sel = sel.replace(/::?(?:[a-z-]+)([.[#~&^:*]|\s|\n|$)/gi, '$1').trim(); try { - return document.querySelector(sel, document) != null; + return document.querySelector(sel) != null; } catch (e) { + failedSelectors.push(sel + ' -> ' + e.message); return null; } }); @@ -300,6 +303,13 @@ export default class Critters { // If there are no remaining rules, remove the whole rule: return !rule.rules || rule.rules.length !== 0; }); + + if (failedSelectors.length !== 0) { + console.warn( + `${failedSelectors.length} rules skipped due to selector errors:\n `+ + failedSelectors.join('\n ') + ); + } const shouldPreloadFonts = options.fonts === true || options.preloadFonts === true; const shouldInlineFonts = options.fonts !== false || options.inlineFonts === true; -- 2.51.2 From 0368d22d085569a79e0655f95411322256b1a5de Mon Sep 17 00:00:00 2001 From: Jason Miller Date: Sun, 16 Sep 2018 21:09:35 +0000 Subject: [PATCH 7/7] Fix JSDOM + Jest incompat issue --- package.json | 1 + src/dom.js | 5 ++--- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/package.json b/package.json index a600ded..0370044 100644 --- a/package.json +++ b/package.json @@ -33,6 +33,7 @@ }, "jest": { "testEnvironment": "jsdom", + "testURL": "http://localhost", "coverageReporters": [ "text" ], diff --git a/src/dom.js b/src/dom.js index 740280f..15d249a 100644 --- a/src/dom.js +++ b/src/dom.js @@ -13,7 +13,7 @@ * License for the specific language governing permissions and limitations under * the License. */ -import {JSDOM} from 'jsdom'; +import { JSDOM } from 'jsdom'; /** * Parse HTML into a mutable, serializable DOM Document. @@ -22,7 +22,7 @@ import {JSDOM} from 'jsdom'; */ export function createDocument (html) { const jsdom = new JSDOM(html, { - contentType: "text/html", + contentType: 'text/html' }); const { window } = jsdom; const document = window.document; @@ -36,4 +36,3 @@ export function createDocument (html) { export function serializeDocument (document) { return document.$jsdom.serialize(); } -