From 54a7c63d30eec2445e11dd0c98b4d66b1157a27d Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Thu, 3 Dec 2020 13:08:41 -0600 Subject: [PATCH 1/2] core(config): refactor config in preparation for fraggle rock types --- lighthouse-core/config/config-helpers.js | 68 +++++++++++ lighthouse-core/config/config.js | 66 ++--------- lighthouse-core/runner.js | 2 +- .../test/config/config-helpers-test.js | 108 ++++++++++++++++++ types/config.d.ts | 36 +++++- 5 files changed, 219 insertions(+), 61 deletions(-) create mode 100644 lighthouse-core/test/config/config-helpers-test.js diff --git a/lighthouse-core/config/config-helpers.js b/lighthouse-core/config/config-helpers.js index 969178b32c98..c7345c0bc359 100644 --- a/lighthouse-core/config/config-helpers.js +++ b/lighthouse-core/config/config-helpers.js @@ -214,7 +214,75 @@ function resolveModule(moduleIdentifier, configDir, category) { ${relativePath}`); } +/** + * Many objects in the config can be an object whose properties are not serializable. + * We use a shallow clone for these objects instead. + * Any value that isn't an object will not be cloned. + * + * @template T + * @param {T} item + * @return {T} + */ +function shallowClone(item) { + if (typeof item === 'object') { + // Return copy of instance and prototype chain (in case item is instantiated class). + return Object.assign( + Object.create( + Object.getPrototypeOf(item) + ), + item + ); + } + + return item; +} + +/** + * // TODO(bckenny): could adopt "jsonified" type to ensure T will survive JSON + * round trip: https://github.com/Microsoft/TypeScript/issues/21838 + * @template T + * @param {T} json + * @return {T} + */ +function deepClone(json) { + return JSON.parse(JSON.stringify(json)); +} + +/** + * Deep clone a ConfigJson, copying over any "live" gatherer or audit that + * wouldn't make the JSON round trip. + * @param {LH.Config.Json} json + * @return {LH.Config.Json} + */ +function deepCloneConfigJson(json) { + const cloned = deepClone(json); + + // Copy arrays that could contain non-serializable properties to allow for programmatic + // injection of audit and gatherer implementations. + if (Array.isArray(cloned.passes) && Array.isArray(json.passes)) { + for (let i = 0; i < cloned.passes.length; i++) { + const pass = cloned.passes[i]; + pass.gatherers = (json.passes[i].gatherers || []).map(gatherer => shallowClone(gatherer)); + } + } + + if (Array.isArray(json.audits)) { + cloned.audits = json.audits.map(audit => shallowClone(audit)); + } + + if (Array.isArray(json.artifacts)) { + cloned.artifacts = json.artifacts.map(artifact => ({ + ...artifact, + gatherer: shallowClone(artifact.gatherer), + })); + } + + return cloned; +} + module.exports = { + deepClone, + deepCloneConfigJson, mergeOptionsOfItems, requireAudits, resolveModule, diff --git a/lighthouse-core/config/config.js b/lighthouse-core/config/config.js index 1190ecff58d4..7b620b119f98 100644 --- a/lighthouse-core/config/config.js +++ b/lighthouse-core/config/config.js @@ -16,7 +16,12 @@ const path = require('path'); const Runner = require('../runner.js'); const ConfigPlugin = require('./config-plugin.js'); const Budget = require('./budget.js'); -const {requireAudits, resolveModule} = require('./config-helpers.js'); +const { + requireAudits, + resolveModule, + deepClone, + deepCloneConfigJson, +} = require('./config-helpers.js'); /** @typedef {typeof import('../gather/gatherers/gatherer.js')} GathererConstructor */ /** @typedef {InstanceType} Gatherer */ @@ -241,64 +246,7 @@ function _merge(base, extension, overwriteArrays = false) { const merge = _merge; /** - * @template T - * @param {Array} array - * @return {Array} - */ -function cloneArrayWithPluginSafety(array) { - return array.map(item => { - if (typeof item === 'object') { - // Return copy of instance and prototype chain (in case item is instantiated class). - return Object.assign( - Object.create( - Object.getPrototypeOf(item) - ), - item - ); - } - - return item; - }); -} - -/** - * // TODO(bckenny): could adopt "jsonified" type to ensure T will survive JSON - * round trip: https://github.com/Microsoft/TypeScript/issues/21838 - * @template T - * @param {T} json - * @return {T} - */ -function deepClone(json) { - return JSON.parse(JSON.stringify(json)); -} - -/** - * Deep clone a ConfigJson, copying over any "live" gatherer or audit that - * wouldn't make the JSON round trip. - * @param {LH.Config.Json} json - * @return {LH.Config.Json} - */ -function deepCloneConfigJson(json) { - const cloned = deepClone(json); - - // Copy arrays that could contain plugins to allow for programmatic - // injection of plugins. - if (Array.isArray(cloned.passes) && Array.isArray(json.passes)) { - for (let i = 0; i < cloned.passes.length; i++) { - const pass = cloned.passes[i]; - pass.gatherers = cloneArrayWithPluginSafety(json.passes[i].gatherers || []); - } - } - - if (Array.isArray(json.audits)) { - cloned.audits = cloneArrayWithPluginSafety(json.audits); - } - - return cloned; -} - -/** - * @implements {LH.Config.Json} + * @implements {LH.Config.Config} */ class Config { /** diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js index 2bcce2b5bd2e..97d6f6edb7f9 100644 --- a/lighthouse-core/runner.js +++ b/lighthouse-core/runner.js @@ -22,7 +22,7 @@ const generateReport = require('./report/report-generator.js').generateReport; const LHError = require('./lib/lh-error.js'); /** @typedef {import('./gather/connections/connection.js')} Connection */ -/** @typedef {import('./config/config.js')} Config */ +/** @typedef {LH.Config.Config} Config */ class Runner { /** diff --git a/lighthouse-core/test/config/config-helpers-test.js b/lighthouse-core/test/config/config-helpers-test.js new file mode 100644 index 000000000000..555a2b83cb9c --- /dev/null +++ b/lighthouse-core/test/config/config-helpers-test.js @@ -0,0 +1,108 @@ +/** + * @license Copyright 2020 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'; + +const helpers = require('../../config/config-helpers.js'); +const Gatherer = require('../../gather/gatherers/gatherer.js'); +const UserTimingsAudit = require('../../audits/user-timings.js'); + +/* eslint-env jest */ + +describe('.deepClone', () => { + it('should clone things deeply', () => { + const input = {a: {b: {c: 1}}}; + const output = helpers.deepClone(input); + expect(output).not.toBe(input); + expect(output).toEqual(input); + output.a.b.c = 2; + expect(input.a.b.c).toEqual(1); + }); +}); + +describe('.deepCloneConfigJson', () => { + it('should clone a config deeply', () => { + const TimingGatherer = new Gatherer(); + const input = { + artifacts: [{id: 'Timing', gatherer: TimingGatherer}], + passes: [{passName: 'defaultPass', gatherers: []}], + audits: [{path: 'user-timings'}], + categories: {random: {auditRefs: [{id: 'user-timings'}]}}, + }; + + const output = helpers.deepCloneConfigJson(input); + expect(output).not.toBe(input); + expect(output).toEqual(input); + output.artifacts[0].id = 'NewName'; + output.passes[0].passName = 'newName'; + output.audits[0].path = 'new-audit'; + output.categories.random.auditRefs[0].id = 'new-audit'; + expect(input.artifacts[0].id).toEqual('Timing'); + expect(input.passes[0].passName).toEqual('defaultPass'); + expect(input.audits[0].path).toEqual('user-timings'); + expect(input.categories.random.auditRefs[0].id).toEqual('user-timings'); + }); + + it('should preserve gatherer implementations in passes', () => { + const TimingGatherer = new Gatherer(); + const input = { + passes: [{passName: 'defaultPass', gatherers: [TimingGatherer]}], + }; + + const output = helpers.deepCloneConfigJson(input); + expect(output.passes[0].gatherers[0]).toEqual(TimingGatherer); + }); + + it('should preserve gatherer implementations in artifacts', () => { + const TimingGatherer = new Gatherer(); + const input = { + artifacts: [{id: 'Timing', gatherer: TimingGatherer}], + }; + + const output = helpers.deepCloneConfigJson(input); + expect(output.artifacts[0].gatherer).toEqual(TimingGatherer); + }); + + it('should preserve audit implementations', () => { + const input = { + audits: [{implementation: UserTimingsAudit}], + }; + + const output = helpers.deepCloneConfigJson(input); + expect(output.audits[0].implementation).toEqual(UserTimingsAudit); + }); +}); + + +describe('.requireAudits', () => { + it('should expand audit short-hand', () => { + const result = helpers.requireAudits(['user-timings']); + + expect(result).toEqual([{path: 'user-timings', options: {}, implementation: UserTimingsAudit}]); + }); + + it('should handle multiple audit definition styles', () => { + const result = helpers.requireAudits(['user-timings', {implementation: UserTimingsAudit}]); + + expect(result).toMatchObject([{path: 'user-timings'}, {implementation: UserTimingsAudit}]); + }); + + it('should merge audit options', () => { + const audits = [ + 'user-timings', + {path: 'is-on-https', options: {x: 1, y: 1}}, + {path: 'is-on-https', options: {x: 2}}, + ]; + const merged = helpers.requireAudits(audits); + expect(merged).toMatchObject([ + {path: 'user-timings', options: {}}, + {path: 'is-on-https', options: {x: 2, y: 1}}, + ]); + }); + + it('throws for invalid auditDefns', () => { + expect(() => helpers.requireAudits([new Gatherer()])).toThrow(/Invalid Audit type/); + }); +}); diff --git a/types/config.d.ts b/types/config.d.ts index 6a519c301d88..a950f57bd8f5 100644 --- a/types/config.d.ts +++ b/types/config.d.ts @@ -1,3 +1,4 @@ +/* eslint-disable strict */ /** * @license Copyright 2018 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 @@ -16,6 +17,7 @@ declare global { export interface Json { extends?: 'lighthouse:default' | string | boolean; settings?: SharedFlagsSettings; + artifacts?: ArtifactJson[] | null; passes?: PassJson[] | null; audits?: Config.AuditJson[] | null; categories?: Record | null; @@ -23,6 +25,28 @@ declare global { plugins?: Array, } + /** + * The normalized and fully resolved config. + */ + export interface Config { + settings: Settings; + passes: Pass[] | null; + audits: AuditDefn[] | null; + categories: Record | null; + groups: Record | null; + } + + /** + * The normalized and fully resolved Fraggle Rock config. + */ + export interface FRConfig { + settings: Settings; + artifacts: ArtifactDefn[] | null; + audits: AuditDefn[] | null; + categories: Record | null; + groups: Record | null; + } + export interface PassJson { passName: string; loadFailureMode?: 'fatal'|'warn'|'ignore'; @@ -37,6 +61,11 @@ declare global { gatherers?: GathererJson[]; } + export interface ArtifactJson { + id: string; + gatherer: GathererJson; + } + export type GathererJson = { path: string; options?: {}; @@ -86,6 +115,11 @@ declare global { gatherers: GathererDefn[]; } + export interface ArtifactDefn { + id: string; + gatherer: GathererDefn; + } + export interface GathererDefn { implementation?: typeof Gatherer; instance: InstanceType; @@ -125,4 +159,4 @@ declare global { } // empty export to keep file a module -export {} +export {}; From 100a005169f52471a39b3449dfb4dfb7a73cfcc3 Mon Sep 17 00:00:00 2001 From: Patrick Hulce Date: Wed, 16 Dec 2020 16:59:34 -0600 Subject: [PATCH 2/2] fix merge mistake --- lighthouse-core/test/config/config-helpers-test.js | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lighthouse-core/test/config/config-helpers-test.js b/lighthouse-core/test/config/config-helpers-test.js index aa95e39430a0..b19c3584bba0 100644 --- a/lighthouse-core/test/config/config-helpers-test.js +++ b/lighthouse-core/test/config/config-helpers-test.js @@ -9,7 +9,7 @@ const path = require('path'); const {deepClone, deepCloneConfigJson, requireAudits, resolveModule} = - require('../../config/config.js'); + require('../../config/config-helpers.js'); const Gatherer = require('../../gather/gatherers/gatherer.js'); const UserTimingsAudit = require('../../audits/user-timings.js');