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

misc(locales): ignore all locales from devtools & extension build #6170

Merged
merged 6 commits into from
Oct 4, 2018
Merged
Show file tree
Hide file tree
Changes from 2 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
61 changes: 0 additions & 61 deletions lighthouse-extension/app/src/lighthouse-ext-background.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,14 +10,6 @@ const background = require('./lighthouse-background');

const ExtensionProtocol = require('../../../lighthouse-core/gather/connections/extension');
const log = require('lighthouse-logger');
const assetSaver = require('../../../lighthouse-core/lib/asset-saver.js');
const LHError = require('../../../lighthouse-core/lib/lh-error.js');

/** @type {Record<'mobile'|'desktop', LH.Config.Json>} */
const LR_PRESETS = {
mobile: require('../../../lighthouse-core/config/lr-mobile-config'),
desktop: require('../../../lighthouse-core/config/lr-desktop-config'),
};

/** @typedef {import('../../../lighthouse-core/gather/connections/connection.js')} Connection */

Expand Down Expand Up @@ -87,56 +79,6 @@ async function runLighthouseInExtension(flags, categoryIDs) {
await new Promise(resolve => chrome.windows.create({url: blobURL}, resolve));
}

/**
* Run lighthouse for connection and provide similar results as in CLI.
* @param {Connection} connection
* @param {string} url
* @param {LH.Flags} flags Lighthouse flags, including `output`
* @param {{lrDevice?: 'desktop'|'mobile', categoryIDs?: Array<string>, logAssets: boolean}} lrOpts Options coming from Lightrider
* @return {Promise<string|Array<string>|void>}
*/
async function runLighthouseInLR(connection, url, flags, {lrDevice, categoryIDs, logAssets}) {
// Certain fixes need to kick-in under LR, see https://github.com/GoogleChrome/lighthouse/issues/5839
global.isLightRider = true;

// disableStorageReset because it causes render server hang
flags.disableStorageReset = true;
flags.logLevel = flags.logLevel || 'info';
const config = lrDevice === 'desktop' ? LR_PRESETS.desktop : LR_PRESETS.mobile;
if (categoryIDs) {
config.settings = config.settings || {};
config.settings.onlyCategories = categoryIDs;
}

try {
const results = await lighthouse(url, flags, config, connection);
if (!results) return;

if (logAssets) {
await assetSaver.logAssets(results.artifacts, results.lhr.audits);
}
return results.report;
} catch (err) {
// If an error ruined the entire lighthouse run, attempt to return a meaningful error.
let runtimeError;
if (!(err instanceof LHError) || !err.lhrRuntimeError) {
runtimeError = {
code: LHError.UNKNOWN_ERROR,
message: `Unknown error encountered with message '${err.message}'`,
};
} else {
runtimeError = {
code: err.code,
message: err.friendlyMessage ?
`${err.friendlyMessage} (${err.message})` :
err.message,
};
}

return JSON.stringify({runtimeError}, null, 2);
}
}

/**
* @param {string} reportHtml
* @return {string} Blob URL of the report (or error page) HTML
Expand Down Expand Up @@ -241,7 +183,6 @@ if (typeof module !== 'undefined' && module.exports) {
// Export for importing types into popup.js, require()ing into unit tests.
module.exports = {
runLighthouseInExtension,
runLighthouseInLR,
getDefaultCategories: background.getDefaultCategories,
isRunning,
listenForStatus,
Expand All @@ -255,8 +196,6 @@ if (typeof window !== 'undefined') {
// @ts-ignore
window.runLighthouseInExtension = runLighthouseInExtension;
// @ts-ignore
window.runLighthouseInLR = runLighthouseInLR;
// @ts-ignore
window.getDefaultCategories = background.getDefaultCategories;
// @ts-ignore
window.isRunning = isRunning;
Expand Down
95 changes: 95 additions & 0 deletions lighthouse-extension/app/src/lighthouse-lr-background.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,95 @@
/**
* @license Copyright 2018 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 lighthouse = require('../../../lighthouse-core/index');
const background = require('./lighthouse-background');

const log = require('lighthouse-logger');
const assetSaver = require('../../../lighthouse-core/lib/asset-saver.js');
const LHError = require('../../../lighthouse-core/lib/lh-error.js');

/** @type {Record<'mobile'|'desktop', LH.Config.Json>} */
const LR_PRESETS = {
mobile: require('../../../lighthouse-core/config/lr-mobile-config'),
desktop: require('../../../lighthouse-core/config/lr-desktop-config'),
};

/** @typedef {import('../../../lighthouse-core/gather/connections/connection.js')} Connection */

/**
* Run lighthouse for connection and provide similar results as in CLI.
* @param {Connection} connection
* @param {string} url
* @param {LH.Flags} flags Lighthouse flags, including `output`
* @param {{lrDevice?: 'desktop'|'mobile', categoryIDs?: Array<string>, logAssets: boolean}} lrOpts Options coming from Lightrider
* @return {Promise<string|Array<string>|void>}
*/
async function runLighthouseInLR(connection, url, flags, {lrDevice, categoryIDs, logAssets}) {
// Certain fixes need to kick-in under LR, see https://github.com/GoogleChrome/lighthouse/issues/5839
global.isLightRider = true;

// disableStorageReset because it causes render server hang
flags.disableStorageReset = true;
flags.logLevel = flags.logLevel || 'info';
const config = lrDevice === 'desktop' ? LR_PRESETS.desktop : LR_PRESETS.mobile;
if (categoryIDs) {
config.settings = config.settings || {};
config.settings.onlyCategories = categoryIDs;
}

try {
const results = await lighthouse(url, flags, config, connection);
if (!results) return;

if (logAssets) {
await assetSaver.logAssets(results.artifacts, results.lhr.audits);
}
return results.report;
} catch (err) {
// If an error ruined the entire lighthouse run, attempt to return a meaningful error.
let runtimeError;
if (!(err instanceof LHError) || !err.lhrRuntimeError) {
runtimeError = {
code: LHError.UNKNOWN_ERROR,
message: `Unknown error encountered with message '${err.message}'`,
};
} else {
runtimeError = {
code: err.code,
message: err.friendlyMessage ?
`${err.friendlyMessage} (${err.message})` :
err.message,
};
}

return JSON.stringify({runtimeError}, null, 2);
}
}

