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(fr): add support for plugins #13028

Merged
merged 2 commits into from
Sep 13, 2021
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
2 changes: 2 additions & 0 deletions .github/workflows/smoke.yml
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,7 @@ jobs:

- run: yarn install --frozen-lockfile --network-timeout 1000000
- run: yarn build-report
- run: yarn reset-link

- run: sudo apt-get install xvfb
- name: Run smoke tests
Expand Down Expand Up @@ -117,6 +118,7 @@ jobs:

- run: yarn install --frozen-lockfile --network-timeout 1000000
- run: yarn build-report
- run: yarn reset-link

- run: sudo apt-get install xvfb
- name: yarn smoke --fraggle-rock
Expand Down
4 changes: 4 additions & 0 deletions lighthouse-cli/test/smokehouse/test-definitions/core-tests.js
Original file line number Diff line number Diff line change
Expand Up @@ -241,6 +241,10 @@ const smokeTests = [{
id: 'screenshot',
expectations: require('./screenshot/expectations.js'),
config: require('./screenshot/screenshot-config.js'),
}, {
id: 'pubads',
expectations: require('./pubads/expectations.js'),
config: require('./pubads/pubads-config.js'),
}, {
id: 'csp-allow-all',
expectations: csp.allowAll,
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
/**
* @license Copyright 2020 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';

/**
* @type {Smokehouse.ExpectedRunnerResult}
*/
const expectations = {
lhr: {
requestedUrl: 'http://localhost:10200/online-only.html',
finalUrl: 'http://localhost:10200/online-only.html',
// We should receive warnings about no ads being on the page.
runWarnings: {length: '>0'},
audits: {
// We just want to ensure the plugin had a chance to run without error.
'ad-render-blocking-resources': {scoreDisplayMode: 'notApplicable'},
},
},
};

module.exports = expectations;
Original file line number Diff line number Diff line change
@@ -0,0 +1,12 @@
/**
* @license Copyright 2020 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';

/** @type {LH.Config.Json} */
module.exports = {
extends: 'lighthouse:default',
plugins: ['lighthouse-plugin-publisher-ads'],
};
50 changes: 47 additions & 3 deletions lighthouse-core/config/config-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const path = require('path');
const isDeepEqual = require('lodash.isequal');
const constants = require('./constants.js');
const Budget = require('./budget.js');
const ConfigPlugin = require('./config-plugin.js');
const Runner = require('../runner.js');
const i18n = require('../lib/i18n/i18n.js');
const validation = require('../fraggle-rock/config/validation.js');
Expand All @@ -17,6 +18,21 @@ const validation = require('../fraggle-rock/config/validation.js');
/** @typedef {typeof import('../audits/audit.js')} Audit */
/** @typedef {InstanceType<GathererConstructor>} Gatherer */

function isBundledEnvironment() {
// If we're in DevTools or LightRider, we are definitely bundled.
// TODO: refactor and delete `global.isDevtools`.
if (global.isDevtools || global.isLightrider) return true;

try {
// Not foolproof, but `lighthouse-logger` is a dependency of lighthouse that should always be resolvable.
// `require.resolve` will only throw in atypical/bundled environments.
require.resolve('lighthouse-logger');
Copy link
Collaborator

Choose a reason for hiding this comment

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

i cri in esm evry time

( I don't have an alternative suggestion )

Copy link
Collaborator

@connorjclark connorjclark Sep 9, 2021

Choose a reason for hiding this comment

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

except for: making build-bundle inject a globalThis.isBundled = true...

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

we should be able to use createRequire/import.meta.require/require.resolve combo based on environment, though detection will be annoying :)

return false;
} catch (err) {
return true;
}
}

/**
* If any items with identical `path` properties are found in the input array,
* merge their `options` properties into the first instance and then discard any
Expand Down Expand Up @@ -224,9 +240,8 @@ function requireAudit(auditPath, coreAuditList, configDir) {
const coreAudit = coreAuditList.find(a => a === auditPathJs);
let requirePath = `../audits/${auditPath}`;
if (!coreAudit) {
// TODO: refactor and delete `global.isDevtools`.
if (global.isDevtools || global.isLightrider) {
// This is for pubads bundling.
if (isBundledEnvironment()) {
// This is for pubads bundling.
requirePath = auditPath;
} else {
// Otherwise, attempt to find it elsewhere. This throws if not found.
Expand Down Expand Up @@ -298,6 +313,34 @@ function resolveSettings(settingsJson = {}, overrides = undefined) {
}


/**
* @param {LH.Config.Json} configJSON
* @param {string | undefined} configDir
* @param {{plugins?: string[]} | undefined} flags
* @return {LH.Config.Json}
*/
function mergePlugins(configJSON, configDir, flags) {
const configPlugins = configJSON.plugins || [];
const flagPlugins = (flags && flags.plugins) || [];
const pluginNames = new Set([...configPlugins, ...flagPlugins]);

for (const pluginName of pluginNames) {
validation.assertValidPluginName(configJSON, pluginName);

// In bundled contexts, `resolveModulePath` will fail, so use the raw pluginName directly.
const pluginPath = isBundledEnvironment() ?
pluginName :
resolveModulePath(pluginName, configDir, 'plugin');
const rawPluginJson = require(pluginPath);
const pluginJson = ConfigPlugin.parsePlugin(rawPluginJson, pluginName);

configJSON = mergeConfigFragment(configJSON, pluginJson);
}

return configJSON;
}


/**
* Turns a GathererJson into a GathererDefn which involves a few main steps:
* - Expanding the JSON shorthand the full definition format.
Expand Down Expand Up @@ -528,6 +571,7 @@ module.exports = {
mergeOptionsOfItems,
mergeConfigFragment,
mergeConfigFragmentArrayByKey,
mergePlugins,
resolveSettings,
resolveGathererToDefn,
resolveAuditsToDefns,
Expand Down
48 changes: 2 additions & 46 deletions lighthouse-core/config/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,12 @@ const validation = require('./../fraggle-rock/config/validation.js');
const log = require('lighthouse-logger');
const path = require('path');
const Runner = require('../runner.js');
const ConfigPlugin = require('./config-plugin.js');
const {
mergePlugins,
mergeConfigFragment,
resolveSettings,
resolveAuditsToDefns,
resolveGathererToDefn,
resolveModulePath,
deepClone,
deepCloneConfigJson,
} = require('./config-helpers.js');
Expand Down Expand Up @@ -148,22 +147,6 @@ function assertValidFlags(flags) {
}
}

/**
* Throws if pluginName is invalid or (somehow) collides with a category in the
* configJSON being added to.
* @param {LH.Config.Json} configJSON
* @param {string} pluginName
*/
function assertValidPluginName(configJSON, pluginName) {
if (!pluginName.startsWith('lighthouse-plugin-')) {
throw new Error(`plugin name '${pluginName}' does not start with 'lighthouse-plugin-'`);
}

if (configJSON.categories && configJSON.categories[pluginName]) {
throw new Error(`plugin name '${pluginName}' not allowed because it is the id of a category already found in config`); // eslint-disable-line max-len
}
}


/**
* @implements {LH.Config.Config}
Expand Down Expand Up @@ -203,7 +186,7 @@ class Config {
const configDir = configPath ? path.dirname(configPath) : undefined;

// Validate and merge in plugins (if any).
configJSON = Config.mergePlugins(configJSON, flags, configDir);
configJSON = mergePlugins(configJSON, configDir, flags);

if (flags) {
assertValidFlags(flags);
Expand Down Expand Up @@ -294,33 +277,6 @@ class Config {
return mergeConfigFragment(baseJSON, extendJSON);
}

/**
* @param {LH.Config.Json} configJSON
* @param {LH.Flags=} flags
* @param {string=} configDir
* @return {LH.Config.Json}
*/
static mergePlugins(configJSON, flags, configDir) {
const configPlugins = configJSON.plugins || [];
const flagPlugins = (flags && flags.plugins) || [];
const pluginNames = new Set([...configPlugins, ...flagPlugins]);

for (const pluginName of pluginNames) {
assertValidPluginName(configJSON, pluginName);

// TODO: refactor and delete `global.isDevtools`.
const pluginPath = global.isDevtools || global.isLightrider ?
pluginName :
resolveModulePath(pluginName, configDir, 'plugin');
const rawPluginJson = require(pluginPath);
const pluginJson = ConfigPlugin.parsePlugin(rawPluginJson, pluginName);

configJSON = Config.extendConfigJSON(configJSON, pluginJson);
}

return configJSON;
}

/**
* @param {LH.Config.Json['passes']} passes
* @return {?Array<Required<LH.Config.PassJson>>}
Expand Down
3 changes: 2 additions & 1 deletion lighthouse-core/fraggle-rock/config/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@ const {
resolveSettings,
resolveAuditsToDefns,
resolveGathererToDefn,
mergePlugins,
mergeConfigFragment,
mergeConfigFragmentArrayByKey,
} = require('../../config/config-helpers.js');
Expand Down Expand Up @@ -250,7 +251,7 @@ function initializeConfig(configJSON, context) {
let {configWorkingCopy, configDir} = resolveWorkingCopy(configJSON, context); // eslint-disable-line prefer-const

configWorkingCopy = resolveExtensions(configWorkingCopy);

configWorkingCopy = mergePlugins(configWorkingCopy, configDir, context.settingsOverrides);

const settings = resolveSettings(configWorkingCopy.settings || {}, context.settingsOverrides);
overrideSettingsForGatherMode(settings, context);
Expand Down
17 changes: 17 additions & 0 deletions lighthouse-core/fraggle-rock/config/validation.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,22 @@ function isValidArtifactDependency(dependent, dependency) {
return true;
}

/**
* Throws if pluginName is invalid or (somehow) collides with a category in the
* configJSON being added to.
* @param {LH.Config.Json} configJSON
* @param {string} pluginName
*/
function assertValidPluginName(configJSON, pluginName) {
if (!pluginName.startsWith('lighthouse-plugin-')) {
throw new Error(`plugin name '${pluginName}' does not start with 'lighthouse-plugin-'`);
}

if (configJSON.categories && configJSON.categories[pluginName]) {
throw new Error(`plugin name '${pluginName}' not allowed because it is the id of a category already found in config`); // eslint-disable-line max-len
}
}

/**
* Throws an error if the provided object does not implement the required Fraggle Rock gatherer interface.
* @param {LH.Config.AnyFRGathererDefn} gathererDefn
Expand Down Expand Up @@ -283,6 +299,7 @@ function throwInvalidArtifactDependency(artifactId, dependencyKey) {
module.exports = {
isFRGathererDefn,
isValidArtifactDependency,
assertValidPluginName,
assertValidFRGatherer,
assertValidFRNavigations,
assertValidAudit,
Expand Down
60 changes: 60 additions & 0 deletions lighthouse-core/test/config/config-helpers-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const {
resolveGathererToDefn,
resolveAuditsToDefns,
resolveModulePath,
mergePlugins,
mergeConfigFragment,
mergeConfigFragmentArrayByKey,
} = require('../../config/config-helpers.js');
Expand All @@ -23,6 +24,7 @@ const Gatherer = require('../../gather/gatherers/gatherer.js');
const ImageElementsGatherer = require('../../gather/gatherers/image-elements.js');
const UserTimingsAudit = require('../../audits/user-timings.js');
const {isNode12SmallIcu} = require('../test-utils.js');
const {LH_ROOT} = require('../../../root.js');

jest.mock('process', () => ({
cwd: () => jest.fn(),
Expand Down Expand Up @@ -186,6 +188,64 @@ describe('.deepCloneConfigJson', () => {
});
});

describe('.mergePlugins', () => {
// Include a configPath flag so that config.js looks for the plugins in the fixtures dir.
const configDir = `${LH_ROOT}/lighthouse-core/test/fixtures/config-plugins/`;

it('merge plugins from the config', () => {
const configJson = {
audits: ['installable-manifest', 'metrics'],
plugins: ['lighthouse-plugin-simple'],
};

const config = mergePlugins(configJson, configDir, {});
expect(config).toMatchObject({
audits: [
'installable-manifest',
'metrics',
{path: 'redirects'},
{path: 'user-timings'},
],
categories: {
'lighthouse-plugin-simple': {title: 'Simple'},
},
groups: {
'lighthouse-plugin-simple-new-group': {title: 'New Group'},
},
});
});

it('merge plugins from flags', () => {
const configJson = {
audits: ['installable-manifest', 'metrics'],
plugins: ['lighthouse-plugin-simple'],
};
const flags = {plugins: ['lighthouse-plugin-no-groups']};
const config = mergePlugins(configJson, configDir, flags);

expect(config.categories).toHaveProperty('lighthouse-plugin-simple');
expect(config.categories).toHaveProperty('lighthouse-plugin-no-groups');
});

it('validate plugin name', () => {
const configJson = {audits: ['installable-manifest', 'metrics']};
const flags = {plugins: ['not-a-plugin']};
expect(() => mergePlugins(configJson, configDir, flags)).toThrow(/does not start/);
});

it('validate plugin existence', () => {
const configJson = {audits: ['installable-manifest', 'metrics']};
const flags = {plugins: ['lighthouse-plugin-missing']};
expect(() => mergePlugins(configJson, configDir, flags)).toThrow(/Unable to locate plugin/);
});

it('validate plugin structure', () => {
const configJson = {audits: ['installable-manifest', 'metrics']};
const flags = {plugins: ['lighthouse-plugin-no-category']};
expect(() => mergePlugins(configJson, configDir, flags)).toThrow(/no valid category/);
});
});

describe('.resolveSettings', () => {
it('resolves the locale', () => {
const settings = resolveSettings({locale: 'zh-CN'});
Expand Down
20 changes: 18 additions & 2 deletions lighthouse-core/test/fraggle-rock/config/config-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ const BaseAudit = require('../../../audits/audit.js');
const {nonSimulatedPassConfigOverrides} = require('../../../config/constants.js');
const BaseGatherer = require('../../../fraggle-rock/gather/base-gatherer.js');
const {initializeConfig} = require('../../../fraggle-rock/config/config.js');
const {LH_ROOT} = require('../../../../root.js');

/* eslint-env jest */

Expand Down Expand Up @@ -115,6 +116,23 @@ describe('Fraggle Rock Config', () => {
expect(auditIds).not.toContain('robots-txt'); // from skipAudits
});

it('should support plugins', () => {
const {config} = initializeConfig(undefined, {
gatherMode: 'navigation',
configPath: `${LH_ROOT}/lighthouse-core/test/fixtures/config-plugins/`,
settingsOverrides: {plugins: ['lighthouse-plugin-simple']},
});

expect(config).toMatchObject({
categories: {
'lighthouse-plugin-simple': {title: 'Simple'},
},
groups: {
'lighthouse-plugin-simple-new-group': {title: 'New Group'},
},
});
});

describe('resolveArtifactDependencies', () => {
/** @type {LH.Gatherer.FRGathererInstance} */
let dependencyGatherer;
Expand Down Expand Up @@ -460,6 +478,4 @@ describe('Fraggle Rock Config', () => {
const invocation = () => initializeConfig(extensionConfig, {gatherMode: 'navigation'});
expect(invocation).toThrow(/did not support any gather modes/);
});

it.todo('should support plugins');
});
Loading