From 90018b2c3a3a2caf7f67bfc285059326fadc3586 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 19 May 2020 18:35:30 -0700 Subject: [PATCH 01/37] core(driver): create page code using structured interface --- lighthouse-core/gather/driver.js | 10 ++- lighthouse-core/gather/fetcher.js | 3 +- .../gather/gatherers/link-elements.js | 16 ++--- .../gather/gatherers/meta-elements.js | 39 +++++++---- .../gather/gatherers/script-elements.js | 24 +++---- .../gather/gatherers/seo/tap-targets.js | 68 +++++++++---------- .../gather/gatherers/trace-elements.js | 39 ++++++----- lighthouse-core/lib/page-functions.js | 34 +++++++++- lighthouse-core/report/html/renderer/dom.js | 6 +- 9 files changed, 140 insertions(+), 99 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 016e5f40eec8..71209bf4676c 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -423,11 +423,15 @@ class Driver { * will be evaluated in a content script that has access to the page's DOM but whose JavaScript state * is completely separate. * Returns a promise that resolves on the expression's value. - * @param {string} expression - * @param {{useIsolation?: boolean}=} options - * @return {Promise<*>} + * @template T, R + * @param {string | ((...args: T[]) => R)} expression + * @param {{useIsolation?: boolean, mode?: 'iffe'|'function', args?: T[], deps?: (Function|string)[]}=} options + * @return {Promise} */ async evaluateAsync(expression, options = {}) { + if (typeof expression !== 'string') { + expression = pageFunctions.createEvalCode(expression, options); + } const contextId = options.useIsolation ? await this._getOrCreateIsolatedContextId() : undefined; try { diff --git a/lighthouse-core/gather/fetcher.js b/lighthouse-core/gather/fetcher.js index 061a0f51b5e9..4f9faf73561d 100644 --- a/lighthouse-core/gather/fetcher.js +++ b/lighthouse-core/gather/fetcher.js @@ -156,7 +156,8 @@ class Fetcher { document.body.appendChild(iframe); } - await this.driver.evaluateAsync(`${injectIframe}(${JSON.stringify(url)})`, { + await this.driver.evaluateAsync(injectIframe, { + args: [url], useIsolation: true, }); diff --git a/lighthouse-core/gather/gatherers/link-elements.js b/lighthouse-core/gather/gatherers/link-elements.js index 4f04706aec7c..becb852b0ede 100644 --- a/lighthouse-core/gather/gatherers/link-elements.js +++ b/lighthouse-core/gather/gatherers/link-elements.js @@ -9,7 +9,7 @@ const Gatherer = require('./gatherer.js'); const URL = require('../../lib/url-shim.js').URL; const NetworkAnalyzer = require('../../lib/dependency-graph/simulator/network-analyzer.js'); const LinkHeader = require('http-link-header'); -const {getElementsInDocumentString} = require('../../lib/page-functions.js'); +const {getElementsInDocument} = require('../../lib/page-functions.js'); /* globals HTMLLinkElement */ @@ -48,9 +48,7 @@ function getCrossoriginFromHeader(value) { */ /* istanbul ignore next */ function getLinkElementsInDOM() { - /** @type {Array} */ - // @ts-ignore - getElementsInDocument put into scope via stringification - const browserElements = getElementsInDocument('link'); // eslint-disable-line no-undef + const browserElements = getElementsInDocument('link'); /** @type {LH.Artifacts['LinkElements']} */ const linkElements = []; @@ -81,12 +79,10 @@ class LinkElements extends Gatherer { static getLinkElementsInDOM(passContext) { // We'll use evaluateAsync because the `node.getAttribute` method doesn't actually normalize // the values like access from JavaScript does. - return passContext.driver.evaluateAsync(`(() => { - ${getElementsInDocumentString}; - ${getLinkElementsInDOM}; - - return getLinkElementsInDOM(); - })()`, {useIsolation: true}); + return passContext.driver.evaluateAsync(getLinkElementsInDOM, { + useIsolation: true, + deps: [getElementsInDocument], + }); } /** diff --git a/lighthouse-core/gather/gatherers/meta-elements.js b/lighthouse-core/gather/gatherers/meta-elements.js index a1b67b00d6cc..8f4558963807 100644 --- a/lighthouse-core/gather/gatherers/meta-elements.js +++ b/lighthouse-core/gather/gatherers/meta-elements.js @@ -6,7 +6,27 @@ 'use strict'; const Gatherer = require('./gatherer.js'); -const getElementsInDocumentString = require('../../lib/page-functions.js').getElementsInDocumentString; // eslint-disable-line max-len +const {getElementsInDocument} = require('../../lib/page-functions.js'); + +/* istanbul ignore next */ +function collectMetaElements() { + const metas = /** @type {HTMLMetaElement[]} */ (getElementsInDocument('head meta')); + return metas.map(meta => { + /** @param {string} name */ + const getAttribute = name => { + const attr = meta.attributes.getNamedItem(name); + if (!attr) return; + return attr.value; + }; + return { + name: meta.name.toLowerCase(), + content: meta.content, + property: getAttribute('property'), + httpEquiv: meta.httpEquiv ? meta.httpEquiv.toLowerCase() : undefined, + charset: getAttribute('charset'), + }; + }); +} class MetaElements extends Gatherer { /** @@ -18,19 +38,10 @@ class MetaElements extends Gatherer { // We'll use evaluateAsync because the `node.getAttribute` method doesn't actually normalize // the values like access from JavaScript does. - return driver.evaluateAsync(`(() => { - ${getElementsInDocumentString}; - - return getElementsInDocument('head meta').map(meta => { - return { - name: meta.name.toLowerCase(), - content: meta.content, - property: meta.attributes.property ? meta.attributes.property.value : undefined, - httpEquiv: meta.httpEquiv ? meta.httpEquiv.toLowerCase() : undefined, - charset: meta.attributes.charset ? meta.attributes.charset.value : undefined, - }; - }); - })()`, {useIsolation: true}); + return driver.evaluateAsync(collectMetaElements, { + useIsolation: true, + deps: [getElementsInDocument], + }); } } diff --git a/lighthouse-core/gather/gatherers/script-elements.js b/lighthouse-core/gather/gatherers/script-elements.js index 7a4bca6982dd..7c0aef6e5ce2 100644 --- a/lighthouse-core/gather/gatherers/script-elements.js +++ b/lighthouse-core/gather/gatherers/script-elements.js @@ -8,19 +8,15 @@ const Gatherer = require('./gatherer.js'); const NetworkAnalyzer = require('../../lib/dependency-graph/simulator/network-analyzer.js'); const NetworkRequest = require('../../lib/network-request.js'); -const getElementsInDocumentString = require('../../lib/page-functions.js').getElementsInDocumentString; // eslint-disable-line max-len +const {getElementsInDocument, getNodePath} = require('../../lib/page-functions.js'); const pageFunctions = require('../../lib/page-functions.js'); -/* global getNodePath */ - /** * @return {LH.Artifacts['ScriptElements']} */ /* istanbul ignore next */ -function collectAllScriptElements() { - /** @type {HTMLScriptElement[]} */ - // @ts-ignore - getElementsInDocument put into scope via stringification - const scripts = getElementsInDocument('script'); // eslint-disable-line no-undef +function collectScriptElements() { + const scripts = getElementsInDocument('script'); return scripts.map(script => { return { @@ -30,7 +26,6 @@ function collectAllScriptElements() { async: script.async, defer: script.defer, source: /** @type {'head'|'body'} */ (script.closest('head') ? 'head' : 'body'), - // @ts-ignore - getNodePath put into scope via stringification devtoolsNodePath: getNodePath(script), content: script.src ? null : script.text, requestId: null, @@ -72,12 +67,13 @@ class ScriptElements extends Gatherer { const driver = passContext.driver; const mainResource = NetworkAnalyzer.findMainDocument(loadData.networkRecords, passContext.url); - /** @type {LH.Artifacts['ScriptElements']} */ - const scripts = await driver.evaluateAsync(`(() => { - ${getElementsInDocumentString} - ${pageFunctions.getNodePathString}; - return (${collectAllScriptElements.toString()})(); - })()`, {useIsolation: true}); + const scripts = await driver.evaluateAsync(collectScriptElements, { + useIsolation: true, + deps: [ + getElementsInDocument, + pageFunctions.getNodePathString, + ], + }); for (const script of scripts) { if (script.content) script.requestId = mainResource.requestId; diff --git a/lighthouse-core/gather/gatherers/seo/tap-targets.js b/lighthouse-core/gather/gatherers/seo/tap-targets.js index ab7571b9f3e5..175d9a4587f9 100644 --- a/lighthouse-core/gather/gatherers/seo/tap-targets.js +++ b/lighthouse-core/gather/gatherers/seo/tap-targets.js @@ -38,7 +38,7 @@ const tapTargetsSelector = TARGET_SELECTORS.join(','); /** * @param {HTMLElement} element - * @returns {boolean} + * @return {boolean} */ /* istanbul ignore next */ function elementIsVisible(element) { @@ -47,7 +47,7 @@ function elementIsVisible(element) { /** * @param {Element} element - * @returns {LH.Artifacts.Rect[]} + * @return {LH.Artifacts.Rect[]} */ /* istanbul ignore next */ function getClientRects(element) { @@ -69,17 +69,18 @@ function getClientRects(element) { /** * @param {Element} element - * @returns {boolean} + * @param {string} tapTargetsSelector + * @return {boolean} */ /* istanbul ignore next */ -function elementHasAncestorTapTarget(element) { +function elementHasAncestorTapTarget(element, tapTargetsSelector) { if (!element.parentElement) { return false; } if (element.parentElement.matches(tapTargetsSelector)) { return true; } - return elementHasAncestorTapTarget(element.parentElement); + return elementHasAncestorTapTarget(element.parentElement, tapTargetsSelector); } /** @@ -123,7 +124,7 @@ function hasTextNodeSiblingsFormingTextBlock(element) { * Makes a reasonable guess, but for example gets it wrong if the element is surrounded by other * HTML elements instead of direct text nodes. * @param {Element} element - * @returns {boolean} + * @return {boolean} */ /* istanbul ignore next */ function elementIsInTextBlock(element) { @@ -177,7 +178,7 @@ function elementCenterIsAtZAxisTop(el, elCenterPoint) { /** * Finds all position sticky/absolute elements on the page and adds a class * that disables pointer events on them. - * @returns {() => void} - undo function to re-enable pointer events + * @return {() => void} - undo function to re-enable pointer events */ /* istanbul ignore next */ function disableFixedAndStickyElementPointerEvents() { @@ -202,10 +203,11 @@ function disableFixedAndStickyElementPointerEvents() { } /** - * @returns {LH.Artifacts.TapTarget[]} + * @param {string} tapTargetsSelector + * @return {LH.Artifacts.TapTarget[]} */ /* istanbul ignore next */ -function gatherTapTargets() { +function gatherTapTargets(tapTargetsSelector) { /** @type {LH.Artifacts.TapTarget[]} */ const targets = []; @@ -223,7 +225,7 @@ function gatherTapTargets() { const tapTargetsWithClientRects = []; tapTargetElements.forEach(tapTargetElement => { // Filter out tap targets that are likely to cause false failures: - if (elementHasAncestorTapTarget(tapTargetElement)) { + if (elementHasAncestorTapTarget(tapTargetElement, tapTargetsSelector)) { // This is usually intentional, either the tap targets trigger the same action // or there's a child with a related action (like a delete button for an item) return; @@ -305,30 +307,28 @@ class TapTargets extends Gatherer { * @return {Promise} All visible tap targets with their positions and sizes */ afterPass(passContext) { - const expression = `(function() { - const tapTargetsSelector = "${tapTargetsSelector}"; - ${pageFunctions.getElementsInDocumentString}; - ${disableFixedAndStickyElementPointerEvents.toString()}; - ${elementIsVisible.toString()}; - ${elementHasAncestorTapTarget.toString()}; - ${elementCenterIsAtZAxisTop.toString()} - ${truncate.toString()}; - ${getClientRects.toString()}; - ${hasTextNodeSiblingsFormingTextBlock.toString()}; - ${elementIsInTextBlock.toString()}; - ${getRectArea.toString()}; - ${getLargestRect.toString()}; - ${getRectCenterPoint.toString()}; - ${rectContains.toString()}; - ${pageFunctions.getNodePathString}; - ${pageFunctions.getNodeSelectorString}; - ${pageFunctions.getNodeLabelString}; - ${gatherTapTargets.toString()}; - - return gatherTapTargets(); - })()`; - - return passContext.driver.evaluateAsync(expression, {useIsolation: true}); + return passContext.driver.evaluateAsync(gatherTapTargets, { + args: [tapTargetsSelector], + useIsolation: true, + deps: [ + pageFunctions.getElementsInDocument, + disableFixedAndStickyElementPointerEvents, + elementIsVisible, + elementHasAncestorTapTarget, + elementCenterIsAtZAxisTop, + truncate, + getClientRects, + hasTextNodeSiblingsFormingTextBlock, + elementIsInTextBlock, + getRectArea, + getLargestRect, + getRectCenterPoint, + rectContains, + pageFunctions.getNodePath, + pageFunctions.getNodeSelectorString, + pageFunctions.getNodeLabelString, + ], + }); } } diff --git a/lighthouse-core/gather/gatherers/trace-elements.js b/lighthouse-core/gather/gatherers/trace-elements.js index 32a5437b8554..9a7af1ad581c 100644 --- a/lighthouse-core/gather/gatherers/trace-elements.js +++ b/lighthouse-core/gather/gatherers/trace-elements.js @@ -11,10 +11,13 @@ * We take the backend nodeId from the trace and use it to find the corresponding element in the DOM. */ +/* global document */ + const Gatherer = require('./gatherer.js'); -const pageFunctions = require('../../lib/page-functions.js'); const TraceProcessor = require('../../lib/tracehouse/trace-processor.js'); const RectHelpers = require('../../lib/rect-helpers.js'); +const {createEvalCode, getNodePath, getNodeSelector, getNodeLabel, getOuterHTMLSnippet} = + require('../../lib/page-functions.js'); /** * @this {HTMLElement} @@ -23,19 +26,16 @@ const RectHelpers = require('../../lib/rect-helpers.js'); */ /* istanbul ignore next */ function setAttributeMarker(metricName) { - const elem = this.nodeType === document.ELEMENT_NODE ? this : this.parentElement; // eslint-disable-line no-undef + const elem = this.nodeType === document.ELEMENT_NODE ? this : this.parentElement; let traceElement; if (elem) { + const nodeLabel = getNodeLabel(elem); traceElement = { metricName, - // @ts-ignore - put into scope via stringification - devtoolsNodePath: getNodePath(elem), // eslint-disable-line no-undef - // @ts-ignore - put into scope via stringification - selector: getNodeSelector(elem), // eslint-disable-line no-undef - // @ts-ignore - put into scope via stringification - nodeLabel: getNodeLabel(elem), // eslint-disable-line no-undef - // @ts-ignore - put into scope via stringification - snippet: getOuterHTMLSnippet(elem), // eslint-disable-line no-undef + devtoolsNodePath: getNodePath(elem), + selector: getNodeSelector(elem), + nodeLabel: nodeLabel ? nodeLabel : undefined, + snippet: getOuterHTMLSnippet(elem), }; } return traceElement; @@ -138,16 +138,19 @@ class TraceElements extends Gatherer { const resolveNodeResponse = await driver.sendCommand('DOM.resolveNode', {backendNodeId: backendNodeIds[i]}); const objectId = resolveNodeResponse.object.objectId; + // TODO(cjamcl): create driver.evaluateFunctionOn.. const response = await driver.sendCommand('Runtime.callFunctionOn', { objectId, - functionDeclaration: `function () { - ${setAttributeMarker.toString()}; - ${pageFunctions.getNodePathString}; - ${pageFunctions.getNodeSelectorString}; - ${pageFunctions.getNodeLabelString}; - ${pageFunctions.getOuterHTMLSnippetString}; - return setAttributeMarker.call(this, '${metricName}'); - }`, + functionDeclaration: createEvalCode(setAttributeMarker, { + mode: 'function', + deps: [ + getNodePath, + getNodeSelector, + getNodeLabel, + getOuterHTMLSnippet, + ], + args: [metricName], + }), returnByValue: true, awaitPromise: true, }); diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index f1fbc3a8fde2..8141689a384c 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -6,8 +6,34 @@ // @ts-nocheck 'use strict'; +/** @typedef {HTMLElementTagNameMap & {[id: string]: HTMLElement}} HTMLElementTagNameMapWithDefault */ + /* global window document Node ShadowRoot */ +/** + * @template T, R + * @param {(...args: T[]) => R} mainFn + * @param {{mode?: 'iffe'|'function', args?: T[], deps?: (Function|string)[]}} _ + */ +function createEvalCode(mainFn, {mode, args, deps} = {}) { + const argsSerialized = args ? args.map(arg => JSON.stringify(arg)).join(',') : ''; + const depsSerialized = deps ? deps.join('\n') : ''; + + if (!mode || mode === 'iffe') { + return `(() => { + ${depsSerialized} + ${mainFn} + return ${mainFn.name}(${argsSerialized}); + })()`; + } else { + return `function () { + ${depsSerialized} + ${mainFn} + return ${mainFn.name}.call(this, ${argsSerialized}); + }`; + } +} + /** * Helper functions that are passed by `toString()` by Driver to be evaluated in target page. */ @@ -78,9 +104,10 @@ function checkTimeSinceLastLongTask() { } /** - * @param {string=} selector Optional simple CSS selector to filter nodes on. + * @template {string} T + * @param {T} selector Optional simple CSS selector to filter nodes on. * Combinators are not supported. - * @return {Array} + * @return {Array} */ /* istanbul ignore next */ function getElementsInDocument(selector) { @@ -304,14 +331,17 @@ function getNodeLabel(node) { } module.exports = { + createEvalCode, wrapRuntimeEvalErrorInBrowserString: wrapRuntimeEvalErrorInBrowser.toString(), registerPerformanceObserverInPageString: registerPerformanceObserverInPage.toString(), checkTimeSinceLastLongTaskString: checkTimeSinceLastLongTask.toString(), + getElementsInDocument, getElementsInDocumentString: getElementsInDocument.toString(), getOuterHTMLSnippetString: getOuterHTMLSnippet.toString(), getOuterHTMLSnippet: getOuterHTMLSnippet, ultradumbBenchmark: ultradumbBenchmark, ultradumbBenchmarkString: ultradumbBenchmark.toString(), + getNodePath, getNodePathString: getNodePath.toString(), getNodeSelectorString: getNodeSelector.toString(), getNodeSelector: getNodeSelector, diff --git a/lighthouse-core/report/html/renderer/dom.js b/lighthouse-core/report/html/renderer/dom.js index e712d69930b2..4f5889f51446 100644 --- a/lighthouse-core/report/html/renderer/dom.js +++ b/lighthouse-core/report/html/renderer/dom.js @@ -18,7 +18,7 @@ /* globals URL self Util */ -/** @typedef {HTMLElementTagNameMap & {[id: string]: HTMLElement}} HTMLElementByTagName */ +/** @typedef {HTMLElementTagNameMap & {[id: string]: HTMLElement}} HTMLElementTagNameMapWithDefault */ class DOM { /** @@ -38,7 +38,7 @@ class DOM { * @param {Object=} attrs Attribute key/val pairs. * Note: if an attribute key has an undefined value, this method does not * set the attribute on the node. - * @return {HTMLElementByTagName[T]} + * @return {HTMLElementTagNameMapWithDefault[T]} */ createElement(name, className, attrs = {}) { const element = this._document.createElement(name); @@ -69,7 +69,7 @@ class DOM { * @param {Object=} attrs Attribute key/val pairs. * Note: if an attribute key has an undefined value, this method does not * set the attribute on the node. - * @return {HTMLElementByTagName[T]} + * @return {HTMLElementTagNameMapWithDefault[T]} */ createChildOf(parentElem, elementName, className, attrs) { const element = this.createElement(elementName, className, attrs); From f7e38f5a5ceb9a2ed4cc9ad922d1bf3167edf9b1 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 20 May 2020 12:24:37 -0700 Subject: [PATCH 02/37] rename type --- lighthouse-core/lib/page-functions.js | 4 ++-- lighthouse-core/report/html/renderer/dom.js | 6 +++--- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index 8141689a384c..d678dbd88fe3 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -6,7 +6,7 @@ // @ts-nocheck 'use strict'; -/** @typedef {HTMLElementTagNameMap & {[id: string]: HTMLElement}} HTMLElementTagNameMapWithDefault */ +/** @typedef {HTMLElementTagNameMap & {[id: string]: HTMLElement}} HTMLElementByTagName */ /* global window document Node ShadowRoot */ @@ -107,7 +107,7 @@ function checkTimeSinceLastLongTask() { * @template {string} T * @param {T} selector Optional simple CSS selector to filter nodes on. * Combinators are not supported. - * @return {Array} + * @return {Array} */ /* istanbul ignore next */ function getElementsInDocument(selector) { diff --git a/lighthouse-core/report/html/renderer/dom.js b/lighthouse-core/report/html/renderer/dom.js index 4f5889f51446..e712d69930b2 100644 --- a/lighthouse-core/report/html/renderer/dom.js +++ b/lighthouse-core/report/html/renderer/dom.js @@ -18,7 +18,7 @@ /* globals URL self Util */ -/** @typedef {HTMLElementTagNameMap & {[id: string]: HTMLElement}} HTMLElementTagNameMapWithDefault */ +/** @typedef {HTMLElementTagNameMap & {[id: string]: HTMLElement}} HTMLElementByTagName */ class DOM { /** @@ -38,7 +38,7 @@ class DOM { * @param {Object=} attrs Attribute key/val pairs. * Note: if an attribute key has an undefined value, this method does not * set the attribute on the node. - * @return {HTMLElementTagNameMapWithDefault[T]} + * @return {HTMLElementByTagName[T]} */ createElement(name, className, attrs = {}) { const element = this._document.createElement(name); @@ -69,7 +69,7 @@ class DOM { * @param {Object=} attrs Attribute key/val pairs. * Note: if an attribute key has an undefined value, this method does not * set the attribute on the node. - * @return {HTMLElementTagNameMapWithDefault[T]} + * @return {HTMLElementByTagName[T]} */ createChildOf(parentElem, elementName, className, attrs) { const element = this.createElement(elementName, className, attrs); From 78f386ac2c5e4b0673618a65965f28a7146633f1 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 20 May 2020 12:55:15 -0700 Subject: [PATCH 03/37] pr feedback --- lighthouse-core/gather/driver.js | 77 +++++++++++++++++-- lighthouse-core/gather/fetcher.js | 2 +- .../gather/gatherers/link-elements.js | 2 +- .../gather/gatherers/meta-elements.js | 2 +- .../gather/gatherers/script-elements.js | 2 +- .../gather/gatherers/seo/tap-targets.js | 2 +- .../gather/gatherers/trace-elements.js | 23 +++--- lighthouse-core/lib/page-functions.js | 9 ++- 8 files changed, 92 insertions(+), 27 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 71209bf4676c..dddb1eeecd3d 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -418,21 +418,86 @@ class Driver { }); } + /** + * Deprecated: renamed to `evaluate`. + * @param {string} expression + * @param {{useIsolation?: boolean}=} options + * @return {Promise<*>} + */ + async evaluateAsync(expression, options = {}) { + return this.evaluate(expression, options); + } + /** * Evaluate an expression in the context of the current page. If useIsolation is true, the expression * will be evaluated in a content script that has access to the page's DOM but whose JavaScript state * is completely separate. * Returns a promise that resolves on the expression's value. + * See the documentation for `createEvalCode` in `page-functions.js`. * @template T, R - * @param {string | ((...args: T[]) => R)} expression - * @param {{useIsolation?: boolean, mode?: 'iffe'|'function', args?: T[], deps?: (Function|string)[]}=} options + * @param {string | ((...args: T[]) => R)} expressionOrMainFn + * @param {{useIsolation?: boolean, args?: T[], deps?: (Function|string)[]}=} options * @return {Promise} */ - async evaluateAsync(expression, options = {}) { - if (typeof expression !== 'string') { - expression = pageFunctions.createEvalCode(expression, options); + async evaluate(expressionOrMainFn, options = {}) { + if (typeof expressionOrMainFn !== 'string') { + expressionOrMainFn = pageFunctions.createEvalCode(expressionOrMainFn, {mode: 'iffe', ...options}); + } + return this._evaluate(expressionOrMainFn, options); + } + + /** + * Evaluate a statement in the context of a JavaScript object on the current page. + * Returns a promise that resolves on the expression's value. + * See the documentation for `createEvalCode` in `page-functions.js`. + * @template T, R + * @param {string | ((...args: T[]) => R)} statementOrMainFn + * @param {{objectId: string, args?: T[], deps?: (Function|string)[]}} options + * @return {Promise} + */ + async evaluateFunctionOnObject(statementOrMainFn, options) { + if (typeof statementOrMainFn !== 'string') { + statementOrMainFn = pageFunctions.createEvalCode(statementOrMainFn, { + mode: 'function', + ...options, + }); + } + + const response = await this.sendCommand('Runtime.callFunctionOn', { + objectId: options.objectId, + functionDeclaration: statementOrMainFn, + returnByValue: true, + awaitPromise: true, + }); + + if (response.exceptionDetails) { + // An error occurred before we could even create a Promise, should be *very* rare. + // Also occurs when the expression is not valid JavaScript. + const errorMessage = response.exceptionDetails.exception ? + response.exceptionDetails.exception.description : + response.exceptionDetails.text; + return Promise.reject(new Error(`Evaluation exception: ${errorMessage}`)); + } + + // TODO: check if __failedInBrowser happens here too. + if (response && response.result && response.result.value) { + return response.result.value; } - const contextId = options.useIsolation ? await this._getOrCreateIsolatedContextId() : undefined; + return Promise.reject(); + } + + /** + * Evaluate an expression in the context of the current page. If useIsolation is true, the expression + * will be evaluated in a content script that has access to the page's DOM but whose JavaScript state + * is completely separate. + * Returns a promise that resolves on the expression's value. + * @param {string} expression + * @param {{useIsolation?: boolean}} options + * @return {Promise<*>} + */ + async _evaluate(expression, options) { + const contextId = + options.useIsolation ? await this._getOrCreateIsolatedContextId() : undefined; try { // `await` is not redundant here because we want to `catch` the async errors diff --git a/lighthouse-core/gather/fetcher.js b/lighthouse-core/gather/fetcher.js index 4f9faf73561d..c048b1f0427c 100644 --- a/lighthouse-core/gather/fetcher.js +++ b/lighthouse-core/gather/fetcher.js @@ -156,7 +156,7 @@ class Fetcher { document.body.appendChild(iframe); } - await this.driver.evaluateAsync(injectIframe, { + await this.driver.evaluate(injectIframe, { args: [url], useIsolation: true, }); diff --git a/lighthouse-core/gather/gatherers/link-elements.js b/lighthouse-core/gather/gatherers/link-elements.js index becb852b0ede..caa1b2cec21c 100644 --- a/lighthouse-core/gather/gatherers/link-elements.js +++ b/lighthouse-core/gather/gatherers/link-elements.js @@ -79,7 +79,7 @@ class LinkElements extends Gatherer { static getLinkElementsInDOM(passContext) { // We'll use evaluateAsync because the `node.getAttribute` method doesn't actually normalize // the values like access from JavaScript does. - return passContext.driver.evaluateAsync(getLinkElementsInDOM, { + return passContext.driver.evaluate(getLinkElementsInDOM, { useIsolation: true, deps: [getElementsInDocument], }); diff --git a/lighthouse-core/gather/gatherers/meta-elements.js b/lighthouse-core/gather/gatherers/meta-elements.js index 8f4558963807..6d4f5c39d3c9 100644 --- a/lighthouse-core/gather/gatherers/meta-elements.js +++ b/lighthouse-core/gather/gatherers/meta-elements.js @@ -38,7 +38,7 @@ class MetaElements extends Gatherer { // We'll use evaluateAsync because the `node.getAttribute` method doesn't actually normalize // the values like access from JavaScript does. - return driver.evaluateAsync(collectMetaElements, { + return driver.evaluate(collectMetaElements, { useIsolation: true, deps: [getElementsInDocument], }); diff --git a/lighthouse-core/gather/gatherers/script-elements.js b/lighthouse-core/gather/gatherers/script-elements.js index 7c0aef6e5ce2..4d964c1c1e91 100644 --- a/lighthouse-core/gather/gatherers/script-elements.js +++ b/lighthouse-core/gather/gatherers/script-elements.js @@ -67,7 +67,7 @@ class ScriptElements extends Gatherer { const driver = passContext.driver; const mainResource = NetworkAnalyzer.findMainDocument(loadData.networkRecords, passContext.url); - const scripts = await driver.evaluateAsync(collectScriptElements, { + const scripts = await driver.evaluate(collectScriptElements, { useIsolation: true, deps: [ getElementsInDocument, diff --git a/lighthouse-core/gather/gatherers/seo/tap-targets.js b/lighthouse-core/gather/gatherers/seo/tap-targets.js index 175d9a4587f9..28dc8fa51c7c 100644 --- a/lighthouse-core/gather/gatherers/seo/tap-targets.js +++ b/lighthouse-core/gather/gatherers/seo/tap-targets.js @@ -307,7 +307,7 @@ class TapTargets extends Gatherer { * @return {Promise} All visible tap targets with their positions and sizes */ afterPass(passContext) { - return passContext.driver.evaluateAsync(gatherTapTargets, { + return passContext.driver.evaluate(gatherTapTargets, { args: [tapTargetsSelector], useIsolation: true, deps: [ diff --git a/lighthouse-core/gather/gatherers/trace-elements.js b/lighthouse-core/gather/gatherers/trace-elements.js index 9a7af1ad581c..91e946958d70 100644 --- a/lighthouse-core/gather/gatherers/trace-elements.js +++ b/lighthouse-core/gather/gatherers/trace-elements.js @@ -16,7 +16,7 @@ const Gatherer = require('./gatherer.js'); const TraceProcessor = require('../../lib/tracehouse/trace-processor.js'); const RectHelpers = require('../../lib/rect-helpers.js'); -const {createEvalCode, getNodePath, getNodeSelector, getNodeLabel, getOuterHTMLSnippet} = +const {getNodePath, getNodeSelector, getNodeLabel, getOuterHTMLSnippet} = require('../../lib/page-functions.js'); /** @@ -138,11 +138,11 @@ class TraceElements extends Gatherer { const resolveNodeResponse = await driver.sendCommand('DOM.resolveNode', {backendNodeId: backendNodeIds[i]}); const objectId = resolveNodeResponse.object.objectId; - // TODO(cjamcl): create driver.evaluateFunctionOn.. - const response = await driver.sendCommand('Runtime.callFunctionOn', { - objectId, - functionDeclaration: createEvalCode(setAttributeMarker, { - mode: 'function', + if (!objectId) continue; + + try { + const element = await driver.evaluateFunctionOnObject(setAttributeMarker, { + objectId, deps: [ getNodePath, getNodeSelector, @@ -150,14 +150,9 @@ class TraceElements extends Gatherer { getOuterHTMLSnippet, ], args: [metricName], - }), - returnByValue: true, - awaitPromise: true, - }); - - if (response && response.result && response.result.value) { - traceElements.push(response.result.value); - } + }); + if (element) traceElements.push(element); + } catch (_) {} } return traceElements; diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index d678dbd88fe3..e47bdcf7ae3c 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -11,9 +11,14 @@ /* global window document Node ShadowRoot */ /** + * Creates valid JavaScript code given functions, strings of valid code, and arguments. * @template T, R - * @param {(...args: T[]) => R} mainFn - * @param {{mode?: 'iffe'|'function', args?: T[], deps?: (Function|string)[]}} _ + * @param {(...args: T[]) => R} mainFn The main function to call. It's return value will be the return value + * of `createEvalCode`, wrapped in a Promise. + * @param {{mode?: 'iffe'|'function', args?: T[], deps?: (Function|string)[]}} _ Set mode to `iffe` to create a self- + * executing function expression, set to `function` to create just a function declaration statement. Args should match + * the args of `mainFn`, and can be any serializable value. `deps` are functions or string of valid code that must be + * defined for `mainFn` to work. */ function createEvalCode(mainFn, {mode, args, deps} = {}) { const argsSerialized = args ? args.map(arg => JSON.stringify(arg)).join(',') : ''; From 32856e756867fc5882139388493769555b477aff Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 20 May 2020 15:18:10 -0700 Subject: [PATCH 04/37] no strings --- lighthouse-core/gather/driver.js | 4 ++-- lighthouse-core/gather/gatherers/script-elements.js | 2 +- lighthouse-core/gather/gatherers/seo/tap-targets.js | 4 ++-- lighthouse-core/lib/page-functions.js | 8 ++++---- 4 files changed, 9 insertions(+), 9 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index dddb1eeecd3d..23e06413713b 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -436,7 +436,7 @@ class Driver { * See the documentation for `createEvalCode` in `page-functions.js`. * @template T, R * @param {string | ((...args: T[]) => R)} expressionOrMainFn - * @param {{useIsolation?: boolean, args?: T[], deps?: (Function|string)[]}=} options + * @param {{useIsolation?: boolean, args?: T[], deps?: Function[]}=} options * @return {Promise} */ async evaluate(expressionOrMainFn, options = {}) { @@ -452,7 +452,7 @@ class Driver { * See the documentation for `createEvalCode` in `page-functions.js`. * @template T, R * @param {string | ((...args: T[]) => R)} statementOrMainFn - * @param {{objectId: string, args?: T[], deps?: (Function|string)[]}} options + * @param {{objectId: string, args?: T[], deps?: Function[]}} options * @return {Promise} */ async evaluateFunctionOnObject(statementOrMainFn, options) { diff --git a/lighthouse-core/gather/gatherers/script-elements.js b/lighthouse-core/gather/gatherers/script-elements.js index 4d964c1c1e91..ddacfb8fee6c 100644 --- a/lighthouse-core/gather/gatherers/script-elements.js +++ b/lighthouse-core/gather/gatherers/script-elements.js @@ -71,7 +71,7 @@ class ScriptElements extends Gatherer { useIsolation: true, deps: [ getElementsInDocument, - pageFunctions.getNodePathString, + pageFunctions.getNodePath, ], }); diff --git a/lighthouse-core/gather/gatherers/seo/tap-targets.js b/lighthouse-core/gather/gatherers/seo/tap-targets.js index 28dc8fa51c7c..56b9572e067c 100644 --- a/lighthouse-core/gather/gatherers/seo/tap-targets.js +++ b/lighthouse-core/gather/gatherers/seo/tap-targets.js @@ -325,8 +325,8 @@ class TapTargets extends Gatherer { getRectCenterPoint, rectContains, pageFunctions.getNodePath, - pageFunctions.getNodeSelectorString, - pageFunctions.getNodeLabelString, + pageFunctions.getNodeSelector, + pageFunctions.getNodeLabel, ], }); } diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index e47bdcf7ae3c..5ae0e5116348 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -15,10 +15,10 @@ * @template T, R * @param {(...args: T[]) => R} mainFn The main function to call. It's return value will be the return value * of `createEvalCode`, wrapped in a Promise. - * @param {{mode?: 'iffe'|'function', args?: T[], deps?: (Function|string)[]}} _ Set mode to `iffe` to create a self- - * executing function expression, set to `function` to create just a function declaration statement. Args should match - * the args of `mainFn`, and can be any serializable value. `deps` are functions or string of valid code that must be - * defined for `mainFn` to work. + * @param {{mode?: 'iffe'|'function', args?: T[], deps?: Function[]}} _ Set mode to `iffe` to + * create a self-executing function expression, set to `function` to create just a function + * declaration statement. Args should match the args of `mainFn`, and can be any serializable + * value. `deps` are functions that must be defined for `mainFn` to work. */ function createEvalCode(mainFn, {mode, args, deps} = {}) { const argsSerialized = args ? args.map(arg => JSON.stringify(arg)).join(',') : ''; From e2dd63dac018768cdd384028d2310c2906ef79e6 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 20 May 2020 15:41:35 -0700 Subject: [PATCH 05/37] test --- lighthouse-core/gather/driver.js | 5 +- .../test/lib/page-functions-test.js | 85 +++++++++++++++++++ 2 files changed, 89 insertions(+), 1 deletion(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 23e06413713b..92c5cd5969b3 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -441,7 +441,10 @@ class Driver { */ async evaluate(expressionOrMainFn, options = {}) { if (typeof expressionOrMainFn !== 'string') { - expressionOrMainFn = pageFunctions.createEvalCode(expressionOrMainFn, {mode: 'iffe', ...options}); + expressionOrMainFn = pageFunctions.createEvalCode(expressionOrMainFn, { + mode: 'iffe', + ...options, + }); } return this._evaluate(expressionOrMainFn, options); } diff --git a/lighthouse-core/test/lib/page-functions-test.js b/lighthouse-core/test/lib/page-functions-test.js index bea55964c0f5..4125ba2fd79d 100644 --- a/lighthouse-core/test/lib/page-functions-test.js +++ b/lighthouse-core/test/lib/page-functions-test.js @@ -12,6 +12,91 @@ const pageFunctions = require('../../lib/page-functions.js'); /* eslint-env jest */ +describe('createEvalCode', () => { + it('iffe basic', () => { + function mainFn() { + return true; + } + const code = pageFunctions.createEvalCode(mainFn, { + mode: 'iffe', + }); + expect(code).toMatchInlineSnapshot(` + "(() => { + + function mainFn() { + return true; + } + return mainFn(); + })()" + `); + expect(eval(code)).toEqual(true); + }); + + it('iffe complex', () => { + function mainFn({a, b}, passThru) { + return {a: abs(a), b: square(b), passThru}; + } + function abs(val) { + return Math.abs(val); + } + function square(val) { + return val * val; + } + const code = pageFunctions.createEvalCode(mainFn, { + args: [{a: -5, b: 10}, 'hello'], + deps: [abs, square], + mode: 'iffe', + }); + expect(code).toMatchInlineSnapshot(` + "(() => { + function abs(val) { + return Math.abs(val); + } + function square(val) { + return val * val; + } + function mainFn({ + a, + b + }, passThru) { + return { + a: abs(a), + b: square(b), + passThru + }; + } + return mainFn({\\"a\\":-5,\\"b\\":10},\\"hello\\"); + })()" + `); + expect(eval(code)).toEqual({a: 5, b: 100, passThru: 'hello'}); + }); + + it('function', () => { + function mainFn(a, b) { + return adder(a, b); + } + function adder(a, b) { + return a + b; + } + const code = pageFunctions.createEvalCode(mainFn, { + args: [1, 2], + mode: 'function', + deps: [adder], + }); + expect(code).toMatchInlineSnapshot(` + "function () { + function adder(a, b) { + return a + b; + } + function mainFn(a, b) { + return adder(a, b); + } + return mainFn.call(this, 1,2); + }" + `); + }); +}); + describe('Page Functions', () => { let dom; From ce9839b165b388b55fe990ab293a860dcdfe3089 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 3 Jun 2020 19:35:02 -0700 Subject: [PATCH 06/37] test --- lighthouse-core/test/gather/gatherers/link-elements-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/test/gather/gatherers/link-elements-test.js b/lighthouse-core/test/gather/gatherers/link-elements-test.js index 39dfdaf9e92d..fedf3c3cc21a 100644 --- a/lighthouse-core/test/gather/gatherers/link-elements-test.js +++ b/lighthouse-core/test/gather/gatherers/link-elements-test.js @@ -30,7 +30,7 @@ describe('Link Elements gatherer', () => { function getPassData({linkElementsInDOM = [], headers = []}) { const url = 'https://example.com'; const loadData = {networkRecords: [{url, responseHeaders: headers, resourceType: 'Document'}]}; - const driver = {evaluateAsync: () => Promise.resolve(linkElementsInDOM)}; + const driver = {evaluate: () => Promise.resolve(linkElementsInDOM)}; const passContext = {driver, url}; return [passContext, loadData]; } From 4c903def0c4149895a628eadc7e0466eb128f350 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Fri, 5 Jun 2020 13:45:33 -0700 Subject: [PATCH 07/37] restrucutre --- lighthouse-core/gather/driver.js | 35 ++++++++++++++++++++++++-------- 1 file changed, 26 insertions(+), 9 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 92c5cd5969b3..1c455b8ff356 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -419,21 +419,38 @@ class Driver { } /** - * Deprecated: renamed to `evaluate`. + * Evaluate an expression in the context of the current page. If useIsolation is true, the expression + * will be evaluated in a content script that has access to the page's DOM but whose JavaScript state + * is completely separate. + * Returns a promise that resolves on the expression's value. + * @template T * @param {string} expression - * @param {{useIsolation?: boolean}=} options + * @param {{useIsolation?: boolean, args?: T[], deps?: Function[]}=} options * @return {Promise<*>} */ async evaluateAsync(expression, options = {}) { - return this.evaluate(expression, options); + const contextId = + options.useIsolation ? await this._getOrCreateIsolatedContextId() : undefined; + + try { + // `await` is not redundant here because we want to `catch` the async errors + return await this._evaluateInContext(expression, contextId); + } catch (err) { + // If we were using isolation and the context disappeared on us, retry one more time. + if (contextId && err.message.includes('Cannot find context')) { + this._clearIsolatedContextId(); + const freshContextId = await this._getOrCreateIsolatedContextId(); + return this._evaluateInContext(expression, freshContextId); + } + + throw err; + } } /** - * Evaluate an expression in the context of the current page. If useIsolation is true, the expression - * will be evaluated in a content script that has access to the page's DOM but whose JavaScript state - * is completely separate. - * Returns a promise that resolves on the expression's value. - * See the documentation for `createEvalCode` in `page-functions.js`. + * Evaluate an expression (optionally defined in a structured manner, see `createEvalCode` + * in `page-functions.js`). + * See `evaluateAsync`. * @template T, R * @param {string | ((...args: T[]) => R)} expressionOrMainFn * @param {{useIsolation?: boolean, args?: T[], deps?: Function[]}=} options @@ -446,7 +463,7 @@ class Driver { ...options, }); } - return this._evaluate(expressionOrMainFn, options); + return this.evaluateAsync(expressionOrMainFn, options); } /** From 02dc2153fb6d72b20d2360a1f594240395213c23 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 16 Jun 2020 17:50:27 -0700 Subject: [PATCH 08/37] remove obj --- lighthouse-core/gather/driver.js | 40 -------------- .../gather/gatherers/trace-elements.js | 53 ++++++++++--------- 2 files changed, 27 insertions(+), 66 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index a6e07f3738f4..488dac004ffc 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -466,46 +466,6 @@ class Driver { return this.evaluateAsync(expressionOrMainFn, options); } - /** - * Evaluate a statement in the context of a JavaScript object on the current page. - * Returns a promise that resolves on the expression's value. - * See the documentation for `createEvalCode` in `page-functions.js`. - * @template T, R - * @param {string | ((...args: T[]) => R)} statementOrMainFn - * @param {{objectId: string, args?: T[], deps?: Function[]}} options - * @return {Promise} - */ - async evaluateFunctionOnObject(statementOrMainFn, options) { - if (typeof statementOrMainFn !== 'string') { - statementOrMainFn = pageFunctions.createEvalCode(statementOrMainFn, { - mode: 'function', - ...options, - }); - } - - const response = await this.sendCommand('Runtime.callFunctionOn', { - objectId: options.objectId, - functionDeclaration: statementOrMainFn, - returnByValue: true, - awaitPromise: true, - }); - - if (response.exceptionDetails) { - // An error occurred before we could even create a Promise, should be *very* rare. - // Also occurs when the expression is not valid JavaScript. - const errorMessage = response.exceptionDetails.exception ? - response.exceptionDetails.exception.description : - response.exceptionDetails.text; - return Promise.reject(new Error(`Evaluation exception: ${errorMessage}`)); - } - - // TODO: check if __failedInBrowser happens here too. - if (response && response.result && response.result.value) { - return response.result.value; - } - return Promise.reject(); - } - /** * Evaluate an expression in the context of the current page. If useIsolation is true, the expression * will be evaluated in a content script that has access to the page's DOM but whose JavaScript state diff --git a/lighthouse-core/gather/gatherers/trace-elements.js b/lighthouse-core/gather/gatherers/trace-elements.js index 91e946958d70..c3450d9e3417 100644 --- a/lighthouse-core/gather/gatherers/trace-elements.js +++ b/lighthouse-core/gather/gatherers/trace-elements.js @@ -11,13 +11,10 @@ * We take the backend nodeId from the trace and use it to find the corresponding element in the DOM. */ -/* global document */ - const Gatherer = require('./gatherer.js'); +const pageFunctions = require('../../lib/page-functions.js'); const TraceProcessor = require('../../lib/tracehouse/trace-processor.js'); const RectHelpers = require('../../lib/rect-helpers.js'); -const {getNodePath, getNodeSelector, getNodeLabel, getOuterHTMLSnippet} = - require('../../lib/page-functions.js'); /** * @this {HTMLElement} @@ -26,16 +23,19 @@ const {getNodePath, getNodeSelector, getNodeLabel, getOuterHTMLSnippet} = */ /* istanbul ignore next */ function setAttributeMarker(metricName) { - const elem = this.nodeType === document.ELEMENT_NODE ? this : this.parentElement; + const elem = this.nodeType === document.ELEMENT_NODE ? this : this.parentElement; // eslint-disable-line no-undef let traceElement; if (elem) { - const nodeLabel = getNodeLabel(elem); traceElement = { metricName, - devtoolsNodePath: getNodePath(elem), - selector: getNodeSelector(elem), - nodeLabel: nodeLabel ? nodeLabel : undefined, - snippet: getOuterHTMLSnippet(elem), + // @ts-ignore - put into scope via stringification + devtoolsNodePath: getNodePath(elem), // eslint-disable-line no-undef + // @ts-ignore - put into scope via stringification + selector: getNodeSelector(elem), // eslint-disable-line no-undef + // @ts-ignore - put into scope via stringification + nodeLabel: getNodeLabel(elem), // eslint-disable-line no-undef + // @ts-ignore - put into scope via stringification + snippet: getOuterHTMLSnippet(elem), // eslint-disable-line no-undef }; } return traceElement; @@ -135,24 +135,25 @@ class TraceElements extends Gatherer { for (let i = 0; i < backendNodeIds.length; i++) { const metricName = lcpNodeId === backendNodeIds[i] ? 'largest-contentful-paint' : 'cumulative-layout-shift'; - const resolveNodeResponse = - await driver.sendCommand('DOM.resolveNode', {backendNodeId: backendNodeIds[i]}); - const objectId = resolveNodeResponse.object.objectId; + const objectId = await driver.resolveNodeIdToObjectId(backendNodeIds[i]); if (!objectId) continue; + const response = await driver.sendCommand('Runtime.callFunctionOn', { + objectId, + functionDeclaration: `function () { + ${setAttributeMarker.toString()}; + ${pageFunctions.getNodePathString}; + ${pageFunctions.getNodeSelectorString}; + ${pageFunctions.getNodeLabelString}; + ${pageFunctions.getOuterHTMLSnippetString}; + return setAttributeMarker.call(this, '${metricName}'); + }`, + returnByValue: true, + awaitPromise: true, + }); - try { - const element = await driver.evaluateFunctionOnObject(setAttributeMarker, { - objectId, - deps: [ - getNodePath, - getNodeSelector, - getNodeLabel, - getOuterHTMLSnippet, - ], - args: [metricName], - }); - if (element) traceElements.push(element); - } catch (_) {} + if (response && response.result && response.result.value) { + traceElements.push(response.result.value); + } } return traceElements; From 70d821014fd253b7a5177ac19e3e00c2c0b4b41d Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 5 Nov 2020 13:59:59 -0600 Subject: [PATCH 09/37] rm dead code --- lighthouse-core/gather/driver.js | 29 +---------------------------- 1 file changed, 1 insertion(+), 28 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index fe8eca18a7ed..fa5ab41416e2 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -438,6 +438,7 @@ class Driver { } /** + * Note: Prefer `evaluate` instead. * Evaluate an expression in the context of the current page. If useIsolation is true, the expression * will be evaluated in a content script that has access to the page's DOM but whose JavaScript state * is completely separate. @@ -485,34 +486,6 @@ class Driver { return this.evaluateAsync(expressionOrMainFn, options); } - /** - * Evaluate an expression in the context of the current page. If useIsolation is true, the expression - * will be evaluated in a content script that has access to the page's DOM but whose JavaScript state - * is completely separate. - * Returns a promise that resolves on the expression's value. - * @param {string} expression - * @param {{useIsolation?: boolean}} options - * @return {Promise<*>} - */ - async _evaluate(expression, options) { - const contextId = - options.useIsolation ? await this._getOrCreateIsolatedContextId() : undefined; - - try { - // `await` is not redundant here because we want to `catch` the async errors - return await this._evaluateInContext(expression, contextId); - } catch (err) { - // If we were using isolation and the context disappeared on us, retry one more time. - if (contextId && err.message.includes('Cannot find context')) { - this._clearIsolatedContextId(); - const freshContextId = await this._getOrCreateIsolatedContextId(); - return this._evaluateInContext(expression, freshContextId); - } - - throw err; - } - } - /** * Evaluate an expression in the given execution context; an undefined contextId implies the main * page without isolation. From 4e594763c2b199d44ea4df80f0aee9b30b19ca28 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 10 Nov 2020 12:10:09 -0600 Subject: [PATCH 10/37] adam feedback --- lighthouse-core/lib/page-functions.js | 2 +- lighthouse-core/test/lib/page-functions-test.js | 6 ++++-- 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index 8e2fe96a27af..66ba77c05e73 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -21,7 +21,7 @@ * value. `deps` are functions that must be defined for `mainFn` to work. */ function createEvalCode(mainFn, {mode, args, deps} = {}) { - const argsSerialized = args ? args.map(arg => JSON.stringify(arg)).join(',') : ''; + const argsSerialized = args.map(arg => JSON.stringify(arg)).join(','); const depsSerialized = deps ? deps.join('\n') : ''; if (!mode || mode === 'iife') { diff --git a/lighthouse-core/test/lib/page-functions-test.js b/lighthouse-core/test/lib/page-functions-test.js index 34580c1ee469..9b5f18e998c6 100644 --- a/lighthouse-core/test/lib/page-functions-test.js +++ b/lighthouse-core/test/lib/page-functions-test.js @@ -13,11 +13,12 @@ const pageFunctions = require('../../lib/page-functions.js'); /* eslint-env jest */ describe('createEvalCode', () => { - it('iffe basic', () => { + it('iife basic', () => { function mainFn() { return true; } const code = pageFunctions.createEvalCode(mainFn, { + args: [], mode: 'iife', }); expect(code).toMatchInlineSnapshot(` @@ -32,7 +33,7 @@ describe('createEvalCode', () => { expect(eval(code)).toEqual(true); }); - it('iffe complex', () => { + it('iife complex', () => { function mainFn({a, b}, passThru) { return {a: abs(a), b: square(b), passThru}; } @@ -94,6 +95,7 @@ describe('createEvalCode', () => { return mainFn.call(this, 1,2); }" `); + expect(eval(`(${code})()`)).toEqual(3); }); }); From 147b98c277e0f1f35313fdb262b986b0d696e1d1 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 10 Nov 2020 12:23:54 -0600 Subject: [PATCH 11/37] fix tests --- lighthouse-core/gather/driver.js | 2 +- lighthouse-core/gather/gatherers/meta-elements.js | 7 +++++-- lighthouse-core/lib/page-functions.js | 2 +- lighthouse-core/test/lib/page-functions-test.js | 1 - 4 files changed, 7 insertions(+), 5 deletions(-) diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index fa5ab41416e2..6812a38682b2 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -445,7 +445,7 @@ class Driver { * Returns a promise that resolves on the expression's value. * @template T * @param {string} expression - * @param {{useIsolation?: boolean, args?: T[], deps?: Array}=} options + * @param {{useIsolation?: boolean}=} options * @return {Promise<*>} */ async evaluateAsync(expression, options = {}) { diff --git a/lighthouse-core/gather/gatherers/meta-elements.js b/lighthouse-core/gather/gatherers/meta-elements.js index 6d4f5c39d3c9..bb90e9195116 100644 --- a/lighthouse-core/gather/gatherers/meta-elements.js +++ b/lighthouse-core/gather/gatherers/meta-elements.js @@ -6,10 +6,13 @@ 'use strict'; const Gatherer = require('./gatherer.js'); -const {getElementsInDocument} = require('../../lib/page-functions.js'); +const pageFunctions = require('../../lib/page-functions.js'); + +/* global getElementsInDocument */ /* istanbul ignore next */ function collectMetaElements() { + // @ts-expect-error - getElementsInDocument put into scope via stringification const metas = /** @type {HTMLMetaElement[]} */ (getElementsInDocument('head meta')); return metas.map(meta => { /** @param {string} name */ @@ -40,7 +43,7 @@ class MetaElements extends Gatherer { // the values like access from JavaScript does. return driver.evaluate(collectMetaElements, { useIsolation: true, - deps: [getElementsInDocument], + deps: [pageFunctions.getElementsInDocument], }); } } diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index 66ba77c05e73..8e2fe96a27af 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -21,7 +21,7 @@ * value. `deps` are functions that must be defined for `mainFn` to work. */ function createEvalCode(mainFn, {mode, args, deps} = {}) { - const argsSerialized = args.map(arg => JSON.stringify(arg)).join(','); + const argsSerialized = args ? args.map(arg => JSON.stringify(arg)).join(',') : ''; const depsSerialized = deps ? deps.join('\n') : ''; if (!mode || mode === 'iife') { diff --git a/lighthouse-core/test/lib/page-functions-test.js b/lighthouse-core/test/lib/page-functions-test.js index 9b5f18e998c6..7b3427bebb49 100644 --- a/lighthouse-core/test/lib/page-functions-test.js +++ b/lighthouse-core/test/lib/page-functions-test.js @@ -18,7 +18,6 @@ describe('createEvalCode', () => { return true; } const code = pageFunctions.createEvalCode(mainFn, { - args: [], mode: 'iife', }); expect(code).toMatchInlineSnapshot(` From a4051a1218122c0d449a1bb34e09b57ec8967397 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 10 Nov 2020 12:37:20 -0600 Subject: [PATCH 12/37] fix mangle issues --- lighthouse-core/gather/gatherers/link-elements.js | 7 ++++--- lighthouse-core/gather/gatherers/meta-elements.js | 2 +- 2 files changed, 5 insertions(+), 4 deletions(-) diff --git a/lighthouse-core/gather/gatherers/link-elements.js b/lighthouse-core/gather/gatherers/link-elements.js index 2362519e7431..5b409af99f3b 100644 --- a/lighthouse-core/gather/gatherers/link-elements.js +++ b/lighthouse-core/gather/gatherers/link-elements.js @@ -9,7 +9,7 @@ const LinkHeader = require('http-link-header'); const Gatherer = require('./gatherer.js'); const {URL} = require('../../lib/url-shim.js'); const NetworkAnalyzer = require('../../lib/dependency-graph/simulator/network-analyzer.js'); -const {getElementsInDocument, getNodeDetailsString} = require('../../lib/page-functions.js'); +const pageFunctions = require('../../lib/page-functions.js'); /* globals HTMLLinkElement getNodeDetails */ @@ -49,6 +49,7 @@ function getCrossoriginFromHeader(value) { /* istanbul ignore next */ function getLinkElementsInDOM() { /** @type {Array} */ + // @ts-expect-error - getElementsInDocument put into scope via stringification const browserElements = getElementsInDocument('link'); // eslint-disable-line no-undef /** @type {LH.Artifacts['LinkElements']} */ const linkElements = []; @@ -88,8 +89,8 @@ class LinkElements extends Gatherer { return passContext.driver.evaluate(getLinkElementsInDOM, { useIsolation: true, deps: [ - getNodeDetailsString, - getElementsInDocument, + pageFunctions.getNodeDetailsString, + pageFunctions.getElementsInDocument, ], }); } diff --git a/lighthouse-core/gather/gatherers/meta-elements.js b/lighthouse-core/gather/gatherers/meta-elements.js index bb90e9195116..5ccb609a914b 100644 --- a/lighthouse-core/gather/gatherers/meta-elements.js +++ b/lighthouse-core/gather/gatherers/meta-elements.js @@ -8,7 +8,7 @@ const Gatherer = require('./gatherer.js'); const pageFunctions = require('../../lib/page-functions.js'); -/* global getElementsInDocument */ +/* globals getElementsInDocument */ /* istanbul ignore next */ function collectMetaElements() { From 9eea206719ee153879c41eb1d87dfe95d57f3768 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 10 Nov 2020 12:44:22 -0600 Subject: [PATCH 13/37] wip --- docs/recipes/custom-audit/searchable-gatherer.js | 2 +- docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js | 6 +++--- lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html | 2 +- lighthouse-core/gather/driver.js | 3 +-- lighthouse-core/gather/gatherers/accessibility.js | 2 +- lighthouse-core/lib/page-functions.js | 3 ++- 6 files changed, 9 insertions(+), 9 deletions(-) diff --git a/docs/recipes/custom-audit/searchable-gatherer.js b/docs/recipes/custom-audit/searchable-gatherer.js index 3606985a3a71..3daba4c1f61b 100644 --- a/docs/recipes/custom-audit/searchable-gatherer.js +++ b/docs/recipes/custom-audit/searchable-gatherer.js @@ -15,7 +15,7 @@ class TimeToSearchable extends Gatherer { afterPass(options) { const driver = options.driver; - return driver.evaluateAsync('window.myLoadMetrics') + return driver.evaluate('window.myLoadMetrics') // Ensure returned value is what we expect. .then(loadMetrics => { if (!loadMetrics || loadMetrics.searchableTime === undefined) { diff --git a/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js b/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js index cba9ab0b5ca1..32c42a8cb332 100644 --- a/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js +++ b/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js @@ -33,13 +33,13 @@ class CustomGatherer extends Gatherer { el.type = 'number'; document.body.append(el); } - await driver.evaluateAsync(`(${makeInput})()`); + await driver.evaluate(`(${makeInput})()`); await new Promise(resolve => setTimeout(resolve, 100)); // Prove that `driver` (Lighthouse) and `page` (Puppeteer) are talking to the same page. - await driver.evaluateAsync(`document.querySelector('input').value = '1'`); + await driver.evaluate(`document.querySelector('input').value = '1'`); await page.type('input', '23', {delay: 300}); - const value = await driver.evaluateAsync(`document.querySelector('input').value`); + const value = await driver.evaluate(`document.querySelector('input').value`); if (value !== '123') throw new Error('huh?'); // No need to close the browser or page. Puppeteer doesn't own either of them. diff --git a/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html b/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html index 841ae069b6b2..c322a31420e0 100644 --- a/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html +++ b/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html @@ -445,7 +445,7 @@

Do better web tester page

} - diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 6812a38682b2..87cd423c13a1 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -853,7 +853,6 @@ class Driver { let lastTimeout; let canceled = false; - const checkForQuietExpression = `(${pageFunctions.checkTimeSinceLastLongTaskString})()`; /** * @param {Driver} driver * @param {() => void} resolve @@ -861,7 +860,7 @@ class Driver { */ async function checkForQuiet(driver, resolve) { if (canceled) return; - const timeSinceLongTask = await driver.evaluateAsync(checkForQuietExpression); + const timeSinceLongTask = await driver.evaluate(pageFunctions.checkTimeSinceLastLongTask); if (canceled) return; if (typeof timeSinceLongTask === 'number') { diff --git a/lighthouse-core/gather/gatherers/accessibility.js b/lighthouse-core/gather/gatherers/accessibility.js index 95bdd441de99..a814153d5d8a 100644 --- a/lighthouse-core/gather/gatherers/accessibility.js +++ b/lighthouse-core/gather/gatherers/accessibility.js @@ -105,7 +105,7 @@ class Accessibility extends Gatherer { return (${runA11yChecks.toString()}()); })()`; - return driver.evaluateAsync(expression, {useIsolation: true}).then(returnedValue => { + return driver.evaluate(expression, {useIsolation: true}).then(returnedValue => { if (!returnedValue) { throw new Error('No axe-core results returned'); } diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index 8e2fe96a27af..754af2d9061d 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -90,6 +90,7 @@ function registerPerformanceObserverInPage() { /** * Used by _waitForCPUIdle and executed in the context of the page, returns time since last long task. + * @return {number} */ /* istanbul ignore next */ function checkTimeSinceLastLongTask() { @@ -498,7 +499,7 @@ module.exports = { createEvalCode, wrapRuntimeEvalErrorInBrowserString: wrapRuntimeEvalErrorInBrowser.toString(), registerPerformanceObserverInPageString: registerPerformanceObserverInPage.toString(), - checkTimeSinceLastLongTaskString: checkTimeSinceLastLongTask.toString(), + checkTimeSinceLastLongTask, getElementsInDocument, getElementsInDocumentString: getElementsInDocument.toString(), getOuterHTMLSnippetString: getOuterHTMLSnippet.toString(), From 2088109cc391cb7dda95ad32f63837da573332aa Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 2 Dec 2020 15:34:12 -0600 Subject: [PATCH 14/37] fix --- .../custom-audit/searchable-gatherer.js | 2 +- .../custom-gatherer.js | 6 +- .../test/fixtures/dobetterweb/dbw_tester.html | 2 +- lighthouse-core/gather/driver.js | 1 - .../gather/gatherers/script-elements.js | 1 + lighthouse-core/lib/page-functions.js | 1 - .../test/lib/page-functions-test.js | 86 ------------------- types/gatherer.d.ts | 1 + 8 files changed, 7 insertions(+), 93 deletions(-) diff --git a/docs/recipes/custom-audit/searchable-gatherer.js b/docs/recipes/custom-audit/searchable-gatherer.js index 3daba4c1f61b..3606985a3a71 100644 --- a/docs/recipes/custom-audit/searchable-gatherer.js +++ b/docs/recipes/custom-audit/searchable-gatherer.js @@ -15,7 +15,7 @@ class TimeToSearchable extends Gatherer { afterPass(options) { const driver = options.driver; - return driver.evaluate('window.myLoadMetrics') + return driver.evaluateAsync('window.myLoadMetrics') // Ensure returned value is what we expect. .then(loadMetrics => { if (!loadMetrics || loadMetrics.searchableTime === undefined) { diff --git a/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js b/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js index 32c42a8cb332..cba9ab0b5ca1 100644 --- a/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js +++ b/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js @@ -33,13 +33,13 @@ class CustomGatherer extends Gatherer { el.type = 'number'; document.body.append(el); } - await driver.evaluate(`(${makeInput})()`); + await driver.evaluateAsync(`(${makeInput})()`); await new Promise(resolve => setTimeout(resolve, 100)); // Prove that `driver` (Lighthouse) and `page` (Puppeteer) are talking to the same page. - await driver.evaluate(`document.querySelector('input').value = '1'`); + await driver.evaluateAsync(`document.querySelector('input').value = '1'`); await page.type('input', '23', {delay: 300}); - const value = await driver.evaluate(`document.querySelector('input').value`); + const value = await driver.evaluateAsync(`document.querySelector('input').value`); if (value !== '123') throw new Error('huh?'); // No need to close the browser or page. Puppeteer doesn't own either of them. diff --git a/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html b/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html index c322a31420e0..841ae069b6b2 100644 --- a/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html +++ b/lighthouse-cli/test/fixtures/dobetterweb/dbw_tester.html @@ -445,7 +445,7 @@

Do better web tester page

} - diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 6c5f2f055d6e..029c7dbc8837 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -471,7 +471,6 @@ class Driver { * will be evaluated in a content script that has access to the page's DOM but whose JavaScript state * is completely separate. * Returns a promise that resolves on the expression's value. - * @template T * @param {string} expression * @param {{useIsolation?: boolean}=} options * @return {Promise<*>} diff --git a/lighthouse-core/gather/gatherers/script-elements.js b/lighthouse-core/gather/gatherers/script-elements.js index d5e37a2ee707..2a2622d212d9 100644 --- a/lighthouse-core/gather/gatherers/script-elements.js +++ b/lighthouse-core/gather/gatherers/script-elements.js @@ -18,6 +18,7 @@ const pageFunctions = require('../../lib/page-functions.js'); /* istanbul ignore next */ function collectAllScriptElements() { /** @type {HTMLScriptElement[]} */ + // @ts-expect-error - getElementsInDocument put into scope via stringification const scripts = getElementsInDocument('script'); // eslint-disable-line no-undef return scripts.map(script => { diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index 48a9e18a3e9e..74218643c7f2 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -509,7 +509,6 @@ const getNodeDetailsString = `function getNodeDetails(element) { }`; module.exports = { - createEvalCode, wrapRuntimeEvalErrorInBrowserString: wrapRuntimeEvalErrorInBrowser.toString(), registerPerformanceObserverInPageString: registerPerformanceObserverInPage.toString(), checkTimeSinceLastLongTask, diff --git a/lighthouse-core/test/lib/page-functions-test.js b/lighthouse-core/test/lib/page-functions-test.js index 7b3427bebb49..90e8be1c2b25 100644 --- a/lighthouse-core/test/lib/page-functions-test.js +++ b/lighthouse-core/test/lib/page-functions-test.js @@ -12,92 +12,6 @@ const pageFunctions = require('../../lib/page-functions.js'); /* eslint-env jest */ -describe('createEvalCode', () => { - it('iife basic', () => { - function mainFn() { - return true; - } - const code = pageFunctions.createEvalCode(mainFn, { - mode: 'iife', - }); - expect(code).toMatchInlineSnapshot(` - "(() => { - - function mainFn() { - return true; - } - return mainFn(); - })()" - `); - expect(eval(code)).toEqual(true); - }); - - it('iife complex', () => { - function mainFn({a, b}, passThru) { - return {a: abs(a), b: square(b), passThru}; - } - function abs(val) { - return Math.abs(val); - } - function square(val) { - return val * val; - } - const code = pageFunctions.createEvalCode(mainFn, { - args: [{a: -5, b: 10}, 'hello'], - deps: [abs, square], - mode: 'iife', - }); - expect(code).toMatchInlineSnapshot(` - "(() => { - function abs(val) { - return Math.abs(val); - } - function square(val) { - return val * val; - } - function mainFn({ - a, - b - }, passThru) { - return { - a: abs(a), - b: square(b), - passThru - }; - } - return mainFn({\\"a\\":-5,\\"b\\":10},\\"hello\\"); - })()" - `); - expect(eval(code)).toEqual({a: 5, b: 100, passThru: 'hello'}); - }); - - it('function', () => { - function mainFn(a, b) { - return adder(a, b); - } - function adder(a, b) { - return a + b; - } - const code = pageFunctions.createEvalCode(mainFn, { - args: [1, 2], - mode: 'function', - deps: [adder], - }); - expect(code).toMatchInlineSnapshot(` - "function () { - function adder(a, b) { - return a + b; - } - function mainFn(a, b) { - return adder(a, b); - } - return mainFn.call(this, 1,2); - }" - `); - expect(eval(`(${code})()`)).toEqual(3); - }); -}); - describe('Page Functions', () => { let dom; diff --git a/types/gatherer.d.ts b/types/gatherer.d.ts index 0a49db236e7b..95c8497045a2 100644 --- a/types/gatherer.d.ts +++ b/types/gatherer.d.ts @@ -26,6 +26,7 @@ declare global { export interface FRTransitionalDriver { defaultSession: FRProtocolSession; evaluateAsync(expression: string, options?: {useIsolation?: boolean}): Promise; + evaluate(mainFn: (...args: T) => R, options: {args: T, useIsolation?: boolean, deps?: Array}): Promise; } /** The limited context interface shared between pre and post Fraggle Rock Lighthouse. */ From 7b01e8cd8751e770d2169b6b71dfe5ce49204add Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 2 Dec 2020 16:53:39 -0600 Subject: [PATCH 15/37] wip --- .../custom-gatherer.js | 2 +- lighthouse-core/gather/driver.js | 2 +- .../gather/driver/execution-context.js | 2 +- .../gather/gatherers/accessibility.js | 14 +++--- .../gather/gatherers/anchor-elements.js | 16 +++---- .../gather/gatherers/cache-contents.js | 7 +-- .../gather/gatherers/dobetterweb/doctype.js | 2 +- .../gather/gatherers/dobetterweb/domstats.js | 21 ++++---- .../password-inputs-with-prevented-paste.js | 10 ++-- .../dobetterweb/tags-blocking-first-paint.js | 5 +- .../gather/gatherers/full-page-screenshot.js | 23 +++++---- .../gatherers/html-without-javascript.js | 3 +- .../gather/gatherers/iframe-elements.js | 17 ++++--- .../gather/gatherers/image-elements.js | 35 +++++++------- .../gather/gatherers/seo/embedded-content.js | 48 +++++++++++-------- .../gather/gatherers/seo/robots-txt.js | 3 +- .../gather/gatherers/viewport-dimensions.js | 7 +-- lighthouse-core/lib/stack-collector.js | 10 ++-- .../test/lib/stack-collector-test.js | 6 +-- types/externs.d.ts | 2 + types/gatherer.d.ts | 2 +- 21 files changed, 121 insertions(+), 116 deletions(-) diff --git a/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js b/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js index cba9ab0b5ca1..faeb46443b25 100644 --- a/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js +++ b/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js @@ -33,7 +33,7 @@ class CustomGatherer extends Gatherer { el.type = 'number'; document.body.append(el); } - await driver.evaluateAsync(`(${makeInput})()`); + await driver.evaluate(makeInput, {args: []}); await new Promise(resolve => setTimeout(resolve, 100)); // Prove that `driver` (Lighthouse) and `page` (Puppeteer) are talking to the same page. diff --git a/lighthouse-core/gather/driver.js b/lighthouse-core/gather/driver.js index 029c7dbc8837..192d9ab5c675 100644 --- a/lighthouse-core/gather/driver.js +++ b/lighthouse-core/gather/driver.js @@ -489,7 +489,7 @@ class Driver { * @param {{args: T, useIsolation?: boolean, deps?: Array}} options `args` should * match the args of `mainFn`, and can be any serializable value. `deps` are functions that must be * defined for `mainFn` to work. - * @return {Promise} + * @return {FlattenedPromise} */ async evaluate(mainFn, options) { return this._executionContext.evaluate(mainFn, options); diff --git a/lighthouse-core/gather/driver/execution-context.js b/lighthouse-core/gather/driver/execution-context.js index 8964219c39cb..ef4fe1ffa8c9 100644 --- a/lighthouse-core/gather/driver/execution-context.js +++ b/lighthouse-core/gather/driver/execution-context.js @@ -160,7 +160,7 @@ class ExecutionContext { * @param {{args: T, useIsolation?: boolean, deps?: Array}} options `args` should * match the args of `mainFn`, and can be any serializable value. `deps` are functions that must be * defined for `mainFn` to work. - * @return {Promise} + * @return {FlattenedPromise} */ evaluate(mainFn, options) { const argsSerialized = options.args.map(arg => JSON.stringify(arg)).join(','); diff --git a/lighthouse-core/gather/gatherers/accessibility.js b/lighthouse-core/gather/gatherers/accessibility.js index 1df9c29bb243..89bf2cd21bf1 100644 --- a/lighthouse-core/gather/gatherers/accessibility.js +++ b/lighthouse-core/gather/gatherers/accessibility.js @@ -99,13 +99,15 @@ class Accessibility extends Gatherer { */ afterPass(passContext) { const driver = passContext.driver; - const expression = `(function () { - ${pageFunctions.getNodeDetailsString}; - ${axeLibSource}; - return (${runA11yChecks.toString()}()); - })()`; - return driver.evaluate(expression, {useIsolation: true}).then(returnedValue => { + return driver.evaluate(runA11yChecks, { + args: [], + useIsolation: true, + deps: [ + axeLibSource, + pageFunctions.getNodeDetailsString, + ], + }).then(returnedValue => { if (!returnedValue) { throw new Error('No axe-core results returned'); } diff --git a/lighthouse-core/gather/gatherers/anchor-elements.js b/lighthouse-core/gather/gatherers/anchor-elements.js index 746b549c53eb..fc128a1b04d9 100644 --- a/lighthouse-core/gather/gatherers/anchor-elements.js +++ b/lighthouse-core/gather/gatherers/anchor-elements.js @@ -95,15 +95,15 @@ class AnchorElements extends Gatherer { */ async afterPass(passContext) { const driver = passContext.driver; - const expression = `(() => { - ${pageFunctions.getElementsInDocumentString}; - ${pageFunctions.getNodeDetailsString}; - return (${collectAnchorElements})(); - })()`; - - /** @type {LH.Artifacts['AnchorElements']} */ - const anchors = await driver.evaluateAsync(expression, {useIsolation: true}); + const anchors = await driver.evaluate(collectAnchorElements, { + args: [], + useIsolation: true, + deps: [ + pageFunctions.getElementsInDocumentString, + pageFunctions.getNodeDetailsString, + ], + }); await driver.sendCommand('DOM.enable'); // DOM.getDocument is necessary for pushNodesByBackendIdsToFrontend to properly retrieve nodeIds if the `DOM` domain was enabled before this gatherer, invoke it to be safe. diff --git a/lighthouse-core/gather/gatherers/cache-contents.js b/lighthouse-core/gather/gatherers/cache-contents.js index fdf2fe6d489b..e04c10801289 100644 --- a/lighthouse-core/gather/gatherers/cache-contents.js +++ b/lighthouse-core/gather/gatherers/cache-contents.js @@ -46,12 +46,7 @@ class CacheContents extends Gatherer { async afterPass(passContext) { const driver = passContext.driver; - /** @type {Array|void} */ - const cacheUrls = await driver.evaluateAsync(`(${getCacheContents.toString()}())`); - if (!cacheUrls || !Array.isArray(cacheUrls)) { - throw new Error('Unable to retrieve cache contents'); - } - + const cacheUrls = await driver.evaluate(getCacheContents, {args: []}); return cacheUrls; } } diff --git a/lighthouse-core/gather/gatherers/dobetterweb/doctype.js b/lighthouse-core/gather/gatherers/dobetterweb/doctype.js index ee0579496587..d404e713f942 100644 --- a/lighthouse-core/gather/gatherers/dobetterweb/doctype.js +++ b/lighthouse-core/gather/gatherers/dobetterweb/doctype.js @@ -29,7 +29,7 @@ class Doctype extends Gatherer { */ afterPass(passContext) { const driver = passContext.driver; - return driver.evaluateAsync(`(${getDoctype.toString()}())`); + return driver.evaluate(getDoctype, {args: []}); } } diff --git a/lighthouse-core/gather/gatherers/dobetterweb/domstats.js b/lighthouse-core/gather/gatherers/dobetterweb/domstats.js index 0bdb4dfcd8af..ac67c051cfa2 100644 --- a/lighthouse-core/gather/gatherers/dobetterweb/domstats.js +++ b/lighthouse-core/gather/gatherers/dobetterweb/domstats.js @@ -3,20 +3,19 @@ * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. */ -// @ts-nocheck + /** * @fileoverview Gathers stats about the max height and width of the DOM tree * and total number of elements used on the page. */ -/* global getNodeDetails */ +/* global getNodeDetails document */ 'use strict'; const Gatherer = require('../gatherer.js'); const pageFunctions = require('../../../lib/page-functions.js'); - /** * Calculates the maximum tree depth of the DOM. * @param {HTMLElement} element Root of the tree to look in. @@ -24,7 +23,7 @@ const pageFunctions = require('../../../lib/page-functions.js'); * @return {LH.Artifacts.DOMStats} */ /* istanbul ignore next */ -function getDOMStats(element, deep = true) { +function getDOMStats(element = document.body, deep = true) { let deepestElement = null; let maxDepth = -1; let maxWidth = -1; @@ -32,7 +31,7 @@ function getDOMStats(element, deep = true) { let parentWithMostChildren = null; /** - * @param {Element} element + * @param {Element|ShadowRoot} element * @param {number} depth */ const _calcDOMWidthAndHeight = function(element, depth = 1) { @@ -64,10 +63,12 @@ function getDOMStats(element, deep = true) { return { depth: { max: result.maxDepth, + // @ts-expect-error - getNodeDetails put into scope via stringification ...getNodeDetails(deepestElement), }, width: { max: result.maxWidth, + // @ts-expect-error - getNodeDetails put into scope via stringification ...getNodeDetails(parentWithMostChildren), }, totalBodyElements: result.numElements, @@ -82,12 +83,12 @@ class DOMStats extends Gatherer { async afterPass(passContext) { const driver = passContext.driver; - const expression = `(function() { - ${pageFunctions.getNodeDetailsString}; - return (${getDOMStats.toString()}(document.body)); - })()`; await driver.sendCommand('DOM.enable'); - const results = await driver.evaluateAsync(expression, {useIsolation: true}); + const results = await driver.evaluate(getDOMStats, { + args: [], + useIsolation: true, + deps: [pageFunctions.getNodeDetailsString], + }); await driver.sendCommand('DOM.disable'); return results; } diff --git a/lighthouse-core/gather/gatherers/dobetterweb/password-inputs-with-prevented-paste.js b/lighthouse-core/gather/gatherers/dobetterweb/password-inputs-with-prevented-paste.js index 49d7d9251427..84548e5dda5a 100644 --- a/lighthouse-core/gather/gatherers/dobetterweb/password-inputs-with-prevented-paste.js +++ b/lighthouse-core/gather/gatherers/dobetterweb/password-inputs-with-prevented-paste.js @@ -34,12 +34,10 @@ class PasswordInputsWithPreventedPaste extends Gatherer { * @return {Promise} */ afterPass(passContext) { - const expression = `(() => { - ${pageFunctions.getNodeDetailsString}; - return (${findPasswordInputsWithPreventedPaste.toString()}()); - })()`; - - return passContext.driver.evaluateAsync(expression); + return passContext.driver.evaluate(findPasswordInputsWithPreventedPaste, { + args: [], + deps: [pageFunctions.getNodeDetailsString], + }); } } diff --git a/lighthouse-core/gather/gatherers/dobetterweb/tags-blocking-first-paint.js b/lighthouse-core/gather/gatherers/dobetterweb/tags-blocking-first-paint.js index 2f89cc7efff6..be5324d3695f 100644 --- a/lighthouse-core/gather/gatherers/dobetterweb/tags-blocking-first-paint.js +++ b/lighthouse-core/gather/gatherers/dobetterweb/tags-blocking-first-paint.js @@ -42,7 +42,7 @@ function installMediaListener() { } /** - * @return {Promise<{tagName: string, url: string, src: string, href: string, rel: string, media: string, disabled: boolean, mediaChanges: {href: string, media: string, msSinceHTMLEnd: number, matches: boolean}}>} + * @return {Promise>} */ /* istanbul ignore next */ function collectTagsThatBlockFirstPaint() { @@ -132,12 +132,11 @@ class TagsBlockingFirstPaint extends Gatherer { * @param {Array} networkRecords */ static findBlockingTags(driver, networkRecords) { - const scriptSrc = `(${collectTagsThatBlockFirstPaint.toString()}())`; const firstRequestEndTime = networkRecords.reduce( (min, record) => Math.min(min, record.endTime), Infinity ); - return driver.evaluateAsync(scriptSrc).then(tags => { + return driver.evaluate(collectTagsThatBlockFirstPaint, {args: []}).then(tags => { const requests = TagsBlockingFirstPaint._filteredAndIndexedByUrl(networkRecords); return tags.reduce((prev, tag) => { diff --git a/lighthouse-core/gather/gatherers/full-page-screenshot.js b/lighthouse-core/gather/gatherers/full-page-screenshot.js index 4e03a28c21d9..a69e215df8ea 100644 --- a/lighthouse-core/gather/gatherers/full-page-screenshot.js +++ b/lighthouse-core/gather/gatherers/full-page-screenshot.js @@ -92,18 +92,23 @@ class FullPageScreenshot extends Gatherer { return nodes; } - const expression = `(function () { - ${pageFunctions.getBoundingClientRectString}; - return (${resolveNodes.toString()}()); - })()`; + + /** + * @param {boolean} useIsolation + */ + function evaluateInPage(useIsolation) { + return passContext.driver.evaluate(resolveNodes, { + args: [], + useIsolation, + deps: [pageFunctions.getBoundingClientRectString], + }); + } // Collect nodes with the page context (`useIsolation: false`) and with our own, reused - // context (useIsolation: false). Gatherers use both modes when collecting node details, + // context (`useIsolation: false`). Gatherers use both modes when collecting node details, // so we must do the same here too. - const pageContextResult = - await passContext.driver.evaluateAsync(expression, {useIsolation: false}); - const isolatedContextResult = - await passContext.driver.evaluateAsync(expression, {useIsolation: true}); + const pageContextResult = await evaluateInPage(false); + const isolatedContextResult = await evaluateInPage(true); return {...pageContextResult, ...isolatedContextResult}; } diff --git a/lighthouse-core/gather/gatherers/html-without-javascript.js b/lighthouse-core/gather/gatherers/html-without-javascript.js index 4625b08fc92c..b949934e4d09 100644 --- a/lighthouse-core/gather/gatherers/html-without-javascript.js +++ b/lighthouse-core/gather/gatherers/html-without-javascript.js @@ -40,8 +40,7 @@ class HTMLWithoutJavaScript extends Gatherer { // Reset the JS disable. passContext.disableJavaScript = false; - const expression = `(${getBodyText.toString()}())`; - const {bodyText, hasNoScript} = await passContext.driver.evaluateAsync(expression); + const {bodyText, hasNoScript} = await passContext.driver.evaluate(getBodyText, {args: []}); if (typeof bodyText !== 'string') { throw new Error('document body innerText returned by protocol was not a string'); } diff --git a/lighthouse-core/gather/gatherers/iframe-elements.js b/lighthouse-core/gather/gatherers/iframe-elements.js index 01853f1baabe..9262ad576fc3 100644 --- a/lighthouse-core/gather/gatherers/iframe-elements.js +++ b/lighthouse-core/gather/gatherers/iframe-elements.js @@ -43,15 +43,14 @@ class IFrameElements extends Gatherer { async afterPass(passContext) { const driver = passContext.driver; - const expression = `(() => { - ${pageFunctions.getElementsInDocumentString}; - ${pageFunctions.isPositionFixedString}; - ${pageFunctions.getNodeDetailsString}; - return (${collectIFrameElements})(); - })()`; - - /** @type {LH.Artifacts['IFrameElements']} */ - const iframeElements = await driver.evaluateAsync(expression, {useIsolation: true}); + const iframeElements = await driver.evaluate(collectIFrameElements, { + args: [], + deps: [ + pageFunctions.getElementsInDocumentString, + pageFunctions.isPositionFixedString, + pageFunctions.getNodeDetailsString, + ], + }); return iframeElements; } } diff --git a/lighthouse-core/gather/gatherers/image-elements.js b/lighthouse-core/gather/gatherers/image-elements.js index 7e46ce6e032b..353e600701b9 100644 --- a/lighthouse-core/gather/gatherers/image-elements.js +++ b/lighthouse-core/gather/gatherers/image-elements.js @@ -238,7 +238,7 @@ class ImageElements extends Gatherer { * @return {Promise} */ async fetchElementWithSizeInformation(driver, element) { - const url = JSON.stringify(element.src); + const url = element.src; if (this._naturalSizeCache.has(url)) { return Object.assign(element, this._naturalSizeCache.get(url)); } @@ -246,8 +246,9 @@ class ImageElements extends Gatherer { try { // We don't want this to take forever, 250ms should be enough for images that are cached driver.setNextProtocolTimeout(250); - /** @type {{naturalWidth: number, naturalHeight: number}} */ - const size = await driver.evaluateAsync(`(${determineNaturalSize.toString()})(${url})`); + const size = await driver.evaluate(determineNaturalSize, { + args: [url], + }); this._naturalSizeCache.set(url, size); return Object.assign(element, size); } catch (_) { @@ -301,21 +302,19 @@ class ImageElements extends Gatherer { return map; }, /** @type {Object} */ ({})); - const expression = `(function() { - ${pageFunctions.getElementsInDocumentString}; // define function on page - ${pageFunctions.getBoundingClientRectString}; - ${pageFunctions.getNodeDetailsString}; - ${getClientRect.toString()}; - ${getPosition.toString()}; - ${getHTMLImages.toString()}; - ${getCSSImages.toString()}; - ${collectImageElementInfo.toString()}; - - return collectImageElementInfo(); - })()`; - - /** @type {Array} */ - const elements = await driver.evaluateAsync(expression); + const elements = await driver.evaluate(collectImageElementInfo, { + args: [], + deps: [ + pageFunctions.getElementsInDocumentString, + pageFunctions.getBoundingClientRectString, + pageFunctions.getNodeDetailsString, + getClientRect, + getPosition, + getHTMLImages, + getCSSImages, + collectImageElementInfo, + ], + }); /** @type {Array} */ const imageUsage = []; diff --git a/lighthouse-core/gather/gatherers/seo/embedded-content.js b/lighthouse-core/gather/gatherers/seo/embedded-content.js index 4d510aeee91c..a4554d40d672 100644 --- a/lighthouse-core/gather/gatherers/seo/embedded-content.js +++ b/lighthouse-core/gather/gatherers/seo/embedded-content.js @@ -5,36 +5,42 @@ */ 'use strict'; +/* globals getElementsInDocument */ + const Gatherer = require('../gatherer.js'); const pageFunctions = require('../../../lib/page-functions.js'); +function getEmbeddedContent() { + const selector = 'object, embed, applet'; + /** @type {HTMLElement[]} */ + // @ts-expect-error - getElementsInDocument put into scope via stringification + const elements = getElementsInDocument(selector); + return elements + .map(node => ({ + tagName: node.tagName, + type: node.getAttribute('type'), + src: node.getAttribute('src'), + data: node.getAttribute('data'), + code: node.getAttribute('code'), + params: Array.from(node.children) + .filter(el => el.tagName === 'PARAM') + .map(el => ({ + name: el.getAttribute('name') || '', + value: el.getAttribute('value') || '', + })), + })); +} + class EmbeddedContent extends Gatherer { /** * @param {LH.Gatherer.PassContext} passContext * @return {Promise} */ afterPass(passContext) { - const expression = `(function() { - ${pageFunctions.getElementsInDocumentString}; // define function on page - const selector = 'object, embed, applet'; - const elements = getElementsInDocument(selector); - return elements - .map(node => ({ - tagName: node.tagName, - type: node.getAttribute('type'), - src: node.getAttribute('src'), - data: node.getAttribute('data'), - code: node.getAttribute('code'), - params: Array.from(node.children) - .filter(el => el.tagName === 'PARAM') - .map(el => ({ - name: el.getAttribute('name') || '', - value: el.getAttribute('value') || '', - })), - })); - })()`; - - return passContext.driver.evaluateAsync(expression); + return passContext.driver.evaluate(getEmbeddedContent, { + args: [], + deps: [pageFunctions.getElementsInDocument], + }); } } diff --git a/lighthouse-core/gather/gatherers/seo/robots-txt.js b/lighthouse-core/gather/gatherers/seo/robots-txt.js index 749827f94a53..7a0b1294ef01 100644 --- a/lighthouse-core/gather/gatherers/seo/robots-txt.js +++ b/lighthouse-core/gather/gatherers/seo/robots-txt.js @@ -32,7 +32,8 @@ class RobotsTxt extends Gatherer { * @return {Promise} */ afterPass(passContext) { - return passContext.driver.evaluateAsync(`(${getRobotsTxtContent.toString()}())`, { + return passContext.driver.evaluate(getRobotsTxtContent, { + args: [], useIsolation: true, }); } diff --git a/lighthouse-core/gather/gatherers/viewport-dimensions.js b/lighthouse-core/gather/gatherers/viewport-dimensions.js index f5376df26cd9..a01a30fa911f 100644 --- a/lighthouse-core/gather/gatherers/viewport-dimensions.js +++ b/lighthouse-core/gather/gatherers/viewport-dimensions.js @@ -34,9 +34,10 @@ class ViewportDimensions extends Gatherer { async afterPass(passContext) { const driver = passContext.driver; - /** @type {LH.Artifacts.ViewportDimensions} */ - const dimensions = await driver.evaluateAsync(`(${getViewportDimensions.toString()}())`, - {useIsolation: true}); + const dimensions = await driver.evaluate(getViewportDimensions, { + args: [], + useIsolation: true, + }); const allNumeric = Object.values(dimensions).every(Number.isFinite); if (!allNumeric) { diff --git a/lighthouse-core/lib/stack-collector.js b/lighthouse-core/lib/stack-collector.js index 5b301df11183..80bb0a411585 100644 --- a/lighthouse-core/lib/stack-collector.js +++ b/lighthouse-core/lib/stack-collector.js @@ -82,13 +82,11 @@ async function detectLibraries() { async function collectStacks(passContext) { const status = {msg: 'Collect stacks', id: 'lh:gather:collectStacks'}; log.time(status); - const expression = `(function () { - ${libDetectorSource}; - return (${detectLibraries.toString()}()); - })()`; - /** @type {JSLibrary[]} */ - const jsLibraries = await passContext.driver.evaluateAsync(expression); + const jsLibraries = await passContext.driver.evaluate(detectLibraries, { + args: [], + deps: [libDetectorSource], + }); const stacks = jsLibraries.map(lib => ({ detector: /** @type {'js'} */ ('js'), diff --git a/lighthouse-core/test/lib/stack-collector-test.js b/lighthouse-core/test/lib/stack-collector-test.js index 97147df27391..bf2008081b76 100644 --- a/lighthouse-core/test/lib/stack-collector-test.js +++ b/lighthouse-core/test/lib/stack-collector-test.js @@ -10,15 +10,15 @@ const collectStacks = require('../../lib/stack-collector.js'); describe('stack collector', () => { - /** @type {{driver: {evaluateAsync: jest.Mock}}} */ + /** @type {{driver: {evaluate: jest.Mock}}} */ let passContext; beforeEach(() => { - passContext = {driver: {evaluateAsync: jest.fn()}}; + passContext = {driver: {evaluate: jest.fn()}}; }); it('returns the detected stacks', async () => { - passContext.driver.evaluateAsync.mockResolvedValue([ + passContext.driver.evaluate.mockResolvedValue([ {id: 'jquery', name: 'jQuery', version: '2.1.0', npm: 'jquery'}, {id: 'angular', name: 'Angular', version: '', npm: ''}, {id: 'magento', name: 'Magento', version: 2}, diff --git a/types/externs.d.ts b/types/externs.d.ts index 6a85747329a9..b80fcd80c85c 100644 --- a/types/externs.d.ts +++ b/types/externs.d.ts @@ -64,6 +64,8 @@ declare global { type FirstParamType any> = T extends (arg1: infer P, ...args: any[]) => any ? P : never; + type FlattenedPromise = Promise ? X : A>; + module LH { // re-export useful type modules under global LH module. export import Crdp = _Crdp; diff --git a/types/gatherer.d.ts b/types/gatherer.d.ts index 95c8497045a2..972edf1d3725 100644 --- a/types/gatherer.d.ts +++ b/types/gatherer.d.ts @@ -26,7 +26,7 @@ declare global { export interface FRTransitionalDriver { defaultSession: FRProtocolSession; evaluateAsync(expression: string, options?: {useIsolation?: boolean}): Promise; - evaluate(mainFn: (...args: T) => R, options: {args: T, useIsolation?: boolean, deps?: Array}): Promise; + evaluate(mainFn: (...args: T) => R, options: {args: T, useIsolation?: boolean, deps?: Array}): FlattenedPromise; } /** The limited context interface shared between pre and post Fraggle Rock Lighthouse. */ From 4cc1eea98f36b0f1aa85bf8d14a1d020be19a61c Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 2 Dec 2020 17:08:22 -0600 Subject: [PATCH 16/37] fix --- lighthouse-core/fraggle-rock/gather/driver.js | 11 +++++++++++ .../gather/gatherers/form-elements.js | 16 ++++++++-------- 2 files changed, 19 insertions(+), 8 deletions(-) diff --git a/lighthouse-core/fraggle-rock/gather/driver.js b/lighthouse-core/fraggle-rock/gather/driver.js index 568790061194..77becfd5c312 100644 --- a/lighthouse-core/fraggle-rock/gather/driver.js +++ b/lighthouse-core/fraggle-rock/gather/driver.js @@ -55,6 +55,17 @@ class Driver { if (!this._executionContext) throw new Error('Driver not connected to page'); return this._executionContext.evaluateAsync(expression, options); } + + /** + * @template {any[]} T, R + * @param {((...args: T) => R)} mainFn + * @param {{args: T, useIsolation?: boolean, deps?: Array}} options + * @return {FlattenedPromise} + */ + async evaluate(mainFn, options) { + if (!this._executionContext) throw new Error('Driver not connected to page'); + return this._executionContext.evaluate(mainFn, options); + } } module.exports = Driver; diff --git a/lighthouse-core/gather/gatherers/form-elements.js b/lighthouse-core/gather/gatherers/form-elements.js index 56b8df6a1ac2..4cd668527806 100644 --- a/lighthouse-core/gather/gatherers/form-elements.js +++ b/lighthouse-core/gather/gatherers/form-elements.js @@ -90,14 +90,14 @@ class FormElements extends Gatherer { async afterPass(passContext) { const driver = passContext.driver; - const expression = `(() => { - ${pageFunctions.getElementsInDocumentString}; - ${pageFunctions.getNodeDetailsString}; - return (${collectFormElements})(); - })()`; - - /** @type {LH.Artifacts['FormElements']} */ - const formElements = await driver.evaluateAsync(expression, {useIsolation: true}); + const formElements = await driver.evaluate(collectFormElements, { + args: [], + useIsolation: true, + deps: [ + pageFunctions.getElementsInDocumentString, + pageFunctions.getNodeDetailsString, + ], + }); return formElements; } } From 77af88de45986e94517e09f6344803be391cdd1a Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Wed, 2 Dec 2020 18:00:11 -0600 Subject: [PATCH 17/37] fix tests --- .../gather/gatherers/full-page-screenshot.js | 19 ++++++++++++------- .../gather/gatherers/accessibility-test.js | 14 ++++++-------- ...ssword-inputs-with-prevented-paste-test.js | 6 +++--- .../tags-blocking-first-paint-test.js | 4 ++-- .../gatherers/full-page-screenshot-test.js | 11 ++++++----- .../gatherers/html-without-javascript-test.js | 12 ++++++------ .../gatherers/viewport-dimensions-test.js | 6 +++--- 7 files changed, 38 insertions(+), 34 deletions(-) diff --git a/lighthouse-core/gather/gatherers/full-page-screenshot.js b/lighthouse-core/gather/gatherers/full-page-screenshot.js index a69e215df8ea..3e2cba0f9c9f 100644 --- a/lighthouse-core/gather/gatherers/full-page-screenshot.js +++ b/lighthouse-core/gather/gatherers/full-page-screenshot.js @@ -5,7 +5,7 @@ */ 'use strict'; -/* globals window getBoundingClientRect */ +/* globals window document getBoundingClientRect */ const Gatherer = require('./gatherer.js'); const pageFunctions = require('../../lib/page-functions.js'); @@ -141,20 +141,25 @@ class FullPageScreenshot extends Gatherer { // in the LH runner api, which for ex. puppeteer consumers would setup puppeteer emulation, // and then just call that to reset? // https://github.com/GoogleChrome/lighthouse/issues/11122 - const observedDeviceMetrics = await driver.evaluateAsync(`(function() { + + const observedDeviceMetrics = await driver.evaluate(function getObservedDeviceMetrics() { + // Convert the Web API's snake case (landscape-primary) to camel case (landscapePrimary). + const screenOrientationType = /** @type {LH.Crdp.Emulation.ScreenOrientationType} */ ( + snakeCaseToCamelCase(window.screen.orientation.type)); return { width: document.documentElement.clientWidth, height: document.documentElement.clientHeight, screenOrientation: { - type: window.screen.orientation.type, + type: screenOrientationType, angle: window.screen.orientation.angle, }, deviceScaleFactor: window.devicePixelRatio, }; - })()`, {useIsolation: true}); - // Convert the Web API's snake case (landscape-primary) to camel case (landscapePrimary). - observedDeviceMetrics.screenOrientation.type = - snakeCaseToCamelCase(observedDeviceMetrics.screenOrientation.type); + }, { + args: [], + useIsolation: true, + deps: [snakeCaseToCamelCase], + }); await driver.sendCommand('Emulation.setDeviceMetricsOverride', { mobile: passContext.baseArtifacts.TestedAsMobileDevice, // could easily be wrong ...observedDeviceMetrics, diff --git a/lighthouse-core/test/gather/gatherers/accessibility-test.js b/lighthouse-core/test/gather/gatherers/accessibility-test.js index c64d21aab8c5..8b4d6413535d 100644 --- a/lighthouse-core/test/gather/gatherers/accessibility-test.js +++ b/lighthouse-core/test/gather/gatherers/accessibility-test.js @@ -20,9 +20,7 @@ describe('Accessibility gatherer', () => { it('fails if nothing is returned', () => { return accessibilityGather.afterPass({ driver: { - evaluateAsync() { - return Promise.resolve(); - }, + async evaluate() {}, }, }).then( _ => assert.ok(false), @@ -32,10 +30,10 @@ describe('Accessibility gatherer', () => { it('fails if result has no violations array', () => { return accessibilityGather.afterPass({ driver: { - evaluateAsync() { - return Promise.resolve({ + async evaluate() { + return { url: 'https://example.com', - }); + }; }, }, }).then( @@ -47,8 +45,8 @@ describe('Accessibility gatherer', () => { const error = 'There was an error.'; return accessibilityGather.afterPass({ driver: { - evaluateAsync() { - return Promise.reject(new Error(error)); + async evaluate() { + throw new Error(error); }, }, }).then( diff --git a/lighthouse-core/test/gather/gatherers/dobetterweb/password-inputs-with-prevented-paste-test.js b/lighthouse-core/test/gather/gatherers/dobetterweb/password-inputs-with-prevented-paste-test.js index 9537ccca44e0..3ade36c5c4dc 100644 --- a/lighthouse-core/test/gather/gatherers/dobetterweb/password-inputs-with-prevented-paste-test.js +++ b/lighthouse-core/test/gather/gatherers/dobetterweb/password-inputs-with-prevented-paste-test.js @@ -21,12 +21,12 @@ describe('PasswordInputsWithPreventedPaste gatherer', () => { return gatherer .afterPass({ driver: { - evaluateAsync() { - return Promise.resolve([ + async evaluate() { + return [ { snippet: '', }, - ]); + ]; }, }, }) diff --git a/lighthouse-core/test/gather/gatherers/dobetterweb/tags-blocking-first-paint-test.js b/lighthouse-core/test/gather/gatherers/dobetterweb/tags-blocking-first-paint-test.js index f558c8636469..9b61f6d04c2a 100644 --- a/lighthouse-core/test/gather/gatherers/dobetterweb/tags-blocking-first-paint-test.js +++ b/lighthouse-core/test/gather/gatherers/dobetterweb/tags-blocking-first-paint-test.js @@ -151,8 +151,8 @@ describe('First paint blocking tags', () => { return tagsBlockingFirstPaint.afterPass({ driver: { - evaluateAsync() { - return Promise.resolve([linkDetails, linkDetails, scriptDetails]); + async evaluate() { + return [linkDetails, linkDetails, scriptDetails]; }, }, }, traceData).then(artifact => { diff --git a/lighthouse-core/test/gather/gatherers/full-page-screenshot-test.js b/lighthouse-core/test/gather/gatherers/full-page-screenshot-test.js index c53469afb0b6..c75ee5f0f0f1 100644 --- a/lighthouse-core/test/gather/gatherers/full-page-screenshot-test.js +++ b/lighthouse-core/test/gather/gatherers/full-page-screenshot-test.js @@ -14,22 +14,23 @@ const FullPageScreenshotGatherer = require('../../../gather/gatherers/full-page- */ function createMockDriver({contentSize, screenSize, screenshotData}) { return { - evaluateAsync: async function(code) { - if (code === 'window.innerWidth') { + evaluate: async function(fn) { + if (fn.name === 'resolveNodes') { return contentSize.width; - } - if (code.includes('document.documentElement.clientWidth')) { + } else if (fn.name === 'getObservedDeviceMetrics') { return { width: screenSize.width, height: screenSize.height, screenWidth: screenSize.width, screenHeight: screenSize.height, screenOrientation: { - type: 'landscape-primary', + type: 'landscapePrimary', angle: 30, }, deviceScaleFactor: screenSize.dpr, }; + } else { + throw new Error(`unexpected fn ${fn.name}`); } }, beginEmulation: jest.fn(), diff --git a/lighthouse-core/test/gather/gatherers/html-without-javascript-test.js b/lighthouse-core/test/gather/gatherers/html-without-javascript-test.js index 8d16f6778579..7a29fc71a2b1 100644 --- a/lighthouse-core/test/gather/gatherers/html-without-javascript-test.js +++ b/lighthouse-core/test/gather/gatherers/html-without-javascript-test.js @@ -28,8 +28,8 @@ describe('HTML without JavaScript gatherer', () => { const opts = { disableJavaScript: true, driver: { - evaluateAsync() { - return Promise.resolve({bodyText: 'Hello!'}); + async evaluate() { + return {bodyText: 'Hello!'}; }, }, }; @@ -45,8 +45,8 @@ describe('HTML without JavaScript gatherer', () => { const hasNoScript = true; return htmlWithoutJavaScriptGather.afterPass({ driver: { - evaluateAsync() { - return Promise.resolve({bodyText, hasNoScript}); + async evaluate() { + return {bodyText, hasNoScript}; }, }, }).then(artifact => { @@ -57,8 +57,8 @@ describe('HTML without JavaScript gatherer', () => { it('throws an error when driver returns a non-string', () => { return htmlWithoutJavaScriptGather.afterPass({ driver: { - evaluateAsync() { - return Promise.resolve(null); + async evaluate() { + return null; }, }, }).then( diff --git a/lighthouse-core/test/gather/gatherers/viewport-dimensions-test.js b/lighthouse-core/test/gather/gatherers/viewport-dimensions-test.js index 8946314a6d21..4cdc467fed5b 100644 --- a/lighthouse-core/test/gather/gatherers/viewport-dimensions-test.js +++ b/lighthouse-core/test/gather/gatherers/viewport-dimensions-test.js @@ -20,14 +20,14 @@ describe('ViewportDimensions gatherer', () => { it('returns an artifact', () => { return gatherer.afterPass({ driver: { - evaluateAsync() { - return Promise.resolve({ + async evaluate() { + return { innerWidth: 400, outerWidth: 400, innerHeight: 600, outerHeight: 600, devicePixelRatio: 2, - }); + }; }, }, }).then(artifact => { From d35fd5e8776e0a873e39b2f908a03e288e041061 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Thu, 3 Dec 2020 10:35:32 -0600 Subject: [PATCH 18/37] fix --- lighthouse-core/gather/gatherers/viewport-dimensions.js | 6 +++--- lighthouse-core/test/gather/fake-driver.js | 3 +++ 2 files changed, 6 insertions(+), 3 deletions(-) diff --git a/lighthouse-core/gather/gatherers/viewport-dimensions.js b/lighthouse-core/gather/gatherers/viewport-dimensions.js index a01a30fa911f..173775084d51 100644 --- a/lighthouse-core/gather/gatherers/viewport-dimensions.js +++ b/lighthouse-core/gather/gatherers/viewport-dimensions.js @@ -10,20 +10,20 @@ const Gatherer = require('./gatherer.js'); /* global window */ /** - * @return {Promise} + * @return {LH.Artifacts.ViewportDimensions} */ /* istanbul ignore next */ function getViewportDimensions() { // window.innerWidth to get the scrollable size of the window (irrespective of zoom) // window.outerWidth to get the size of the visible area // window.devicePixelRatio to get ratio of logical pixels to physical pixels - return Promise.resolve({ + return { innerWidth: window.innerWidth, innerHeight: window.innerHeight, outerWidth: window.outerWidth, outerHeight: window.outerHeight, devicePixelRatio: window.devicePixelRatio, - }); + }; } class ViewportDimensions extends Gatherer { diff --git a/lighthouse-core/test/gather/fake-driver.js b/lighthouse-core/test/gather/fake-driver.js index db166317525b..4af9decceef1 100644 --- a/lighthouse-core/test/gather/fake-driver.js +++ b/lighthouse-core/test/gather/fake-driver.js @@ -71,6 +71,9 @@ function makeFakeDriver({protocolGetVersionResponse}) { evaluateAsync() { return Promise.resolve({}); }, + evaluate() { + return Promise.resolve({}); + }, /** @param {{x: number, y: number}} position */ scrollTo(position) { scrollPosition = position; From eb897cf6fd3885a5f45ef43982a36baaca31e5d8 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 21 Dec 2020 17:29:16 -0600 Subject: [PATCH 19/37] tmp --- lighthouse-core/audits/audit.js | 14 ++++++++++++++ .../audits/non-composited-animations.js | 6 ++++-- lighthouse-core/audits/unsized-images.js | 9 ++------- lighthouse-core/lib/page-functions.js | 11 ++++++----- types/audit-details.d.ts | 2 +- 5 files changed, 27 insertions(+), 15 deletions(-) diff --git a/lighthouse-core/audits/audit.js b/lighthouse-core/audits/audit.js index 2ec83e0fd72a..b76575cf697b 100644 --- a/lighthouse-core/audits/audit.js +++ b/lighthouse-core/audits/audit.js @@ -216,6 +216,20 @@ class Audit { }; } + /** + * @param {LH.Artifacts.NodeDetails} node + * @return {LH.Audit.Details.NodeValue} + */ + static makeNodeValue(node) { + const {devtoolsNodePath, ...rest} = node; + + return { + type: 'node', + ...rest, + path: devtoolsNodePath, + }; + } + /** * @param {number|null} score * @param {LH.Audit.ScoreDisplayMode} scoreDisplayMode diff --git a/lighthouse-core/audits/non-composited-animations.js b/lighthouse-core/audits/non-composited-animations.js index f59f34b6f16c..3f6904b4d4a1 100644 --- a/lighthouse-core/audits/non-composited-animations.js +++ b/lighthouse-core/audits/non-composited-animations.js @@ -129,18 +129,20 @@ 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 */ + /** @type {LH.Audit.Details.NodeValue} */ const node = { type: '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, }; const animations = element.animations || []; diff --git a/lighthouse-core/audits/unsized-images.js b/lighthouse-core/audits/unsized-images.js index b7bd308fbca8..eaae3d769d4e 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.makeNodeValue(image.node), }); } diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index 07f1e7ff1d3a..9ae9bc5cadb4 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -370,7 +370,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) { @@ -460,6 +460,7 @@ function wrapRequestIdleCallback(cpuSlowdownMultiplier) { /** * @param {HTMLElement} element + * @return {LH.Artifacts.NodeDetails} */ function getNodeDetailsImpl(element) { // This bookkeeping is for the FullPageScreenshot gatherer. @@ -490,11 +491,11 @@ function getNodeDetailsImpl(element) { const details = { lhId, - devtoolsNodePath: getNodePath(element), - selector: getNodeSelector(htmlElement), + devtoolsNodePath: getNodePath(element) || '', + selector: getNodeSelector(htmlElement) || '', boundingRect: getBoundingClientRect(htmlElement), - snippet: getOuterHTMLSnippet(element), - nodeLabel: getNodeLabel(htmlElement), + snippet: getOuterHTMLSnippet(element) || '', + nodeLabel: getNodeLabel(htmlElement) || '', }; return details; diff --git a/types/audit-details.d.ts b/types/audit-details.d.ts index 55b08112d687..a7e2534d6218 100644 --- a/types/audit-details.d.ts +++ b/types/audit-details.d.ts @@ -207,7 +207,7 @@ declare global { selector?: string; boundingRect?: Artifacts.Rect; /** An HTML snippet used to identify the node. */ - snippet?: string; + snippet: string; /** A human-friendly text descriptor that's used to identify the node more quickly. */ nodeLabel?: string; } From d643baf09ef49d4974ac7e75d84e8946b87d3f64 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 21 Dec 2020 17:46:26 -0600 Subject: [PATCH 20/37] update --- docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js | 2 +- lighthouse-core/fraggle-rock/gather/driver.js | 8 +++++--- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js b/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js index faeb46443b25..7f31143b5cad 100644 --- a/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js +++ b/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js @@ -33,7 +33,7 @@ class CustomGatherer extends Gatherer { el.type = 'number'; document.body.append(el); } - await driver.evaluate(makeInput, {args: []}); + await driver.evaluateAsync(makeInput, {args: []}); await new Promise(resolve => setTimeout(resolve, 100)); // Prove that `driver` (Lighthouse) and `page` (Puppeteer) are talking to the same page. diff --git a/lighthouse-core/fraggle-rock/gather/driver.js b/lighthouse-core/fraggle-rock/gather/driver.js index 77becfd5c312..04d69fb0f095 100644 --- a/lighthouse-core/fraggle-rock/gather/driver.js +++ b/lighthouse-core/fraggle-rock/gather/driver.js @@ -58,11 +58,13 @@ class Driver { /** * @template {any[]} T, R - * @param {((...args: T) => R)} mainFn - * @param {{args: T, useIsolation?: boolean, deps?: Array}} options + * @param {((...args: T) => R)} mainFn The main function to call. + * @param {{args: T, useIsolation?: boolean, deps?: Array}} options `args` should + * match the args of `mainFn`, and can be any serializable value. `deps` are functions that must be + * defined for `mainFn` to work. * @return {FlattenedPromise} */ - async evaluate(mainFn, options) { + evaluate(mainFn, options) { if (!this._executionContext) throw new Error('Driver not connected to page'); return this._executionContext.evaluate(mainFn, options); } From c526772eba46aece4e5d659bfa689de4de2244d2 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 21 Dec 2020 17:56:24 -0600 Subject: [PATCH 21/37] fixtest --- .../test/gather/gatherers/full-page-screenshot-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/test/gather/gatherers/full-page-screenshot-test.js b/lighthouse-core/test/gather/gatherers/full-page-screenshot-test.js index 307d3069597f..7b7f3ed5bc53 100644 --- a/lighthouse-core/test/gather/gatherers/full-page-screenshot-test.js +++ b/lighthouse-core/test/gather/gatherers/full-page-screenshot-test.js @@ -16,7 +16,7 @@ function createMockDriver({contentSize, screenSize, screenshotData}) { return { evaluate: async function(fn) { if (fn.name === 'resolveNodes') { - return contentSize.width; + return {}; } else if (fn.name === 'getObservedDeviceMetrics') { return { width: screenSize.width, From b893bb17105d92e2bf26b65783cbc544fcdccc98 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 21 Dec 2020 18:00:03 -0600 Subject: [PATCH 22/37] fix --- docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js b/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js index 7f31143b5cad..faeb46443b25 100644 --- a/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js +++ b/docs/recipes/custom-gatherer-puppeteer/custom-gatherer.js @@ -33,7 +33,7 @@ class CustomGatherer extends Gatherer { el.type = 'number'; document.body.append(el); } - await driver.evaluateAsync(makeInput, {args: []}); + await driver.evaluate(makeInput, {args: []}); await new Promise(resolve => setTimeout(resolve, 100)); // Prove that `driver` (Lighthouse) and `page` (Puppeteer) are talking to the same page. From 606db62db2719074fec77fcb5c6576bea944c871 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 21 Dec 2020 18:00:28 -0600 Subject: [PATCH 23/37] oops --- .../gatherers/html-without-javascript-test.js | 68 ------------------- 1 file changed, 68 deletions(-) delete mode 100644 lighthouse-core/test/gather/gatherers/html-without-javascript-test.js diff --git a/lighthouse-core/test/gather/gatherers/html-without-javascript-test.js b/lighthouse-core/test/gather/gatherers/html-without-javascript-test.js deleted file mode 100644 index 7a29fc71a2b1..000000000000 --- a/lighthouse-core/test/gather/gatherers/html-without-javascript-test.js +++ /dev/null @@ -1,68 +0,0 @@ -/** - * @license Copyright 2016 The Lighthouse Authors. All Rights Reserved. - * Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0 - * Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License. - */ -'use strict'; - -/* eslint-env jest */ - -const HTMLWithoutJavaScriptGather = require('../../../gather/gatherers/html-without-javascript.js'); -const assert = require('assert').strict; -let htmlWithoutJavaScriptGather; - -describe('HTML without JavaScript gatherer', () => { - // Reset the Gatherer before each test. - beforeEach(() => { - htmlWithoutJavaScriptGather = new HTMLWithoutJavaScriptGather(); - }); - - it('updates the options', () => { - const opts = {disableJavaScript: false}; - htmlWithoutJavaScriptGather.beforePass(opts); - - return assert.equal(opts.disableJavaScript, true); - }); - - it('resets the options', () => { - const opts = { - disableJavaScript: true, - driver: { - async evaluate() { - return {bodyText: 'Hello!'}; - }, - }, - }; - return htmlWithoutJavaScriptGather - .afterPass(opts) - .then(_ => { - assert.equal(opts.disableJavaScript, false); - }); - }); - - it('returns an artifact', () => { - const bodyText = 'Hello!'; - const hasNoScript = true; - return htmlWithoutJavaScriptGather.afterPass({ - driver: { - async evaluate() { - return {bodyText, hasNoScript}; - }, - }, - }).then(artifact => { - assert.deepStrictEqual(artifact, {bodyText, hasNoScript}); - }); - }); - - it('throws an error when driver returns a non-string', () => { - return htmlWithoutJavaScriptGather.afterPass({ - driver: { - async evaluate() { - return null; - }, - }, - }).then( - _ => assert.ok(false), - _ => assert.ok(true)); - }); -}); From 8c98d055d80977e8a0b041a1c269789221e25864 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 21 Dec 2020 18:02:14 -0600 Subject: [PATCH 24/37] pr --- lighthouse-core/gather/gatherers/image-elements.js | 1 - 1 file changed, 1 deletion(-) diff --git a/lighthouse-core/gather/gatherers/image-elements.js b/lighthouse-core/gather/gatherers/image-elements.js index 622e1c2eef1c..afb966743ebf 100644 --- a/lighthouse-core/gather/gatherers/image-elements.js +++ b/lighthouse-core/gather/gatherers/image-elements.js @@ -297,7 +297,6 @@ class ImageElements extends Gatherer { getPosition, getHTMLImages, getCSSImages, - collectImageElementInfo, ], }); From 3195324f423c9dc17c467c5c1c7de7e2ed878553 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Mon, 21 Dec 2020 19:17:40 -0600 Subject: [PATCH 25/37] update --- .../audits/seo/crawlable-anchors.js | 8 +------ lighthouse-core/audits/seo/hreflang.js | 21 ++++++++++++------- lighthouse-core/audits/seo/is-crawlable.js | 2 +- lighthouse-core/audits/seo/plugins.js | 2 +- lighthouse-core/audits/seo/tap-targets.js | 7 +------ .../gather/gatherers/meta-elements.js | 13 +++++++++--- .../gather/gatherers/seo/embedded-content.js | 17 +++++++++++---- lighthouse-core/lib/page-functions.js | 1 + .../test/audits/seo/is-crawlable-test.js | 3 ++- .../test/audits/seo/plugins-test.js | 15 +++++++++++++ types/artifacts.d.ts | 5 +++-- 11 files changed, 62 insertions(+), 32 deletions(-) diff --git a/lighthouse-core/audits/seo/crawlable-anchors.js b/lighthouse-core/audits/seo/crawlable-anchors.js index 9a721a18e4fd..70db09817383 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.makeNodeValue(anchor.node), }; }); diff --git a/lighthouse-core/audits/seo/hreflang.js b/lighthouse-core/audits/seo/hreflang.js index 3d49a3f0e280..e7362f33b27c 100644 --- a/lighthouse-core/audits/seo/hreflang.js +++ b/lighthouse-core/audits/seo/hreflang.js @@ -113,13 +113,20 @@ class Hreflang extends Audit { } if (link.source === 'head') { - source = { - type: 'node', - snippet: ``, - path: link.node !== null ? link.node.devtoolsNodePath : '', - selector: link.node !== null ? link.node.selector : '', - nodeLabel: link.node !== null ? link.node.nodeLabel : '', - }; + if (link.node) { + source = { + ...Audit.makeNodeValue(link.node), + snippet: ``, + }; + } else { + source = { + type: 'node', + snippet: ``, + // path: '', + // selector: '', + // nodeLabel: '', + }; + } } else if (link.source === 'headers') { source = `Link: <${link.hrefRaw}>; rel="alternate"; hreflang="${link.hreflang}"`; } diff --git a/lighthouse-core/audits/seo/is-crawlable.js b/lighthouse-core/audits/seo/is-crawlable.js index c9c2082ac692..f577f625f3fe 100644 --- a/lighthouse-core/audits/seo/is-crawlable.js +++ b/lighthouse-core/audits/seo/is-crawlable.js @@ -105,7 +105,7 @@ class IsCrawlable extends Audit { if (isBlocking) { blockingDirectives.push({ source: { - type: /** @type {'node'} */ ('node'), + ...Audit.makeNodeValue(metaRobots.node), snippet: ``, }, }); diff --git a/lighthouse-core/audits/seo/plugins.js b/lighthouse-core/audits/seo/plugins.js index 5e1c275fe3ea..15593f2fcc1e 100644 --- a/lighthouse-core/audits/seo/plugins.js +++ b/lighthouse-core/audits/seo/plugins.js @@ -146,7 +146,7 @@ class Plugins extends Audit { return { source: { - type: /** @type {'node'} */ ('node'), + ...Audit.makeNodeValue(plugin.node), snippet: `<${tagName}${attributes}>${params}`, }, }; diff --git a/lighthouse-core/audits/seo/tap-targets.js b/lighthouse-core/audits/seo/tap-targets.js index 3125dde3deac..54ab476b9ed4 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.makeNodeValue(target.node), boundingRect, - nodeLabel: target.node.nodeLabel, }; } diff --git a/lighthouse-core/gather/gatherers/meta-elements.js b/lighthouse-core/gather/gatherers/meta-elements.js index c2a38b86e1c0..3e135b6fe907 100644 --- a/lighthouse-core/gather/gatherers/meta-elements.js +++ b/lighthouse-core/gather/gatherers/meta-elements.js @@ -8,12 +8,18 @@ const Gatherer = require('./gatherer.js'); const pageFunctions = require('../../lib/page-functions.js'); -/* globals getElementsInDocument */ +/* globals getElementsInDocument getNodeDetails */ /* istanbul ignore next */ function collectMetaElements() { - // @ts-expect-error - getElementsInDocument put into scope via stringification - const metas = /** @type {HTMLMetaElement[]} */ (getElementsInDocument('head meta')); + 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 metas = /** @type {HTMLMetaElement[]} */ (functions.getElementsInDocument('head meta')); return metas.map(meta => { /** @param {string} name */ const getAttribute = name => { @@ -27,6 +33,7 @@ function collectMetaElements() { property: getAttribute('property'), httpEquiv: meta.httpEquiv ? meta.httpEquiv.toLowerCase() : undefined, charset: getAttribute('charset'), + node: functions.getNodeDetails(meta), }; }); } diff --git a/lighthouse-core/gather/gatherers/seo/embedded-content.js b/lighthouse-core/gather/gatherers/seo/embedded-content.js index a4554d40d672..4af150c9fd45 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), })); } diff --git a/lighthouse-core/lib/page-functions.js b/lighthouse-core/lib/page-functions.js index 7c043411c837..2af7927e003d 100644 --- a/lighthouse-core/lib/page-functions.js +++ b/lighthouse-core/lib/page-functions.js @@ -523,6 +523,7 @@ module.exports = { computeBenchmarkIndex: computeBenchmarkIndex, computeBenchmarkIndexString: computeBenchmarkIndex.toString(), getNodeDetailsString, + getNodeDetails: getNodeDetailsImpl, getNodePathString: getNodePath.toString(), getNodeSelectorString: getNodeSelector.toString(), getNodePath, diff --git a/lighthouse-core/test/audits/seo/is-crawlable-test.js b/lighthouse-core/test/audits/seo/is-crawlable-test.js index dbb5efc1950b..c06b986b7fe3 100644 --- a/lighthouse-core/test/audits/seo/is-crawlable-test.js +++ b/lighthouse-core/test/audits/seo/is-crawlable-test.js @@ -12,7 +12,7 @@ const networkRecordsToDevtoolsLog = require('../../network-records-to-devtools-l /* eslint-env jest */ describe('SEO: Is page crawlable audit', () => { - const makeMetaElements = content => [{name: 'robots', content}]; + const makeMetaElements = content => [{name: 'robots', content, node: {}}]; it('fails when page is blocked from indexing with a robots metatag', () => { const robotsValues = [ @@ -323,6 +323,7 @@ describe('SEO: Is page crawlable audit', () => { Array [ Object { "source": Object { + "path": undefined, "snippet": "", "type": "node", }, diff --git a/lighthouse-core/test/audits/seo/plugins-test.js b/lighthouse-core/test/audits/seo/plugins-test.js index f54a74170a8a..23f81dda1dae 100644 --- a/lighthouse-core/test/audits/seo/plugins-test.js +++ b/lighthouse-core/test/audits/seo/plugins-test.js @@ -16,31 +16,37 @@ describe('SEO: Avoids plugins', () => { [{ tagName: 'APPLET', params: [], + node: {}, }], [{ tagName: 'OBJECT', type: 'application/x-shockwave-flash', params: [], + node: {}, }], [{ tagName: 'EMBED', type: 'application/x-java-applet;jpi-version=1.4', params: [], + node: {}, }], [{ tagName: 'OBJECT', type: 'application/x-silverlight-2', params: [], + node: {}, }], [{ tagName: 'OBJECT', data: 'https://example.com/movie_name.swf?uid=123', params: [], + node: {}, }], [{ tagName: 'EMBED', src: '/path/to/movie_name.latest.swf', params: [], + node: {}, }], [{ tagName: 'OBJECT', @@ -48,12 +54,14 @@ describe('SEO: Avoids plugins', () => { {name: 'quality', value: 'low'}, {name: 'movie', value: 'movie.swf?id=123'}, ], + node: {}, }], [{ tagName: 'OBJECT', params: [ {name: 'code', value: '../HelloWorld.class'}, ], + node: {}, }], ]; @@ -75,15 +83,18 @@ describe('SEO: Avoids plugins', () => { tagName: 'EMBED', type: 'application/x-java-applet;jpi-version=1.4', params: [], + node: {}, }, { tagName: 'OBJECT', type: 'application/x-silverlight-2', params: [], + node: {}, }, { tagName: 'APPLET', params: [], + node: {}, }, ], }; @@ -110,17 +121,20 @@ describe('SEO: Avoids plugins', () => { type: 'image/svg+xml', data: 'https://example.com/test.svg', params: [], + node: {}, }, { tagName: 'OBJECT', data: 'https://example.com', params: [], + node: {}, }, { tagName: 'EMBED', type: 'video/quicktime', src: 'movie.mov', params: [], + node: {}, }, { tagName: 'OBJECT', @@ -131,6 +145,7 @@ describe('SEO: Avoids plugins', () => { name: 'movie', value: 'http://www.youtube.com/v/example', }], + node: {}, }, ], }; diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts index 6350a3e6281d..97fdb5ce7a14 100644 --- a/types/artifacts.d.ts +++ b/types/artifacts.d.ts @@ -73,7 +73,7 @@ declare global { /** All the link elements on the page or equivalently declared in `Link` headers. @see https://html.spec.whatwg.org/multipage/links.html */ LinkElements: Artifacts.LinkElement[]; /** The values of the elements in the head. */ - MetaElements: Array<{name?: string, content?: string, property?: string, httpEquiv?: string, charset?: string}>; + MetaElements: Array<{name?: string, content?: string, property?: string, httpEquiv?: string, charset?: string, node: LH.Artifacts.NodeDetails}>; /** Information on all script elements in the page. Also contains the content of all requested scripts and the networkRecord requestId that contained their content. Note, HTML documents will have one entry per script tag, all with the same requestId. */ ScriptElements: Array; /** The dimensions and devicePixelRatio of the loaded viewport. */ @@ -205,7 +205,8 @@ declare global { src: string | null; data: string | null; code: string | null; - params: {name: string; value: string}[]; + params: Array<{name: string; value: string}>; + node: LH.Artifacts.NodeDetails; } export interface IFrameElement { From f7995834359b4a44c02fc17aa29913b03d0435c7 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 22 Dec 2020 14:34:32 -0600 Subject: [PATCH 26/37] update --- .../audits/non-composited-animations.js | 12 +--- lighthouse-core/test/results/sample_v2.json | 62 ++++++++++++++----- 2 files changed, 49 insertions(+), 25 deletions(-) diff --git a/lighthouse-core/audits/non-composited-animations.js b/lighthouse-core/audits/non-composited-animations.js index 3f6904b4d4a1..4b8865816d5f 100644 --- a/lighthouse-core/audits/non-composited-animations.js +++ b/lighthouse-core/audits/non-composited-animations.js @@ -134,16 +134,6 @@ class NonCompositedAnimations extends Audit { let shouldAddAnimationNameColumn = false; artifacts.TraceElements.forEach(element => { if (element.traceEventType !== 'animation') return; - /** @type {LH.Audit.Details.NodeValue} */ - const node = { - type: '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, - }; const animations = element.animations || []; const animationReasons = new Map(); @@ -173,7 +163,7 @@ class NonCompositedAnimations extends Audit { } results.push({ - node, + node: Audit.makeNodeValue(element.node), subItems: { type: 'subitems', items: allFailureReasons, diff --git a/lighthouse-core/test/results/sample_v2.json b/lighthouse-core/test/results/sample_v2.json index 0c6ebb642e3d..9bcc9b2ed99b 100644 --- a/lighthouse-core/test/results/sample_v2.json +++ b/lighthouse-core/test/results/sample_v2.json @@ -2033,10 +2033,10 @@ { "node": { "type": "node", - "path": "1,HTML,1,BODY,0,DIV", "selector": "body > div#animated-boi", "nodeLabel": "div", - "snippet": "
" + "snippet": "
", + "path": "1,HTML,1,BODY,0,DIV" }, "subItems": { "type": "subitems", @@ -2088,20 +2088,38 @@ "url": "blob:http://localhost:54657/822c70a0-b912-41c7-9a21-56c3d309e75b", "node": { "type": "node", - "path": "3,HTML,1,BODY,63,IMG", + "lhId": "page-7-IMG", "selector": "body > img", + "boundingRect": { + "top": 1295, + "bottom": 1311, + "left": 322, + "right": 338, + "width": 16, + "height": 16 + }, + "snippet": "", "nodeLabel": "img", - "snippet": "" + "path": "3,HTML,1,BODY,63,IMG" } }, { "url": "filesystem:http://localhost:54657/temporary/empty-0.8423185960902964.png", "node": { "type": "node", - "path": "3,HTML,1,BODY,72,IMG", + "lhId": "page-8-IMG", "selector": "body > img", + "boundingRect": { + "top": 1317, + "bottom": 1333, + "left": 8, + "right": 24, + "width": 16, + "height": 16 + }, + "snippet": "", "nodeLabel": "img", - "snippet": "" + "path": "3,HTML,1,BODY,72,IMG" } } ] @@ -4603,19 +4621,35 @@ { "node": { "type": "node", - "path": "3,HTML,1,BODY,38,A", "selector": "body > a", + "boundingRect": { + "top": 1166, + "bottom": 1184, + "left": 175, + "right": 255, + "width": 80, + "height": 18 + }, + "snippet": "", "nodeLabel": "external link", - "snippet": "" + "path": "3,HTML,1,BODY,38,A" } }, { "node": { "type": "node", - "path": "3,HTML,1,BODY,54,A", "selector": "body > a", + "boundingRect": { + "top": 1275, + "bottom": 1293, + "left": 192, + "right": 192, + "width": 0, + "height": 18 + }, + "snippet": "", "nodeLabel": "a", - "snippet": "" + "path": "3,HTML,1,BODY,54,A" } } ] @@ -4671,8 +4705,8 @@ "tapTarget": { "type": "node", "snippet": "", - "path": "3,HTML,1,BODY,17,BUTTON", "selector": "body > button.small-button", + "nodeLabel": "Do something", "boundingRect": { "left": 8, "right": 208, @@ -4681,13 +4715,13 @@ "width": 200, "height": 18 }, - "nodeLabel": "Do something" + "path": "3,HTML,1,BODY,17,BUTTON" }, "overlappingTarget": { "type": "node", "snippet": "", - "path": "3,HTML,1,BODY,18,BUTTON", "selector": "body > button.small-button", + "nodeLabel": "Do something else", "boundingRect": { "left": 8, "right": 208, @@ -4696,7 +4730,7 @@ "width": 200, "height": 18 }, - "nodeLabel": "Do something else" + "path": "3,HTML,1,BODY,18,BUTTON" }, "tapTargetScore": 864, "overlappingTargetScore": 720, From d850391e760da3ac4c0c2cd329f58357bf50d077 Mon Sep 17 00:00:00 2001 From: Connor Clark Date: Tue, 22 Dec 2020 16:43:22 -0600 Subject: [PATCH 27/37] more --- .../audits/accessibility/axe-audit.js | 8 +- 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 +- lighthouse-core/audits/seo/hreflang.js | 3 - .../test/audits/autocomplete-test.js | 8 +- lighthouse-core/test/results/sample_v2.json | 274 +++++++++++------- types/artifacts.d.ts | 4 +- 11 files changed, 187 insertions(+), 167 deletions(-) diff --git a/lighthouse-core/audits/accessibility/axe-audit.js b/lighthouse-core/audits/accessibility/axe-audit.js index e8d26570340f..6fedf2b3eb5d 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.makeNodeValue(axeNode.node), explanation: axeNode.failureSummary, - nodeLabel: axeNode.node.nodeLabel, }, })); } diff --git a/lighthouse-core/audits/autocomplete.js b/lighthouse-core/audits/autocomplete.js index f1227df26da5..0b221087e008 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.makeNodeValue(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..9bd34eda91f0 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.makeNodeValue(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.makeNodeValue(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..ab09664248dc 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.makeNodeValue(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..784bde4a011d 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.makeNodeValue(input.node), }); }); diff --git a/lighthouse-core/audits/largest-contentful-paint-element.js b/lighthouse-core/audits/largest-contentful-paint-element.js index 9f644617d6fc..91d0224e3dc3 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.makeNodeValue(lcpElement.node), }); } diff --git a/lighthouse-core/audits/layout-shift-elements.js b/lighthouse-core/audits/layout-shift-elements.js index 1ca937bb779c..ecc96af8c2b2 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.makeNodeValue(element.node), score: element.score, }; }); diff --git a/lighthouse-core/audits/seo/hreflang.js b/lighthouse-core/audits/seo/hreflang.js index e7362f33b27c..5511c7410b5d 100644 --- a/lighthouse-core/audits/seo/hreflang.js +++ b/lighthouse-core/audits/seo/hreflang.js @@ -122,9 +122,6 @@ class Hreflang extends Audit { source = { type: 'node', snippet: ``, - // path: '', - // selector: '', - // nodeLabel: '', }; } } else if (link.source === 'headers') { diff --git a/lighthouse-core/test/audits/autocomplete-test.js b/lighthouse-core/test/audits/autocomplete-test.js index 76ebba4156d2..2405ac830c14 100644 --- a/lighthouse-core/test/audits/autocomplete-test.js +++ b/lighthouse-core/test/audits/autocomplete-test.js @@ -114,18 +114,18 @@ describe('Best Practices: autocomplete audit', () => { { 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,20 +310,20 @@ describe('Best Practices: autocomplete audit', () => { { current: 'shipping section-red cc-name', node: { + type: 'node', nodeLabel: 'textarea', // eslint-disable-next-line max-len snippet: '