Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

core(scripts): move to ScriptElements artifact #7920

Merged
merged 5 commits into from
Apr 4, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions lighthouse-cli/test/cli/__snapshots__/index-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -1069,9 +1069,6 @@ Object {
"blockedUrlPatterns": Array [],
"cpuQuietThresholdMs": 1000,
"gatherers": Array [
Object {
"path": "scripts",
},
Object {
"path": "css-usage",
},
Expand Down Expand Up @@ -1099,6 +1096,9 @@ Object {
Object {
"path": "meta-elements",
},
Object {
"path": "script-elements",
},
Object {
"path": "dobetterweb/appcache",
},
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ class UnminifiedJavaScript extends ByteEfficiencyAudit {
title: str_(UIStrings.title),
description: str_(UIStrings.description),
scoreDisplayMode: ByteEfficiencyAudit.SCORING_MODES.NUMERIC,
requiredArtifacts: ['Scripts', 'devtoolsLogs', 'traces'],
requiredArtifacts: ['ScriptElements', 'devtoolsLogs', 'traces'],
};
}

Expand Down Expand Up @@ -78,10 +78,11 @@ class UnminifiedJavaScript extends ByteEfficiencyAudit {
/** @type {Array<LH.Audit.ByteEfficiencyItem>} */
const items = [];
const warnings = [];
for (const {requestId, inline, content} of artifacts.Scripts) {
for (const {requestId, src, content} of artifacts.ScriptElements) {
if (!content) continue;

const networkRecord = networkRecords.find(record => record.requestId === requestId);
const displayUrl = inline || !networkRecord ?
const displayUrl = !src || !networkRecord ?
`inline: ${content.substr(0, 40)}...` :
networkRecord.url;
try {
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/config/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -98,7 +98,6 @@ const defaultConfig = {
networkQuietThresholdMs: 1000,
cpuQuietThresholdMs: 1000,
gatherers: [
'scripts',
'css-usage',
'viewport-dimensions',
'runtime-exceptions',
Expand All @@ -108,6 +107,7 @@ const defaultConfig = {
'image-elements',
'link-elements',
'meta-elements',
'script-elements',
'dobetterweb/appcache',
'dobetterweb/doctype',
'dobetterweb/domstats',
Expand Down
91 changes: 91 additions & 0 deletions lighthouse-core/gather/gatherers/script-elements.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,91 @@
/**
* @license Copyright 2017 Google Inc. 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';

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

/**
* @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

return scripts.map(script => {
return {
type: script.type || null,
src: script.src || null,
async: script.async,
defer: script.defer,
source: /** @type {'head'|'body'} */ (script.closest('head') ? 'head' : 'body'),
content: script.src ? null : script.text,
requestId: null,
};
});
}

/**
* @fileoverview Gets JavaScript file contents.
*/
class ScriptElements extends Gatherer {
/**
* @param {LH.Gatherer.PassContext} passContext
* @param {LH.Gatherer.LoadData} loadData
* @return {Promise<LH.Artifacts['ScriptElements']>}
*/
async afterPass(passContext, loadData) {
const driver = passContext.driver;
const mainResource = NetworkAnalyzer.findMainDocument(loadData.networkRecords, passContext.url);

/** @type {LH.Artifacts['ScriptElements']} */
const scripts = await driver.evaluateAsync(`(() => {
${getElementsInDocumentString}
return (${collectAllScriptElements.toString()})();
})()`, {useIsolation: true});

for (const script of scripts) {
if (script.content) script.requestId = mainResource.requestId;
}

const scriptRecords = loadData.networkRecords
// Ignore records from OOPIFs
.filter(record => !record.sessionId)
// Only get the content of script requests
.filter(record => record.resourceType === NetworkRequest.TYPES.Script);

for (const record of scriptRecords) {
try {
const content = await driver.getRequestContent(record.requestId);
if (!content) continue;

const matchedScriptElement = scripts.find(script => script.src === record.url);
if (matchedScriptElement) {
matchedScriptElement.requestId = record.requestId;
matchedScriptElement.content = content;
} else {
scripts.push({
type: null,
src: record.url,
async: false,
defer: false,
source: 'network',
requestId: record.requestId,
content,
});
}
} catch (e) {}
}

return scripts;
}
}

module.exports = ScriptElements;
83 changes: 0 additions & 83 deletions lighthouse-core/gather/gatherers/scripts.js

This file was deleted.

Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,10 @@ const resourceType = 'Script';
describe('Page uses optimized responses', () => {
it('fails when given unminified scripts', () => {
const auditResult = UnminifiedJavascriptAudit.audit_({
Scripts: [
ScriptElements: [
{
requestId: '123.1',
src: 'foo.js',
content: `
var foo = new Set();
foo.add(1);
Expand All @@ -31,6 +32,7 @@ describe('Page uses optimized responses', () => {
},
{
requestId: '123.2',
src: 'other.js',
content: `
const foo = new Set();
foo.add(1);
Expand All @@ -43,6 +45,7 @@ describe('Page uses optimized responses', () => {
},
{
requestId: '123.3',
src: 'valid-ish.js',
content: /* eslint-disable no-useless-escape */
`
const foo = 1
Expand All @@ -51,6 +54,7 @@ describe('Page uses optimized responses', () => {
},
{
requestId: '123.4',
src: 'invalid.js',
content: '#$*%dense',
},
],
Expand All @@ -75,7 +79,7 @@ describe('Page uses optimized responses', () => {

it('fails when given unminified scripts even with missing network record', () => {
const auditResult = UnminifiedJavascriptAudit.audit_({
Scripts: [
ScriptElements: [
{
requestId: '123.1',
content: `
Expand Down Expand Up @@ -106,13 +110,15 @@ describe('Page uses optimized responses', () => {

it('passes when scripts are already minified', () => {
const auditResult = UnminifiedJavascriptAudit.audit_({
Scripts: [
ScriptElements: [
{
requestId: '123.1',
src: 'foo.js',
content: 'var f=new Set();f.add(1);f.add(2);if(f.has(2))console.log(1234)',
},
{
requestId: '123.2',
src: 'other.js',
content: `
const foo = new Set();
foo.add(1);
Expand All @@ -125,6 +131,7 @@ describe('Page uses optimized responses', () => {
},
{
requestId: '123.3',
src: 'invalid.js',
content: 'for{(wtf',
},
],
Expand Down
7 changes: 4 additions & 3 deletions lighthouse-core/test/config/config-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -1103,8 +1103,8 @@ describe('Config', () => {
passes: [{
passName: 'defaultPass',
gatherers: [
// `options` merged into default `scripts` gatherer.
{path: 'scripts', options: {gOpt}},
// `options` merged into default `script-elements` gatherer.
{path: 'script-elements', options: {gOpt}},
],
}],
audits: [
Expand All @@ -1117,7 +1117,8 @@ describe('Config', () => {
const printedConfig = JSON.parse(printed);

// Check that options weren't completely eliminated.
const scriptsGatherer = printedConfig.passes[0].gatherers.find(g => g.path === 'scripts');
const scriptsGatherer = printedConfig.passes[0].gatherers
.find(g => g.path === 'script-elements');
assert.strictEqual(scriptsGatherer.options.gOpt, gOpt);
const metricsAudit = printedConfig.audits.find(a => a.path === 'metrics');
assert.strictEqual(metricsAudit.options.aOpt, aOpt);
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/index-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ describe('Module Tests', function() {
return lighthouse('chrome://version', {}, {
passes: [{
gatherers: [
'scripts',
'script-elements',
],
}],
audits: [
Expand Down
Loading