Skip to content
This repository has been archived by the owner on Oct 25, 2024. It is now read-only.

Commit

Permalink
refactor: couple of minor refactors (#158)
Browse files Browse the repository at this point in the history 
- Reduce checks where possible
- Use Sets instead of Array for quicker access
- Remove `pretty-bytes` dependency.
- Use for of loops
  • Loading branch information
@alan-agius4
alan-agius4 authored Mar 4, 2024
1 parent df5666d commit 01eeab4
Show file tree
Hide file tree
Showing 4 changed files with 131 additions and 137 deletions.
3 changes: 1 addition & 2 deletions packages/critters/package.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,7 @@
"domhandler": "^5.0.2",
"htmlparser2": "^8.0.2",
"postcss": "^8.4.23",
"postcss-media-query-parser": "^0.2.3",
"pretty-bytes": "^5.3.0"
"postcss-media-query-parser": "^0.2.3"
},
"devDependencies": {
"documentation": "^13.0.2",
Expand Down
43 changes: 19 additions & 24 deletions packages/critters/src/css.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,10 @@ export function walkStyleRulesWithReverseMirror(node, node2, iterator) {
// @keyframes are an exception since they are evaluated as a whole
function hasNestedRules(rule) {
return (
rule.nodes &&
rule.nodes.length &&
rule.nodes.some((n) => n.type === 'rule' || n.type === 'atrule') &&
rule.nodes?.length &&
rule.name !== 'keyframes' &&
rule.name !== '-webkit-keyframes'
rule.name !== '-webkit-keyframes' &&
rule.nodes.some((n) => n.type === 'rule' || n.type === 'atrule')
);
}

Expand Down Expand Up @@ -199,33 +198,29 @@ function filterSelectors(predicate) {

const MEDIA_TYPES = new Set(['all', 'print', 'screen', 'speech']);
const MEDIA_KEYWORDS = new Set(['and', 'not', ',']);
const MEDIA_FEATURES = [
'width',
'aspect-ratio',
'color',
'color-index',
'grid',
'height',
'monochrome',
'orientation',
'resolution',
'scan'
];
const MEDIA_FEATURES = new Set(
[
'width',
'aspect-ratio',
'color',
'color-index',
'grid',
'height',
'monochrome',
'orientation',
'resolution',
'scan'
].flatMap((feature) => [feature, `min-${feature}`, `max-${feature}`])
);

function validateMediaType(node) {
const { type: nodeType, value: nodeValue } = node;

if (nodeType === 'media-type') {
return MEDIA_TYPES.has(nodeValue);
} else if (nodeType === 'keyword') {
return MEDIA_KEYWORDS.has(nodeValue);
} else if (nodeType === 'media-feature') {
return MEDIA_FEATURES.some((feature) => {
return (
nodeValue === feature ||
nodeValue === `min-${feature}` ||
nodeValue === `max-${feature}`
);
});
return MEDIA_FEATURES.has(nodeValue);
}
}

Expand Down
77 changes: 45 additions & 32 deletions packages/critters/src/index.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

import path from 'path';
import { readFile } from 'fs';
import prettyBytes from 'pretty-bytes';
import { createDocument, serializeDocument } from './dom';
import {
parseStylesheet,
Expand Down Expand Up @@ -241,7 +240,7 @@ export default class Critters {
// path on disk (with output.publicPath removed)
let normalizedPath = href.replace(/^\//, '');
const pathPrefix = (publicPath || '').replace(/(^\/|\/$)/g, '') + '/';
if (normalizedPath.indexOf(pathPrefix) === 0) {
if (normalizedPath.startsWith(pathPrefix)) {
normalizedPath = normalizedPath
.substring(pathPrefix.length)
.replace(/^\//, '');
Expand Down Expand Up @@ -325,7 +324,7 @@ export default class Critters {
const preloadMode = this.options.preload;

// skip filtered resources, or network resources if no filter is provided
if (this.urlFilter ? this.urlFilter(href) : !(href || '').match(/\.css$/)) {
if (this.urlFilter ? this.urlFilter(href) : !href?.endsWith('.css')) {
return Promise.resolve();
}

Expand Down Expand Up @@ -478,13 +477,18 @@ export default class Critters {

const failedSelectors = [];

const criticalKeyframeNames = [];
const criticalKeyframeNames = new Set();

let includeNext = false;
let includeAll = false;
let excludeNext = false;
let excludeAll = false;

const shouldPreloadFonts =
options.fonts === true || options.preloadFonts === true;
const shouldInlineFonts =
options.fonts !== false && options.inlineFonts === true;

// Walk all CSS rules, marking unused rules with `.$$remove=true` for removal in the second pass.
// This first pass is also used to collect font and keyframe usage used in the second pass.
walkStyleRules(
Expand Down Expand Up @@ -553,9 +557,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.
if (
sel === ':root' ||
sel.match(/^::?(before|after)$/) ||
sel === 'html' ||
sel === 'body'
sel === 'body' ||
/^::?(before|after)$/.test(sel)
) {
return true;
}
Expand All @@ -582,21 +586,22 @@ export default class Critters {
}

if (rule.nodes) {
for (let i = 0; i < rule.nodes.length; i++) {
const decl = rule.nodes[i];

for (const decl of rule.nodes) {
// detect used fonts
if (decl.prop && decl.prop.match(/\bfont(-family)?\b/i)) {
if (
shouldInlineFonts &&
decl.prop &&
/\bfont(-family)?\b/i.test(decl.prop)
) {
criticalFonts += ' ' + decl.value;
}

// detect used keyframes
if (decl.prop === 'animation' || decl.prop === 'animation-name') {
// @todo: parse animation declarations and extract only the name. for now we'll do a lazy match.
const names = decl.value.split(/\s+/);
for (let j = 0; j < names.length; j++) {
const name = names[j].trim();
if (name) criticalKeyframeNames.push(name);
for (const name of decl.value.split(/\s+/)) {
// @todo: parse animation declarations and extract only the name. for now we'll do a lazy match.
const nameTrimmed = name.trim();
if (nameTrimmed) criticalKeyframeNames.add(nameTrimmed);
}
}
}
Expand All @@ -607,7 +612,7 @@ export default class Critters {
if (rule.type === 'atrule' && rule.name === 'font-face') return;

// If there are no remaining rules, remove the whole rule:
const rules = rule.nodes && rule.nodes.filter((rule) => !rule.$$remove);
const rules = rule.nodes?.filter((rule) => !rule.$$remove);
return !rules || rules.length !== 0;
})
);
Expand All @@ -622,12 +627,7 @@ export default class Critters {
);
}

const shouldPreloadFonts =
options.fonts === true || options.preloadFonts === true;
const shouldInlineFonts =
options.fonts !== false && options.inlineFonts === true;

const preloadedFonts = [];
const preloadedFonts = new Set();
// Second pass, using data picked up from the first
walkStyleRulesWithReverseMirror(ast, astInverse, (rule) => {
// remove any rules marked in the first pass
Expand All @@ -639,14 +639,13 @@ export default class Critters {
if (rule.type === 'atrule' && rule.name === 'keyframes') {
if (keyframesMode === 'none') return false;
if (keyframesMode === 'all') return true;
return criticalKeyframeNames.indexOf(rule.params) !== -1;
return criticalKeyframeNames.has(rule.params);
}

// prune @font-face rules
if (rule.type === 'atrule' && rule.name === 'font-face') {
let family, src;
for (let i = 0; i < rule.nodes.length; i++) {
const decl = rule.nodes[i];
for (const decl of rule.nodes) {
if (decl.prop === 'src') {
// @todo parse this properly and generate multiple preloads with type="font/woff2" etc
src = (decl.value.match(/url\s*\(\s*(['"]?)(.+?)\1\s*\)/) || [])[2];
Expand All @@ -655,8 +654,8 @@ export default class Critters {
}
}

if (src && shouldPreloadFonts && preloadedFonts.indexOf(src) === -1) {
preloadedFonts.push(src);
if (src && shouldPreloadFonts && !preloadedFonts.has(src)) {
preloadedFonts.add(src);
const preload = document.createElement('link');
preload.setAttribute('rel', 'preload');
preload.setAttribute('as', 'font');
Expand All @@ -667,10 +666,10 @@ export default class Critters {

// if we're missing info, if the font is unused, or if critical font inlining is disabled, remove the rule:
if (
!shouldInlineFonts ||
!family ||
!src ||
criticalFonts.indexOf(family) === -1 ||
!shouldInlineFonts
!criticalFonts.includes(family)
) {
return false;
}
Expand Down Expand Up @@ -702,7 +701,7 @@ export default class Critters {
const percent = (sheetInverse.length / before.length) * 100;
afterText = `, reducing non-inlined size ${
percent | 0
}% to ${prettyBytes(sheetInverse.length)}`;
}% to ${formatSize(sheetInverse.length)}`;
}
}

Expand All @@ -715,15 +714,29 @@ export default class Critters {
const percent = ((sheet.length / before.length) * 100) | 0;
this.logger.info(
'\u001b[32mInlined ' +
prettyBytes(sheet.length) +
formatSize(sheet.length) +
' (' +
percent +
'% of original ' +
prettyBytes(before.length) +
formatSize(before.length) +
') of ' +
name +
afterText +
'.\u001b[39m'
);
}
}

function formatSize(size) {
if (size <= 0) {
return '0 bytes';
}

const abbreviations = ['bytes', 'kB', 'MB', 'GB'];
const index = Math.floor(Math.log(size) / Math.log(1024));
const roundedSize = size / Math.pow(1024, index);
// bytes don't have a fraction
const fractionDigits = index === 0 ? 0 : 2;

return `${roundedSize.toFixed(fractionDigits)} ${abbreviations[index]}`;
}
Loading

0 comments on commit 01eeab4

Please sign in to comment.