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 config extension support #12832

Merged
merged 2 commits into from
Jul 28, 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
36 changes: 36 additions & 0 deletions lighthouse-core/config/config-helpers.js
Original file line number Diff line number Diff line change
Expand Up @@ -93,6 +93,41 @@ function _mergeConfigFragment(base, extension, overwriteArrays = false) {
*/
const mergeConfigFragment = _mergeConfigFragment;

/**
* Merge an array of items by a caller-defined key. `mergeConfigFragment` is used to merge any items
* with a matching key.
*
* @template T
* @param {Array<T>|null|undefined} baseArray
* @param {Array<T>|null|undefined} extensionArray
* @param {(item: T) => string} keyFn
* @return {Array<T>}
*/
function mergeConfigFragmentArrayByKey(baseArray, extensionArray, keyFn) {
/** @type {Map<string, {index: number, item: T}>} */
const itemsByKey = new Map();
const mergedArray = baseArray || [];
for (let i = 0; i < mergedArray.length; i++) {
const item = mergedArray[i];
itemsByKey.set(keyFn(item), {index: i, item});
}

for (const item of extensionArray || []) {
const baseItemEntry = itemsByKey.get(keyFn(item));
if (baseItemEntry) {
const baseItem = baseItemEntry.item;
const merged = typeof item === 'object' && typeof baseItem === 'object' ?
mergeConfigFragment(baseItem, item, true) :
item;
mergedArray[baseItemEntry.index] = merged;
} else {
mergedArray.push(item);
}
}

return mergedArray;
}

/**
* Validate the settings after they've been built
* @param {LH.Config.Settings} settings
Expand Down Expand Up @@ -558,6 +593,7 @@ module.exports = {
deepCloneConfigJson,
mergeOptionsOfItems,
mergeConfigFragment,
mergeConfigFragmentArrayByKey,
resolveSettings,
resolveGathererToDefn,
resolveAuditsToDefns,
Expand Down
36 changes: 34 additions & 2 deletions lighthouse-core/fraggle-rock/config/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,8 @@ const {
resolveSettings,
resolveAuditsToDefns,
resolveGathererToDefn,
mergeConfigFragment,
mergeConfigFragmentArrayByKey,
} = require('../../config/config-helpers.js');
const defaultConfigPath = path.join(__dirname, './default-config.js');

Expand Down Expand Up @@ -53,6 +55,35 @@ function resolveWorkingCopy(configJSON, context) {
};
}

/**
* @param {LH.Config.Json} configJSON
* @return {LH.Config.Json}
*/
function resolveExtensions(configJSON) {
if (!configJSON.extends) return configJSON;

if (configJSON.extends !== 'lighthouse:default') {
throw new Error('`lighthouse:default` is the only valid extension method.');
}

const {artifacts, navigations, ...extensionJSON} = configJSON;
const defaultClone = deepCloneConfigJson(defaultConfig);
const mergedConfig = mergeConfigFragment(defaultClone, extensionJSON);

mergedConfig.artifacts = mergeConfigFragmentArrayByKey(
defaultClone.artifacts,
artifacts,
artifact => artifact.id
);
mergedConfig.navigations = mergeConfigFragmentArrayByKey(
defaultClone.navigations,
navigations,
navigation => navigation.id
);

return mergedConfig;
}

/**
* Looks up the required artifact IDs for each dependency, throwing if no earlier artifact satisfies the dependency.
*
Expand Down Expand Up @@ -170,9 +201,10 @@ function initializeConfig(configJSON, context) {
const status = {msg: 'Initialize config', id: 'lh:config'};
log.time(status, 'verbose');

const {configWorkingCopy, configDir} = resolveWorkingCopy(configJSON, context);
let {configWorkingCopy, configDir} = resolveWorkingCopy(configJSON, context); // eslint-disable-line prefer-const

configWorkingCopy = resolveExtensions(configWorkingCopy);

// TODO(FR-COMPAT): handle config extension
// TODO(FR-COMPAT): handle config plugins

const settings = resolveSettings(configWorkingCopy.settings || {}, context.settingsOverrides);
Expand Down
8 changes: 7 additions & 1 deletion lighthouse-core/fraggle-rock/config/default-config.js
Original file line number Diff line number Diff line change
Expand Up @@ -191,7 +191,13 @@ const defaultConfig = {
},
],
settings: legacyDefaultConfig.settings,
audits: [...(legacyDefaultConfig.audits || []), ...frAudits],
audits: [
...(legacyDefaultConfig.audits || []).map(audit => {
if (typeof audit === 'string') return {path: audit};
return audit;
}),
...frAudits,
],
categories: mergeCategories(),
groups: legacyDefaultConfig.groups,
};
Expand Down
39 changes: 39 additions & 0 deletions lighthouse-core/test/config/config-helpers-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ const {
resolveAuditsToDefns,
resolveModulePath,
mergeConfigFragment,
mergeConfigFragmentArrayByKey,
} = require('../../config/config-helpers.js');
const Runner = require('../../runner.js');
const Gatherer = require('../../gather/gatherers/gatherer.js');
Expand Down Expand Up @@ -83,6 +84,44 @@ describe('.mergeConfigFragment', () => {
});
});

describe('.mergeConfigFragmentArrayByKey', () => {
it('should use mergeConfigFragment to merge items', () => {
const base = [{a: 1, b: 'yes', c: true}];
const extension = [{a: 2, c: false, d: 123}];
const merged = mergeConfigFragmentArrayByKey(base, extension, () => 'key');
expect(merged).toBe(base);
expect(merged).toEqual([{a: 2, b: 'yes', c: false, d: 123}]);
});

it('should merge by the keyFn', () => {
const base = [{id: 'a', value: 1}, {id: 'b', value: 2}];
const extension = [{id: 'b', value: 1}, {id: 'a', value: 2}, {id: 'c'}];
const merged = mergeConfigFragmentArrayByKey(base, extension, item => item.id);
expect(merged).toEqual([{id: 'a', value: 2}, {id: 'b', value: 1}, {id: 'c'}]);
});

it('should merge recursively', () => {
const base = [{foo: {bar: 1}}];
const extension = [{foo: {baz: 2, bam: 3}}];
const merged = mergeConfigFragmentArrayByKey(base, extension, () => 'key');
expect(merged).toEqual([{foo: {bar: 1, baz: 2, bam: 3}}]);
});

it('should handle null items in base', () => {
const base = [null];
const extension = [{x: 1}];
const merged = mergeConfigFragmentArrayByKey(base, extension, () => '');
expect(merged).toEqual([{x: 1}]);
});

it('should handle undefined items in extension', () => {
const base = [{x: 1}];
const extension = [undefined];
const merged = mergeConfigFragmentArrayByKey(base, extension, () => '');
expect(merged).toEqual([undefined]);
});
});

describe('.deepClone', () => {
it('should clone things deeply', () => {
const input = {a: {b: {c: 1}}};
Expand Down
127 changes: 125 additions & 2 deletions lighthouse-core/test/fraggle-rock/config/config-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
*/
'use strict';

const BaseAudit = require('../../../audits/audit.js');
const BaseGatherer = require('../../../fraggle-rock/gather/base-gatherer.js');
const {initializeConfig} = require('../../../fraggle-rock/config/config.js');

Expand Down Expand Up @@ -262,9 +263,131 @@ describe('Fraggle Rock Config', () => {
});
});