/** @param {(status: [string, string, string]) => void} listenCallback */
function listenForStatus(listenCallback) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

you can drop this

log.events.addListener('status', listenCallback);
}

if (typeof module !== 'undefined' && module.exports) {
// Export for importing types into popup.js, require()ing into unit tests.
module.exports = {
getDefaultCategories: background.getDefaultCategories,
runLighthouseInLR,
listenForStatus,
};
}

// Expose on window for extension, other browser-residing consumers of file.
if (typeof window !== 'undefined') {
// @ts-ignore
window.runLighthouseInLR = runLighthouseInLR;
// @ts-ignore
window.getDefaultCategories = background.getDefaultCategories;
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do I need to expose this one?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nah you can remove it and the listen fn

// @ts-ignore
window.listenForStatus = listenForStatus;
}
32 changes: 23 additions & 9 deletions lighthouse-extension/gulpfile.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,13 @@ const pkg = require('../package.json');

const distDir = 'dist';

// list of all consumers we build for (easier to understand which file is used for which)
const CONSUMERS = {
DEVTOOLS: 'lighthouse-background.js',
EXTENSION: 'lighthouse-ext-background.js',
LIGHTRIDER: 'lighthouse-lr-background.js',
};

const VERSION = pkg.version;
const COMMIT_HASH = require('child_process')
.execSync('git rev-parse HEAD')
Expand All @@ -50,6 +57,13 @@ const gatherers = LighthouseRunner.getGathererList()
const computedArtifacts = LighthouseRunner.getComputedGathererList()
.map(f => '../lighthouse-core/gather/computed/' + f.replace(/\.js$/, ''));

const locales = fs.readdirSync('../lighthouse-core/lib/i18n/locales/')
.map(f => require.resolve(`../lighthouse-core/lib/i18n/locales/${f}`));

const isDevtools = (file) => new RegExp(`${CONSUMERS.DEVTOOLS}$`).test(file);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

endsWith?

const isExtension = (file) => new RegExp(`${CONSUMERS.EXTENSION}$`).test(file);
// const isLightrider = (file) => new RegExp(`${CONSUMERS.LIGHTRIDER}$`).test(file);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

guess you can remove now


gulp.task('extras', () => {
return gulp.src([
'app/*.*',
Expand Down Expand Up @@ -96,7 +110,7 @@ gulp.task('chromeManifest', () => {
const manifestOpts = {
buildnumber: false,
background: {
target: 'scripts/lighthouse-ext-background.js',
target: `scripts/${CONSUMERS.EXTENSION}`,
},
};
return gulp.src('app/manifest.json')
Expand All @@ -114,10 +128,7 @@ function applyBrowserifyTransforms(bundle) {
}

gulp.task('browserify-lighthouse', () => {
return gulp.src([
'app/src/lighthouse-background.js',
'app/src/lighthouse-ext-background.js',
], {read: false})
return gulp.src(Object.values(CONSUMERS).map(consumer => `app/src/${consumer}`), {read: false})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lots going on here. extract the obj.values().map() to a const?

.pipe(tap(file => {
let bundle = browserify(file.path); // , {debug: true}); // for sourcemaps
bundle = applyBrowserifyTransforms(bundle);
Expand All @@ -135,10 +146,15 @@ gulp.task('browserify-lighthouse', () => {
bundle.ignore(require.resolve('../lighthouse-core/gather/connections/cri.js'));

// Prevent the DevTools background script from getting the stringified HTML.
if (/lighthouse-background/.test(file.path)) {
if (isDevtools(file.path)) {
wardpeet marked this conversation as resolved.
Show resolved Hide resolved
bundle.ignore(require.resolve('../lighthouse-core/report/html/html-report-assets.js'));
}

if (isDevtools(file.path) || isExtension(file.path)) {
// eslint-disable-next-line
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can we specify which rule to disable?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

sorry I don't need it anymore it was for testing console.log

bundle.ignore(locales);
}

// Expose the audits, gatherers, and computed artifacts so they can be dynamically loaded.
const corePath = '../lighthouse-core/';
const driverPath = `${corePath}gather/`;
Expand Down Expand Up @@ -197,9 +213,7 @@ gulp.task('compilejs', () => {
// sourceMaps: 'both'
};

return gulp.src([
'dist/scripts/lighthouse-background.js',
'dist/scripts/lighthouse-ext-background.js'])
return gulp.src(Object.values(CONSUMERS).map(consumer => `dist/scripts/${consumer}`))
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

extract to a const

.pipe(tap(file => {
const minified = babel.transform(file.contents.toString(), opts).code;
file.contents = new Buffer(minified);
Expand Down