From 2e242d6cb0a3c4f816175629de31726eda0d506c Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 12 Jan 2021 17:33:20 -0600 Subject: [PATCH] core: normalize creation of NodeValue (#11877) --- .../audits/accessibility/axe-audit.js | 8 +- lighthouse-core/audits/audit.js | 16 + lighthouse-core/audits/autocomplete.js | 6 +- .../audits/dobetterweb/dom-size.js | 16 +- .../external-anchors-use-rel-noopener.js | 8 +- .../password-inputs-can-be-pasted-into.js | 7 +- .../largest-contentful-paint-element.js | 10 +- .../audits/layout-shift-elements.js | 10 +- .../audits/non-composited-animations.js | 12 +- .../audits/seo/crawlable-anchors.js | 8 +- lighthouse-core/audits/seo/plugins.js | 20 +- lighthouse-core/audits/seo/tap-targets.js | 7 +- lighthouse-core/audits/unsized-images.js | 9 +- .../gather/gatherers/seo/embedded-content.js | 22 +- lighthouse-core/lib/page-functions.js | 9 +- .../test/audits/autocomplete-test.js | 24 +- .../test/audits/seo/plugins-test.js | 31 ++ lighthouse-core/test/results/sample_v2.json | 312 ++++++++++++------ types/artifacts.d.ts | 7 +- types/externs.d.ts | 2 +- 20 files changed, 305 insertions(+), 239 deletions(-) diff --git a/lighthouse-core/audits/accessibility/axe-audit.js b/lighthouse-core/audits/accessibility/axe-audit.js index e8d26570340f..d053031b9201 100644 --- a/lighthouse-core/audits/accessibility/axe-audit.js +++ b/lighthouse-core/audits/accessibility/axe-audit.js @@ -76,14 +76,8 @@ class AxeAudit extends Audit { if (rule && rule.nodes) { items = rule.nodes.map(axeNode => ({ node: { - type: /** @type {'node'} */ ('node'), - lhId: axeNode.node.lhId, - selector: axeNode.node.selector, - path: axeNode.node.devtoolsNodePath, - snippet: axeNode.node.snippet, - boundingRect: axeNode.node.boundingRect, + ...Audit.makeNodeItem(axeNode.node), explanation: axeNode.failureSummary, - nodeLabel: axeNode.node.nodeLabel, }, })); } diff --git a/lighthouse-core/audits/audit.js b/lighthouse-core/audits/audit.js index 2ec83e0fd72a..ad25fb83394b 100644 --- a/lighthouse-core/audits/audit.js +++ b/lighthouse-core/audits/audit.js @@ -216,6 +216,22 @@ class Audit { }; } + /** + * @param {LH.Artifacts.NodeDetails} node + * @return {LH.Audit.Details.NodeValue} + */ + static makeNodeItem(node) { + return { + type: 'node', + lhId: node.lhId, + path: node.devtoolsNodePath, + selector: node.selector, + boundingRect: node.boundingRect, + snippet: node.snippet, + nodeLabel: node.nodeLabel, + }; + } + /** * @param {number|null} score * @param {LH.Audit.ScoreDisplayMode} scoreDisplayMode diff --git a/lighthouse-core/audits/autocomplete.js b/lighthouse-core/audits/autocomplete.js index f1227df26da5..078de1b9fa71 100644 --- a/lighthouse-core/audits/autocomplete.js +++ b/lighthouse-core/audits/autocomplete.js @@ -260,11 +260,7 @@ class AutocompleteAudit extends Audit { continue; } failingFormsData.push({ - node: { - type: /** @type {'node'} */ ('node'), - snippet: input.node.snippet, - nodeLabel: input.node.nodeLabel, - }, + node: Audit.makeNodeItem(input.node), suggestion: suggestion, current: input.autocomplete.attribute, }); diff --git a/lighthouse-core/audits/dobetterweb/dom-size.js b/lighthouse-core/audits/dobetterweb/dom-size.js index acf7c4818b27..7ff253eae131 100644 --- a/lighthouse-core/audits/dobetterweb/dom-size.js +++ b/lighthouse-core/audits/dobetterweb/dom-size.js @@ -98,24 +98,12 @@ class DOMSize extends Audit { value: stats.totalBodyElements, }, { - node: { - type: /** @type {'node'} */ ('node'), - path: stats.depth.devtoolsNodePath, - snippet: stats.depth.snippet, - selector: stats.depth.selector, - nodeLabel: stats.depth.nodeLabel, - }, + node: Audit.makeNodeItem(stats.depth), statistic: str_(UIStrings.statisticDOMDepth), value: stats.depth.max, }, { - node: { - type: /** @type {'node'} */ ('node'), - path: stats.width.devtoolsNodePath, - snippet: stats.width.snippet, - selector: stats.width.selector, - nodeLabel: stats.width.nodeLabel, - }, + node: Audit.makeNodeItem(stats.width), statistic: str_(UIStrings.statisticDOMWidth), value: stats.width.max, }, diff --git a/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js b/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js index d55e7bcdab33..7b62c616f38e 100644 --- a/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js +++ b/lighthouse-core/audits/dobetterweb/external-anchors-use-rel-noopener.js @@ -69,13 +69,7 @@ class ExternalAnchorsUseRelNoopenerAudit extends Audit { }) .map(anchor => { return { - node: { - type: /** @type {'node'} */ ('node'), - path: anchor.node.devtoolsNodePath || '', - selector: anchor.node.selector || '', - nodeLabel: anchor.node.nodeLabel || '', - snippet: anchor.node.snippet || '', - }, + node: Audit.makeNodeItem(anchor.node), href: anchor.href || 'Unknown', target: anchor.target || '', rel: anchor.rel || '', diff --git a/lighthouse-core/audits/dobetterweb/password-inputs-can-be-pasted-into.js b/lighthouse-core/audits/dobetterweb/password-inputs-can-be-pasted-into.js index c160198bc853..fb08ef99071a 100644 --- a/lighthouse-core/audits/dobetterweb/password-inputs-can-be-pasted-into.js +++ b/lighthouse-core/audits/dobetterweb/password-inputs-can-be-pasted-into.js @@ -43,13 +43,10 @@ class PasswordInputsCanBePastedIntoAudit extends Audit { /** @type {LH.Audit.Details.Table['items']} */ const items = []; + passwordInputsWithPreventedPaste.forEach(input => { items.push({ - node: { - type: /** @type {'node'} */ ('node'), - snippet: input.node.snippet, - path: input.node.devtoolsNodePath, - }, + node: Audit.makeNodeItem(input.node), }); }); diff --git a/lighthouse-core/audits/largest-contentful-paint-element.js b/lighthouse-core/audits/largest-contentful-paint-element.js index 9f644617d6fc..c2b1642c8a95 100644 --- a/lighthouse-core/audits/largest-contentful-paint-element.js +++ b/lighthouse-core/audits/largest-contentful-paint-element.js @@ -42,15 +42,7 @@ class LargestContentfulPaintElement extends Audit { const lcpElementDetails = []; if (lcpElement) { lcpElementDetails.push({ - node: { - type: /** @type {'node'} */ ('node'), - lhId: lcpElement.node.lhId, - path: lcpElement.node.devtoolsNodePath, - selector: lcpElement.node.selector, - nodeLabel: lcpElement.node.nodeLabel, - snippet: lcpElement.node.snippet, - boundingRect: lcpElement.node.boundingRect, - }, + node: Audit.makeNodeItem(lcpElement.node), }); } diff --git a/lighthouse-core/audits/layout-shift-elements.js b/lighthouse-core/audits/layout-shift-elements.js index 1ca937bb779c..6d3f4f9bde5b 100644 --- a/lighthouse-core/audits/layout-shift-elements.js +++ b/lighthouse-core/audits/layout-shift-elements.js @@ -43,15 +43,7 @@ class LayoutShiftElements extends Audit { const clsElementData = clsElements.map(element => { return { - node: { - type: /** @type {'node'} */ ('node'), - lhId: element.node.lhId, - path: element.node.devtoolsNodePath, - selector: element.node.selector, - nodeLabel: element.node.nodeLabel, - snippet: element.node.snippet, - boundingRect: element.node.boundingRect, - }, + node: Audit.makeNodeItem(element.node), score: element.score, }; }); diff --git a/lighthouse-core/audits/non-composited-animations.js b/lighthouse-core/audits/non-composited-animations.js index f59f34b6f16c..b0691e4ed273 100644 --- a/lighthouse-core/audits/non-composited-animations.js +++ b/lighthouse-core/audits/non-composited-animations.js @@ -129,19 +129,11 @@ class NonCompositedAnimations extends Audit { }; } - /** @type LH.Audit.Details.TableItem[] */ + /** @type {LH.Audit.Details.TableItem[]} */ const results = []; let shouldAddAnimationNameColumn = false; artifacts.TraceElements.forEach(element => { if (element.traceEventType !== 'animation') return; - /** @type LH.Audit.Details.NodeValue */ - const node = { - type: 'node', - path: element.node.devtoolsNodePath, - selector: element.node.selector, - nodeLabel: element.node.nodeLabel, - snippet: element.node.snippet, - }; const animations = element.animations || []; const animationReasons = new Map(); @@ -171,7 +163,7 @@ class NonCompositedAnimations extends Audit { } results.push({ - node, + node: Audit.makeNodeItem(element.node), subItems: { type: 'subitems', items: allFailureReasons, diff --git a/lighthouse-core/audits/seo/crawlable-anchors.js b/lighthouse-core/audits/seo/crawlable-anchors.js index 9a721a18e4fd..53cb3760e0bc 100644 --- a/lighthouse-core/audits/seo/crawlable-anchors.js +++ b/lighthouse-core/audits/seo/crawlable-anchors.js @@ -81,13 +81,7 @@ class CrawlableAnchors extends Audit { /** @type {LH.Audit.Details.Table['items']} */ const itemsToDisplay = failingAnchors.map(anchor => { return { - node: { - type: 'node', - path: anchor.node.devtoolsNodePath || '', - selector: anchor.node.selector || '', - nodeLabel: anchor.node.nodeLabel || '', - snippet: anchor.node.snippet || '', - }, + node: Audit.makeNodeItem(anchor.node), }; }); diff --git a/lighthouse-core/audits/seo/plugins.js b/lighthouse-core/audits/seo/plugins.js index 5e1c275fe3ea..6b2d2c338a92 100644 --- a/lighthouse-core/audits/seo/plugins.js +++ b/lighthouse-core/audits/seo/plugins.js @@ -129,26 +129,8 @@ class Plugins extends Audit { return failingParams.length > 0; }) .map(plugin => { - const tagName = plugin.tagName.toLowerCase(); - /** @type {Array} */ - const attributeKeys = ['src', 'data', 'code', 'type']; - const attributes = attributeKeys - .reduce((result, attr) => { - if (plugin[attr] !== null) { - result += ` ${attr}="${plugin[attr]}"`; - } - return result; - }, ''); - const params = plugin.params - .filter(param => SOURCE_PARAMS.has(param.name.trim().toLowerCase())) - .map(param => ``) - .join(''); - return { - source: { - type: /** @type {'node'} */ ('node'), - snippet: `<${tagName}${attributes}>${params}`, - }, + source: Audit.makeNodeItem(plugin.node), }; }); diff --git a/lighthouse-core/audits/seo/tap-targets.js b/lighthouse-core/audits/seo/tap-targets.js index 3125dde3deac..cda6b5ffb1d8 100644 --- a/lighthouse-core/audits/seo/tap-targets.js +++ b/lighthouse-core/audits/seo/tap-targets.js @@ -248,13 +248,8 @@ function targetToTableNode(target) { const boundingRect = getBoundingRect(target.clientRects); return { - type: 'node', - lhId: target.node.lhId, - snippet: target.node.snippet, - path: target.node.devtoolsNodePath, - selector: target.node.selector, + ...Audit.makeNodeItem(target.node), boundingRect, - nodeLabel: target.node.nodeLabel, }; } diff --git a/lighthouse-core/audits/unsized-images.js b/lighthouse-core/audits/unsized-images.js index b7bd308fbca8..a953765cbfaa 100644 --- a/lighthouse-core/audits/unsized-images.js +++ b/lighthouse-core/audits/unsized-images.js @@ -96,15 +96,10 @@ class UnsizedImages extends Audit { if (isFixedImage || UnsizedImages.isSizedImage(image)) continue; const url = URL.elideDataURI(image.src); + unsizedImages.push({ url, - node: { - type: /** @type {'node'} */ ('node'), - path: image.node.devtoolsNodePath, - selector: image.node.selector, - nodeLabel: image.node.nodeLabel, - snippet: image.node.snippet, - }, + node: Audit.makeNodeItem(image.node), }); } diff --git a/lighthouse-core/gather/gatherers/seo/embedded-content.js b/lighthouse-core/gather/gatherers/seo/embedded-content.js index a4554d40d672..bbd64a1aa660 100644 --- a/lighthouse-core/gather/gatherers/seo/embedded-content.js +++ b/lighthouse-core/gather/gatherers/seo/embedded-content.js @@ -5,16 +5,24 @@ */ 'use strict'; -/* globals getElementsInDocument */ +/* globals getElementsInDocument getNodeDetails */ const Gatherer = require('../gatherer.js'); const pageFunctions = require('../../../lib/page-functions.js'); +/** + * @return {LH.Artifacts.EmbeddedContentInfo[]} + */ function getEmbeddedContent() { + const functions = /** @type {typeof pageFunctions} */ ({ + // @ts-expect-error - getElementsInDocument put into scope via stringification + getElementsInDocument, + // @ts-expect-error - getNodeDetails put into scope via stringification + getNodeDetails, + }); + const selector = 'object, embed, applet'; - /** @type {HTMLElement[]} */ - // @ts-expect-error - getElementsInDocument put into scope via stringification - const elements = getElementsInDocument(selector); + const elements = functions.getElementsInDocument(selector); return elements .map(node => ({ tagName: node.tagName, @@ -28,6 +36,7 @@ function getEmbeddedContent() { name: el.getAttribute('name') || '', value: el.getAttribute('value') || '', })), + node: functions.getNodeDetails(node), })); } @@ -39,7 +48,10 @@ class EmbeddedContent extends Gatherer { afterPass(passContext) { return passContext.driver.evaluate(getEmbeddedContent, { args: [], - deps: [pageFunctions.getElementsInDocument], + deps: [ + pageFunctions.getElementsInDocument, + pageFunctions.getNodeDetailsString, + ], }); } } diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index 8c9d62234b24..66dbc6feec8f 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -371,7 +371,7 @@ function isPositionFixed(element) { * strings like the innerText or alt attribute. * Falls back to the tagName if no useful label is found. * @param {Element} node - * @return {string|null} + * @return {string} */ /* istanbul ignore next */ function getNodeLabel(node) { @@ -461,8 +461,9 @@ function wrapRequestIdleCallback(cpuSlowdownMultiplier) { /** * @param {HTMLElement} element + * @return {LH.Artifacts.NodeDetails} */ -function getNodeDetailsImpl(element) { +function getNodeDetails(element) { // This bookkeeping is for the FullPageScreenshot gatherer. if (!window.__lighthouseNodesDontTouchOrAllVarianceGoesAway) { window.__lighthouseNodesDontTouchOrAllVarianceGoesAway = new Map(); @@ -507,8 +508,7 @@ const getNodeDetailsString = `function getNodeDetails(element) { ${getBoundingClientRect.toString()}; ${getOuterHTMLSnippet.toString()}; ${getNodeLabel.toString()}; - ${getNodeDetailsImpl.toString()}; - return getNodeDetailsImpl(element); + return (${getNodeDetails.toString()})(element); }`; module.exports = { @@ -522,6 +522,7 @@ module.exports = { computeBenchmarkIndex: computeBenchmarkIndex, computeBenchmarkIndexString: computeBenchmarkIndex.toString(), getNodeDetailsString, + getNodeDetails, getNodePathString: getNodePath.toString(), getNodeSelectorString: getNodeSelector.toString(), getNodePath, diff --git a/lighthouse-core/test/audits/autocomplete-test.js b/lighthouse-core/test/audits/autocomplete-test.js index 76ebba4156d2..26f75f16b397 100644 --- a/lighthouse-core/test/audits/autocomplete-test.js +++ b/lighthouse-core/test/audits/autocomplete-test.js @@ -69,7 +69,7 @@ describe('Best Practices: autocomplete audit', () => { ]; const {score, details} = Autocomplete.audit(artifacts); expect(score).toBe(0); - expect(details.items).toStrictEqual(expectedItems); + expect(details.items).toMatchObject(expectedItems); }); it('fails when an there is an invalid autocomplete attribute set', () => { @@ -110,22 +110,22 @@ describe('Best Practices: autocomplete audit', () => { }; const {score, details} = Autocomplete.audit(artifacts); expect(score).toBe(0); - expect(details.items).toStrictEqual([ + expect(details.items).toMatchObject([ { current: 'namez', node: { + type: 'node', nodeLabel: 'input', snippet: '', - type: 'node', }, suggestion: expect.toBeDisplayString('Requires manual review'), }, { current: 'ccc-num', node: { + type: 'node', nodeLabel: 'input', snippet: '', - type: 'node', }, suggestion: 'cc-number', }, @@ -310,27 +310,27 @@ describe('Best Practices: autocomplete audit', () => { { current: 'shipping section-red cc-name', node: { + type: 'node', nodeLabel: 'textarea', // eslint-disable-next-line max-len snippet: '