it.todo('should support extension');
describe('.resolveExtensions', () => {
/** @type {LH.Config.Json} */
let extensionConfig;

beforeEach(() => {
const gatherer = new BaseGatherer();
gatherer.meta = {supportedModes: ['navigation']};

class ExtraAudit extends BaseAudit {
static get meta() {
return {
id: 'extra-audit',
title: 'Extra',
description: 'Extra',
requiredArtifacts: /** @type {*} */ (['ExtraArtifact']),
};
}

/** @return {LH.Audit.Product} */
static audit() {
throw new Error('Unimplemented');
}
}

extensionConfig = {
extends: 'lighthouse:default',
artifacts: [
{id: 'ExtraArtifact', gatherer: {instance: gatherer}},
],
navigations: [
{id: 'default', artifacts: ['ExtraArtifact']},
],
audits: [
{implementation: ExtraAudit},
],
categories: {
performance: {
title: 'Performance',
auditRefs: [
{id: 'extra-audit', weight: 0},
],
},
},
};
});

it('should do nothing when not extending', () => {
const {config} = initializeConfig({
artifacts: [
{id: 'Accessibility', gatherer: 'accessibility'},
],
navigations: [
{id: 'default', artifacts: ['Accessibility']},
],
}, {gatherMode: 'navigation'});

expect(config).toMatchObject({
audits: null,
groups: null,
artifacts: [
{id: 'Accessibility'},
],
navigations: [
{id: 'default', artifacts: [{id: 'Accessibility'}]},
],
});
});

it('should extend the default config with filters', () => {
const gatherMode = 'navigation';
const {config} = initializeConfig({
extends: 'lighthouse:default',
settings: {onlyCategories: ['accessibility']},
}, {gatherMode});
if (!config.artifacts) throw new Error(`No artifacts created`);
if (!config.audits) throw new Error(`No audits created`);

const hasAccessibilityArtifact = config.artifacts.some(a => a.id === 'Accessibility');
if (!hasAccessibilityArtifact) expect(config.artifacts).toContain('Accessibility');

const hasAccessibilityAudit = config.audits.
some(a => a.implementation.meta.id === 'color-contrast');
if (!hasAccessibilityAudit) expect(config.audits).toContain('color-contrast');

expect(config.categories).toHaveProperty('accessibility');
expect(config.categories).not.toHaveProperty('performance');
});

it('should merge in artifacts', () => {
const {config} = initializeConfig(extensionConfig, {gatherMode: 'navigation'});
if (!config.artifacts) throw new Error(`No artifacts created`);

const hasExtraArtifact = config.artifacts.some(a => a.id === 'ExtraArtifact');
if (!hasExtraArtifact) expect(config.artifacts).toContain('ExtraArtifact');
});

it('should merge in navigations', () => {
const {config} = initializeConfig(extensionConfig, {gatherMode: 'navigation'});
if (!config.navigations) throw new Error(`No navigations created`);

expect(config.navigations).toHaveLength(1);
const hasNavigation = config.navigations[0].artifacts.
some(a => a.id === 'ExtraArtifact');
if (!hasNavigation) expect(config.navigations[0].artifacts).toContain('ExtraArtifact');
});

it('should merge in audits', () => {
const {config} = initializeConfig(extensionConfig, {gatherMode: 'navigation'});
if (!config.audits) throw new Error(`No audits created`);

const hasExtraAudit = config.audits.
some(a => a.implementation.meta.id === 'extra-audit');
if (!hasExtraAudit) expect(config.audits).toContain('extra-audit');
});

it('should merge in categories', () => {
const {config} = initializeConfig(extensionConfig, {gatherMode: 'navigation'});
if (!config.categories) throw new Error(`No categories created`);

const hasCategory = config.categories.performance.auditRefs.some(a => a.id === 'extra-audit');
if (!hasCategory) expect(config.categories.performance.auditRefs).toContain('extra-audit');
});
});

it.todo('should support plugins');
it.todo('should adjust default pass options for throttling method');
it.todo('should validate audit/gatherer interdependencies');
it.todo('should validate gatherers do not support all 3 modes');
});