core(config): clean flags for config settings (#4960)

Author: patrickhulce

lighthouse-core/config/config.js

@@ -246,8 +246,27 @@ function expandArtifacts(artifacts) {
   return artifacts;
 }
 
+/**
+ * Creates a settings object from potential flags object by dropping all the properties
+ * that don't exist on Config.Settings.
+ *
+ * @param {LH.Flags=} flags
+ * @return {Partial<LH.Config.Settings>}
+ */
+function cleanFlagsForSettings(flags = {}) {
+  const settings = {};
+  for (const key of Object.keys(flags)) {
+    if (typeof constants.defaultSettings[key] !== 'undefined') {
+      settings[key] = flags[key];
+    }
+  }
+
+  return settings;
+}
+
 function merge(base, extension) {
-  if (typeof base === 'undefined') {
+  // If the default value doesn't exist or is explicitly null, defer to the extending value
+  if (typeof base === 'undefined' || base === null) {
     return extension;
   } else if (Array.isArray(extension)) {
     if (!Array.isArray(base)) throw new TypeError(`Expected array but got ${typeof base}`);
@@ -330,7 +349,7 @@ class Config {
     configJSON.passes = Config.expandGathererShorthandAndMergeOptions(configJSON.passes);
 
     // Override any applicable settings with CLI flags
-    configJSON.settings = merge(configJSON.settings || {}, flags || {});
+    configJSON.settings = merge(configJSON.settings || {}, cleanFlagsForSettings(flags));
 
     // Generate a limited config if specified
     if (Array.isArray(configJSON.settings.onlyCategories) ||

lighthouse-core/config/constants.js

@@ -26,10 +26,24 @@ const throttling = {
   },
 };
 
+/** @type {LH.Config.Settings} */
 const defaultSettings = {
   maxWaitForLoad: 45 * 1000,
   throttlingMethod: 'devtools',
   throttling: throttling.mobile3G,
+  auditMode: false,
+  gatherMode: false,
+  disableStorageReset: false,
+  disableDeviceEmulation: false,
+
+  // the following settings have no defaults but we still want ensure that `key in settings`
+  // in config will work in a typechecked way
+  blockedUrlPatterns: null,
+  additionalTraceCategories: null,
+  extraHeaders: null,
+  onlyAudits: null,
+  onlyCategories: null,
+  skipAudits: null,
 };
 
 const defaultPassConfig = {

lighthouse-core/gather/driver.js

@@ -828,7 +828,7 @@ class Driver {
   }
 
   /**
-   * @param {{additionalTraceCategories?: string}=} settings
+   * @param {{additionalTraceCategories?: string|null}=} settings
    * @return {Promise<void>}
    */
   beginTrace(settings) {
@@ -1019,7 +1019,7 @@ class Driver {
   }
 
   /**
-   * @param {LH.Crdp.Network.Headers=} headers key/value pairs of HTTP Headers.
+   * @param {LH.Crdp.Network.Headers|null} headers key/value pairs of HTTP Headers.
    * @return {Promise<void>}
    */
   async setExtraHTTPHeaders(headers) {

lighthouse-core/test/config/config-test.js

@@ -418,6 +418,12 @@ describe('Config', () => {
     });
   });
 
+  it('cleans up flags for settings', () => {
+    const config = new Config({extends: true}, {nonsense: 1, foo: 2, throttlingMethod: 'provided'});
+    assert.equal(config.settings.throttlingMethod, 'provided');
+    assert.ok(config.settings.nonsense === undefined, 'did not cleanup settings');
+  });
+
   it('extends the full config', () => {
     class CustomAudit extends Audit {
       static get meta() {

typings/config.d.ts

@@ -24,11 +24,11 @@ declare global {
         settings?: SettingsJson;
         passes?: PassJson[];
       }
-  
+
       export interface SettingsJson extends SharedFlagsSettings {
-        extraHeaders?: Crdp.Network.Headers;
+        extraHeaders?: Crdp.Network.Headers | null;
       }
-  
+
       export interface PassJson {
         passName: string;
         recordTrace?: boolean;
@@ -41,7 +41,7 @@ declare global {
         blankDuration?: number;
         gatherers: GathererJson[];
       }
-  
+
       export type GathererJson = {
         path: string;
         options?: {};
@@ -52,14 +52,14 @@ declare global {
         instance: InstanceType<typeof Gatherer>;
         options?: {};
       } | string;
-  
+
       // TODO(bckenny): we likely don't want to require all these
       export type Settings = Required<SettingsJson>;
-  
+
       export interface Pass extends Required<PassJson> {
         gatherers: GathererDefn[];
       }
-  
+
       export interface GathererDefn {
         implementation: typeof Gatherer;
         instance: InstanceType<typeof Gatherer>;

typings/externs.d.ts

@@ -35,17 +35,17 @@ declare global {
 
     interface SharedFlagsSettings {
       maxWaitForLoad?: number;
-      blockedUrlPatterns?: string[];
-      additionalTraceCategories?: string;
+      blockedUrlPatterns?: string[] | null;
+      additionalTraceCategories?: string | null;
       auditMode?: boolean | string;
       gatherMode?: boolean | string;
       disableStorageReset?: boolean;
       disableDeviceEmulation?: boolean;
       throttlingMethod?: 'devtools'|'simulate'|'provided';
       throttling?: ThrottlingSettings;
-      onlyAudits?: string[];
-      onlyCategories?: string[];
-      skipAudits?: string[];
+      onlyAudits?: string[] | null;
+      onlyCategories?: string[] | null;
+      skipAudits?: string[] | null;
     }
 
     export interface Flags extends SharedFlagsSettings {