From 75149ae85625f5ce546c038897016f8c94f5d1c6 Mon Sep 17 00:00:00 2001 From: "Alex N. Jose" Date: Wed, 16 Nov 2022 11:33:20 -0800 Subject: [PATCH] core: refactor audits to use async syntax for improved readability --- .../byte-efficiency/uses-long-cache-ttl.js | 175 +++++++++--------- core/audits/critical-request-chains.js | 83 ++++----- core/audits/is-on-https.js | 77 ++++---- core/audits/seo/is-crawlable.js | 105 +++++------ core/audits/user-timings.js | 85 +++++---- 5 files changed, 259 insertions(+), 266 deletions(-) diff --git a/core/audits/byte-efficiency/uses-long-cache-ttl.js b/core/audits/byte-efficiency/uses-long-cache-ttl.js index 7e296dc314e9..b0dbdda07887 100644 --- a/core/audits/byte-efficiency/uses-long-cache-ttl.js +++ b/core/audits/byte-efficiency/uses-long-cache-ttl.js @@ -194,103 +194,102 @@ class CacheHeaders extends Audit { * @param {LH.Audit.Context} context * @return {Promise} */ - static audit(artifacts, context) { + static async audit(artifacts, context) { const devtoolsLogs = artifacts.devtoolsLogs[Audit.DEFAULT_PASS]; - return NetworkRecords.request(devtoolsLogs, context).then(records => { - const results = []; - let totalWastedBytes = 0; - - for (const record of records) { - if (!CacheHeaders.isCacheableAsset(record)) continue; - - /** @type {Map} */ - const headers = new Map(); - for (const header of record.responseHeaders || []) { - if (headers.has(header.name.toLowerCase())) { - const previousHeaderValue = headers.get(header.name.toLowerCase()); - headers.set(header.name.toLowerCase(), - `${previousHeaderValue}, ${header.value}`); - } else { - headers.set(header.name.toLowerCase(), header.value); - } - } - - const cacheControl = parseCacheControl(headers.get('cache-control')); - if (this.shouldSkipRecord(headers, cacheControl)) { - continue; + const records = await NetworkRecords.request(devtoolsLogs, context); + const results = []; + let totalWastedBytes = 0; + + for (const record of records) { + if (!CacheHeaders.isCacheableAsset(record)) continue; + + /** @type {Map} */ + const headers = new Map(); + for (const header of record.responseHeaders || []) { + if (headers.has(header.name.toLowerCase())) { + const previousHeaderValue = headers.get(header.name.toLowerCase()); + headers.set(header.name.toLowerCase(), + `${previousHeaderValue}, ${header.value}`); + } else { + headers.set(header.name.toLowerCase(), header.value); } + } - // Ignore if cacheLifetimeInSeconds is a nonpositive number. - let cacheLifetimeInSeconds = CacheHeaders.computeCacheLifetimeInSeconds( - headers, cacheControl); - if (cacheLifetimeInSeconds !== null && - (!Number.isFinite(cacheLifetimeInSeconds) || cacheLifetimeInSeconds <= 0)) { - continue; - } - cacheLifetimeInSeconds = cacheLifetimeInSeconds || 0; - - // Ignore assets whose cache lifetime is already high enough - const cacheHitProbability = CacheHeaders.getCacheHitProbability(cacheLifetimeInSeconds); - if (cacheHitProbability > IGNORE_THRESHOLD_IN_PERCENT) continue; - - const url = UrlUtils.elideDataURI(record.url); - const totalBytes = record.transferSize || 0; - const wastedBytes = (1 - cacheHitProbability) * totalBytes; - - totalWastedBytes += wastedBytes; - - // Include cacheControl info (if it exists) per url as a diagnostic. - /** @type {LH.Audit.Details.DebugData|undefined} */ - let debugData; - if (cacheControl) { - debugData = { - type: 'debugdata', - ...cacheControl, - }; - } + const cacheControl = parseCacheControl(headers.get('cache-control')); + if (this.shouldSkipRecord(headers, cacheControl)) { + continue; + } - results.push({ - url, - debugData, - cacheLifetimeMs: cacheLifetimeInSeconds * 1000, - cacheHitProbability, - totalBytes, - wastedBytes, - }); + // Ignore if cacheLifetimeInSeconds is a nonpositive number. + let cacheLifetimeInSeconds = CacheHeaders.computeCacheLifetimeInSeconds( + headers, cacheControl); + if (cacheLifetimeInSeconds !== null && + (!Number.isFinite(cacheLifetimeInSeconds) || cacheLifetimeInSeconds <= 0)) { + continue; + } + cacheLifetimeInSeconds = cacheLifetimeInSeconds || 0; + + // Ignore assets whose cache lifetime is already high enough + const cacheHitProbability = CacheHeaders.getCacheHitProbability(cacheLifetimeInSeconds); + if (cacheHitProbability > IGNORE_THRESHOLD_IN_PERCENT) continue; + + const url = UrlUtils.elideDataURI(record.url); + const totalBytes = record.transferSize || 0; + const wastedBytes = (1 - cacheHitProbability) * totalBytes; + + totalWastedBytes += wastedBytes; + + // Include cacheControl info (if it exists) per url as a diagnostic. + /** @type {LH.Audit.Details.DebugData|undefined} */ + let debugData; + if (cacheControl) { + debugData = { + type: 'debugdata', + ...cacheControl, + }; } - results.sort((a, b) => { - return a.cacheLifetimeMs - b.cacheLifetimeMs || - b.totalBytes - a.totalBytes || - a.url.localeCompare(b.url); + results.push({ + url, + debugData, + cacheLifetimeMs: cacheLifetimeInSeconds * 1000, + cacheHitProbability, + totalBytes, + wastedBytes, }); + } - const score = Audit.computeLogNormalScore( - {p10: context.options.p10, median: context.options.median}, - totalWastedBytes - ); - - /** @type {LH.Audit.Details.Table['headings']} */ - const headings = [ - {key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)}, - // TODO(i18n): pre-compute localized duration - {key: 'cacheLifetimeMs', valueType: 'ms', label: str_(i18n.UIStrings.columnCacheTTL), - displayUnit: 'duration'}, - {key: 'totalBytes', valueType: 'bytes', label: str_(i18n.UIStrings.columnTransferSize), - displayUnit: 'kb', granularity: 1}, - ]; - - const summary = {wastedBytes: totalWastedBytes}; - const details = Audit.makeTableDetails(headings, results, summary); - - return { - score, - numericValue: totalWastedBytes, - numericUnit: 'byte', - displayValue: str_(UIStrings.displayValue, {itemCount: results.length}), - details, - }; + results.sort((a, b) => { + return a.cacheLifetimeMs - b.cacheLifetimeMs || + b.totalBytes - a.totalBytes || + a.url.localeCompare(b.url); }); + + const score = Audit.computeLogNormalScore( + {p10: context.options.p10, median: context.options.median}, + totalWastedBytes + ); + + /** @type {LH.Audit.Details.Table['headings']} */ + const headings = [ + {key: 'url', valueType: 'url', label: str_(i18n.UIStrings.columnURL)}, + // TODO(i18n): pre-compute localized duration + {key: 'cacheLifetimeMs', valueType: 'ms', label: str_(i18n.UIStrings.columnCacheTTL), + displayUnit: 'duration'}, + {key: 'totalBytes', valueType: 'bytes', label: str_(i18n.UIStrings.columnTransferSize), + displayUnit: 'kb', granularity: 1}, + ]; + + const summary = {wastedBytes: totalWastedBytes}; + const details = Audit.makeTableDetails(headings, results, summary); + + return { + score, + numericValue: totalWastedBytes, + numericUnit: 'byte', + displayValue: str_(UIStrings.displayValue, {itemCount: results.length}), + details, + }; } } diff --git a/core/audits/critical-request-chains.js b/core/audits/critical-request-chains.js index 4ea6e44759b1..653e7add86e5 100644 --- a/core/audits/critical-request-chains.js +++ b/core/audits/critical-request-chains.js @@ -166,52 +166,51 @@ class CriticalRequestChains extends Audit { * @param {LH.Audit.Context} context * @return {Promise} */ - static audit(artifacts, context) { + static async audit(artifacts, context) { const trace = artifacts.traces[Audit.DEFAULT_PASS]; const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS]; const URL = artifacts.URL; - return ComputedChains.request({devtoolsLog, trace, URL}, context).then(chains => { - let chainCount = 0; - /** - * @param {LH.Audit.Details.SimpleCriticalRequestNode} node - * @param {number} depth - */ - function walk(node, depth) { - const childIds = Object.keys(node); - - childIds.forEach(id => { - const child = node[id]; - if (child.children) { - walk(child.children, depth + 1); - } else { - // if the node doesn't have a children field, then it is a leaf, so +1 - chainCount++; - } - }, ''); - } - // Convert - const flattenedChains = CriticalRequestChains.flattenRequests(chains); - - // Account for initial navigation - const initialNavKey = Object.keys(flattenedChains)[0]; - const initialNavChildren = initialNavKey && flattenedChains[initialNavKey].children; - if (initialNavChildren && Object.keys(initialNavChildren).length > 0) { - walk(initialNavChildren, 0); - } + const chains = await ComputedChains.request({devtoolsLog, trace, URL}, context); + let chainCount = 0; + /** + * @param {LH.Audit.Details.SimpleCriticalRequestNode} node + * @param {number} depth + */ + function walk(node, depth) { + const childIds = Object.keys(node); - const longestChain = CriticalRequestChains._getLongestChain(flattenedChains); - - return { - score: Number(chainCount === 0), - notApplicable: chainCount === 0, - displayValue: chainCount ? str_(UIStrings.displayValue, {itemCount: chainCount}) : '', - details: { - type: 'criticalrequestchain', - chains: flattenedChains, - longestChain, - }, - }; - }); + childIds.forEach(id => { + const child = node[id]; + if (child.children) { + walk(child.children, depth + 1); + } else { + // if the node doesn't have a children field, then it is a leaf, so +1 + chainCount++; + } + }, ''); + } + // Convert + const flattenedChains = CriticalRequestChains.flattenRequests(chains); + + // Account for initial navigation + const initialNavKey = Object.keys(flattenedChains)[0]; + const initialNavChildren = initialNavKey && flattenedChains[initialNavKey].children; + if (initialNavChildren && Object.keys(initialNavChildren).length > 0) { + walk(initialNavChildren, 0); + } + + const longestChain = CriticalRequestChains._getLongestChain(flattenedChains); + + return { + score: Number(chainCount === 0), + notApplicable: chainCount === 0, + displayValue: chainCount ? str_(UIStrings.displayValue, {itemCount: chainCount}) : '', + details: { + type: 'criticalrequestchain', + chains: flattenedChains, + longestChain, + }, + }; } } diff --git a/core/audits/is-on-https.js b/core/audits/is-on-https.js index 24230fb6ab03..10325ddc7f98 100644 --- a/core/audits/is-on-https.js +++ b/core/audits/is-on-https.js @@ -70,53 +70,52 @@ class HTTPS extends Audit { * @param {LH.Audit.Context} context * @return {Promise} */ - static audit(artifacts, context) { + static async audit(artifacts, context) { const devtoolsLogs = artifacts.devtoolsLogs[Audit.DEFAULT_PASS]; - return NetworkRecords.request(devtoolsLogs, context).then(networkRecords => { - const insecureURLs = networkRecords - .filter(record => !NetworkRequest.isSecureRequest(record)) - .map(record => UrlUtils.elideDataURI(record.url)); + const networkRecords = await NetworkRecords.request(devtoolsLogs, context); + const insecureURLs = networkRecords + .filter(record => !NetworkRequest.isSecureRequest(record)) + .map(record => UrlUtils.elideDataURI(record.url)); - /** @type {Array<{url: string, resolution?: LH.IcuMessage|string}>} */ - const items = Array.from(new Set(insecureURLs)).map(url => ({url, resolution: undefined})); + /** @type {Array<{url: string, resolution?: LH.IcuMessage|string}>} */ + const items = Array.from(new Set(insecureURLs)).map(url => ({url, resolution: undefined})); - /** @type {LH.Audit.Details.Table['headings']} */ - const headings = [ - {key: 'url', valueType: 'url', label: str_(UIStrings.columnInsecureURL)}, - {key: 'resolution', valueType: 'text', label: str_(UIStrings.columnResolution)}, - ]; + /** @type {LH.Audit.Details.Table['headings']} */ + const headings = [ + {key: 'url', valueType: 'url', label: str_(UIStrings.columnInsecureURL)}, + {key: 'resolution', valueType: 'text', label: str_(UIStrings.columnResolution)}, + ]; - for (const details of artifacts.InspectorIssues.mixedContentIssue) { - let item = items.find(item => item.url === details.insecureURL); - if (!item) { - item = {url: details.insecureURL}; - items.push(item); - } - item.resolution = resolutionToString[details.resolutionStatus] ? - str_(resolutionToString[details.resolutionStatus]) : - details.resolutionStatus; + for (const details of artifacts.InspectorIssues.mixedContentIssue) { + let item = items.find(item => item.url === details.insecureURL); + if (!item) { + item = {url: details.insecureURL}; + items.push(item); } + item.resolution = resolutionToString[details.resolutionStatus] ? + str_(resolutionToString[details.resolutionStatus]) : + details.resolutionStatus; + } - // If a resolution wasn't assigned from an InspectorIssue, then the item - // is not blocked by the browser but we've determined it is insecure anyhow. - // For example, if the URL is localhost, all `http` requests are valid - // (localhost is a secure context), but we still identify `http` requests - // as an "Allowed" insecure URL. - for (const item of items) { - if (!item.resolution) item.resolution = str_(UIStrings.allowed); - } + // If a resolution wasn't assigned from an InspectorIssue, then the item + // is not blocked by the browser but we've determined it is insecure anyhow. + // For example, if the URL is localhost, all `http` requests are valid + // (localhost is a secure context), but we still identify `http` requests + // as an "Allowed" insecure URL. + for (const item of items) { + if (!item.resolution) item.resolution = str_(UIStrings.allowed); + } - let displayValue; - if (items.length > 0) { - displayValue = str_(UIStrings.displayValue, {itemCount: items.length}); - } + let displayValue; + if (items.length > 0) { + displayValue = str_(UIStrings.displayValue, {itemCount: items.length}); + } - return { - score: Number(items.length === 0), - displayValue, - details: Audit.makeTableDetails(headings, items), - }; - }); + return { + score: Number(items.length === 0), + displayValue, + details: Audit.makeTableDetails(headings, items), + }; } } diff --git a/core/audits/seo/is-crawlable.js b/core/audits/seo/is-crawlable.js index fba22d530220..0792b6d731d2 100644 --- a/core/audits/seo/is-crawlable.js +++ b/core/audits/seo/is-crawlable.js @@ -90,63 +90,60 @@ class IsCrawlable extends Audit { * @param {LH.Audit.Context} context * @return {Promise} */ - static audit(artifacts, context) { + static async audit(artifacts, context) { const devtoolsLog = artifacts.devtoolsLogs[Audit.DEFAULT_PASS]; const metaRobots = artifacts.MetaElements.find(meta => meta.name === 'robots'); + const mainResource = await MainResource.request({devtoolsLog, URL: artifacts.URL}, context); + /** @type {LH.Audit.Details.Table['items']} */ + const blockingDirectives = []; + + if (metaRobots) { + const metaRobotsContent = metaRobots.content || ''; + const isBlocking = hasBlockingDirective(metaRobotsContent); + + if (isBlocking) { + blockingDirectives.push({ + source: { + ...Audit.makeNodeItem(metaRobots.node), + snippet: ``, + }, + }); + } + } + + mainResource.responseHeaders && mainResource.responseHeaders + .filter(h => h.name.toLowerCase() === ROBOTS_HEADER && !hasUserAgent(h.value) && + hasBlockingDirective(h.value)) + .forEach(h => blockingDirectives.push({source: `${h.name}: ${h.value}`})); + + if (artifacts.RobotsTxt.content) { + const robotsFileUrl = new URL('/robots.txt', mainResource.url); + const robotsTxt = robotsParser(robotsFileUrl.href, artifacts.RobotsTxt.content); + + if (!robotsTxt.isAllowed(mainResource.url)) { + const line = robotsTxt.getMatchingLineNumber(mainResource.url) || 1; + blockingDirectives.push({ + source: { + type: /** @type {const} */ ('source-location'), + url: robotsFileUrl.href, + urlProvider: /** @type {const} */ ('network'), + line: line - 1, + column: 0, + }, + }); + } + } + + /** @type {LH.Audit.Details.Table['headings']} */ + const headings = [ + {key: 'source', valueType: 'code', label: 'Blocking Directive Source'}, + ]; + const details = Audit.makeTableDetails(headings, blockingDirectives); - return MainResource.request({devtoolsLog, URL: artifacts.URL}, context) - .then(mainResource => { - /** @type {LH.Audit.Details.Table['items']} */ - const blockingDirectives = []; - - if (metaRobots) { - const metaRobotsContent = metaRobots.content || ''; - const isBlocking = hasBlockingDirective(metaRobotsContent); - - if (isBlocking) { - blockingDirectives.push({ - source: { - ...Audit.makeNodeItem(metaRobots.node), - snippet: ``, - }, - }); - } - } - - mainResource.responseHeaders && mainResource.responseHeaders - .filter(h => h.name.toLowerCase() === ROBOTS_HEADER && !hasUserAgent(h.value) && - hasBlockingDirective(h.value)) - .forEach(h => blockingDirectives.push({source: `${h.name}: ${h.value}`})); - - if (artifacts.RobotsTxt.content) { - const robotsFileUrl = new URL('/robots.txt', mainResource.url); - const robotsTxt = robotsParser(robotsFileUrl.href, artifacts.RobotsTxt.content); - - if (!robotsTxt.isAllowed(mainResource.url)) { - const line = robotsTxt.getMatchingLineNumber(mainResource.url) || 1; - blockingDirectives.push({ - source: { - type: /** @type {const} */ ('source-location'), - url: robotsFileUrl.href, - urlProvider: /** @type {const} */ ('network'), - line: line - 1, - column: 0, - }, - }); - } - } - - /** @type {LH.Audit.Details.Table['headings']} */ - const headings = [ - {key: 'source', valueType: 'code', label: 'Blocking Directive Source'}, - ]; - const details = Audit.makeTableDetails(headings, blockingDirectives); - - return { - score: Number(blockingDirectives.length === 0), - details, - }; - }); + return { + score: Number(blockingDirectives.length === 0), + details, + }; } } diff --git a/core/audits/user-timings.js b/core/audits/user-timings.js index a611bada12d2..d091f04ab882 100644 --- a/core/audits/user-timings.js +++ b/core/audits/user-timings.js @@ -64,54 +64,53 @@ class UserTimings extends Audit { * @param {LH.Audit.Context} context * @return {Promise} */ - static audit(artifacts, context) { + static async audit(artifacts, context) { const trace = artifacts.traces[Audit.DEFAULT_PASS]; - return ComputedUserTimings.request(trace, context).then(computedUserTimings => { - const userTimings = computedUserTimings.filter(UserTimings.excludeEvent); - const tableRows = userTimings.map(item => { - return { - name: item.name, - startTime: item.startTime, - duration: item.isMark ? undefined : item.duration, - timingType: item.isMark ? 'Mark' : 'Measure', - }; - }).sort((itemA, itemB) => { - if (itemA.timingType === itemB.timingType) { - // If both items are the same type, sort in ascending order by time - return itemA.startTime - itemB.startTime; - } else if (itemA.timingType === 'Measure') { - // Put measures before marks - return -1; - } else { - return 1; - } - }); + const computedUserTimings = await ComputedUserTimings.request(trace, context); + const userTimings = computedUserTimings.filter(UserTimings.excludeEvent); + const tableRows = userTimings.map(item => { + return { + name: item.name, + startTime: item.startTime, + duration: item.isMark ? undefined : item.duration, + timingType: item.isMark ? 'Mark' : 'Measure', + }; + }).sort((itemA, itemB) => { + if (itemA.timingType === itemB.timingType) { + // If both items are the same type, sort in ascending order by time + return itemA.startTime - itemB.startTime; + } else if (itemA.timingType === 'Measure') { + // Put measures before marks + return -1; + } else { + return 1; + } + }); - /** @type {LH.Audit.Details.Table['headings']} */ - const headings = [ - {key: 'name', valueType: 'text', label: str_(i18n.UIStrings.columnName)}, - {key: 'timingType', valueType: 'text', label: str_(UIStrings.columnType)}, - {key: 'startTime', valueType: 'ms', granularity: 0.01, - label: str_(i18n.UIStrings.columnStartTime)}, - {key: 'duration', valueType: 'ms', granularity: 0.01, - label: str_(i18n.UIStrings.columnDuration)}, - ]; + /** @type {LH.Audit.Details.Table['headings']} */ + const headings = [ + {key: 'name', valueType: 'text', label: str_(i18n.UIStrings.columnName)}, + {key: 'timingType', valueType: 'text', label: str_(UIStrings.columnType)}, + {key: 'startTime', valueType: 'ms', granularity: 0.01, + label: str_(i18n.UIStrings.columnStartTime)}, + {key: 'duration', valueType: 'ms', granularity: 0.01, + label: str_(i18n.UIStrings.columnDuration)}, + ]; - const details = Audit.makeTableDetails(headings, tableRows); + const details = Audit.makeTableDetails(headings, tableRows); - let displayValue; - if (userTimings.length) { - displayValue = str_(UIStrings.displayValue, {itemCount: userTimings.length}); - } + let displayValue; + if (userTimings.length) { + displayValue = str_(UIStrings.displayValue, {itemCount: userTimings.length}); + } - return { - // mark the audit as notApplicable if there were no user timings - score: Number(userTimings.length === 0), - notApplicable: userTimings.length === 0, - displayValue, - details, - }; - }); + return { + // mark the audit as notApplicable if there were no user timings + score: Number(userTimings.length === 0), + notApplicable: userTimings.length === 0, + displayValue, + details, + }; } }