cli(tsc): make LH.Flags type correct and consistent

lighthouse-cli/bin.js

@@ -14,7 +14,6 @@ const getFlags = require('./cli-flags.js').getFlags;
 const runLighthouse = require('./run').runLighthouse;
 
 const log = require('lighthouse-logger');
-// @ts-ignore
 const pkg = require('../package.json');
 const Sentry = require('../lighthouse-core/lib/sentry');
 
@@ -31,7 +30,7 @@ function isDev() {
 // Tell user if there's a newer version of LH.
 updateNotifier({pkg}).notify();
 
-const /** @type {LH.Flags} */ cliFlags = getFlags();
+const cliFlags = getFlags();
 
 // Process terminating command
 if (cliFlags.listAllAudits) {
@@ -43,7 +42,6 @@ if (cliFlags.listTraceCategories) {
   commands.listTraceCategories();
 }
 
-/** @type {string} */
 const url = cliFlags._[0];
 
 /** @type {LH.Config.Json|undefined} */
@@ -79,11 +77,18 @@ if (
 }
 
 if (cliFlags.extraHeaders) {
-  if (cliFlags.extraHeaders.substr(0, 1) !== '{') {
-    cliFlags.extraHeaders = fs.readFileSync(cliFlags.extraHeaders, 'utf-8');
+  // TODO: LH.Flags.extraHeaders is actually a string at this point, but needs to be
+  // copied over to LH.Settings.extraHeaders, which is LH.Crdp.Network.Headers. Force
+  // the conversion here, but long term either the CLI flag or the setting should have
+  // a different name.
+  // @ts-ignore
+  let extraHeadersStr = /** @type {string} */ (cliFlags.extraHeaders);
+  // If not a JSON object, assume it's a path to a JSON file.
+  if (extraHeadersStr.substr(0, 1) !== '{') {
+    extraHeadersStr = fs.readFileSync(extraHeadersStr, 'utf-8');
   }
 
-  cliFlags.extraHeaders = JSON.parse(cliFlags.extraHeaders);
+  cliFlags.extraHeaders = JSON.parse(extraHeadersStr);
 }
 
 /**

lighthouse-cli/cli-flags.js

@@ -8,13 +8,12 @@
 /* eslint-disable max-len */
 
 const yargs = require('yargs');
-// @ts-ignore
 const pkg = require('../package.json');
 const printer = require('./printer');
 
 /**
  * @param {string=} manualArgv
- * @return {!LH.Flags}
+ * @return {LH.CliFlags}
  */
 function getFlags(manualArgv) {
   // @ts-ignore yargs() is incorrectly typed as not returning itself
@@ -118,7 +117,7 @@ function getFlags(manualArgv) {
       // boolean values
       .boolean([
         'disable-storage-reset', 'disable-device-emulation', 'save-assets', 'list-all-audits',
-        'list-trace-categories', 'view', 'verbose', 'quiet', 'help',
+        'list-trace-categories', 'view', 'verbose', 'quiet', 'help', 'enable-error-reporting',
       ])
       .choices('output', printer.getValidOutputOptions())
       .choices('throttling-method', ['devtools', 'provided', 'simulate'])
@@ -137,7 +136,7 @@ function getFlags(manualArgv) {
       .default('output', ['html'])
       .default('port', 0)
       .default('hostname', 'localhost')
-      .check(/** @param {!LH.Flags} argv */ (argv) => {
+      .check(/** @param {LH.CliFlags} argv */ (argv) => {
         // Lighthouse doesn't need a URL if...
         //   - We're in auditMode (and we have artifacts already)
         //   - We're just listing the available options.

lighthouse-cli/printer.js

@@ -13,6 +13,7 @@ const log = require('lighthouse-logger');
  *   'json': JSON formatted results
  *   'html': An HTML report
  *   'csv': CSV formatted results
+ * @type {SelfMap<LH.OutputMode>}
  */
 const OutputMode = {
   json: 'json',
@@ -36,7 +37,7 @@ function checkOutputPath(path) {
 /**
  * Writes the output to stdout.
  * @param {string} output
- * @return {!Promise<undefined>}
+ * @return {Promise<void>}
  */
 function writeToStdout(output) {
   return new Promise(resolve => {
@@ -52,8 +53,8 @@ function writeToStdout(output) {
  * Writes the output to a file.
  * @param {string} filePath
  * @param {string} output
- * @param {string} outputMode
- * @return {!Promise<undefined>}
+ * @param {LH.OutputMode} outputMode
+ * @return {Promise<void>}
  */
 function writeFile(filePath, output, outputMode) {
   return new Promise((resolve, reject) => {
@@ -71,7 +72,7 @@ function writeFile(filePath, output, outputMode) {
 /**
  * Writes the output.
  * @param {string} output
- * @param {string} mode
+ * @param {LH.OutputMode} mode
  * @param {string} path
  * @return {Promise<void>}
  */
@@ -84,7 +85,7 @@ async function write(output, mode, path) {
 
 /**
  * Returns a list of valid output options.
- * @return {!Array<string>}
+ * @return {Array<string>}
  */
 function getValidOutputOptions() {
   return Object.keys(OutputMode);

lighthouse-cli/run.js

@@ -26,7 +26,7 @@ const _PROTOCOL_TIMEOUT_EXIT_CODE = 67;
 /**
  * exported for testing
  * @param {string} flags
- * @return {!Array<string>}
+ * @return {Array<string>}
  */
 function parseChromeFlags(flags = '') {
   const parsed = yargsParser(
@@ -49,8 +49,8 @@ function parseChromeFlags(flags = '') {
 /**
  * Attempts to connect to an instance of Chrome with an open remote-debugging
  * port. If none is found, launches a debuggable instance.
- * @param {!LH.Flags} flags
- * @return {Promise<LH.LaunchedChrome>}
+ * @param {LH.CliFlags} flags
+ * @return {Promise<ChromeLauncher.LaunchedChrome>}
  */
 function getDebuggableChrome(flags) {
   return ChromeLauncher.launch({
@@ -71,7 +71,7 @@ function showProtocolTimeoutError() {
 }
 
 /**
- * @param {!LH.LighthouseError} err
+ * @param {LH.LighthouseError} err
  */
 function showRuntimeError(err) {
   console.error('Runtime error encountered:', err.friendlyMessage || err.message);
@@ -82,7 +82,7 @@ function showRuntimeError(err) {
 }
 
 /**
- * @param {!LH.LighthouseError} err
+ * @param {LH.LighthouseError} err
  */
 function handleError(err) {
   if (err.code === 'ECONNREFUSED') {
@@ -95,8 +95,8 @@ function handleError(err) {
 }
 
 /**
- * @param {!LH.RunnerResult} runnerResult
- * @param {!LH.Flags} flags
+ * @param {LH.RunnerResult} runnerResult
+ * @param {LH.CliFlags} flags
  * @return {Promise<void>}
  */
 async function saveResults(runnerResult, flags) {
@@ -138,12 +138,12 @@ async function saveResults(runnerResult, flags) {
 
 /**
  * @param {string} url
- * @param {LH.Flags} flags
+ * @param {LH.CliFlags} flags
  * @param {LH.Config.Json|undefined} config
  * @return {Promise<LH.RunnerResult|void>}
  */
 function runLighthouse(url, flags, config) {
-  /** @type {!LH.LaunchedChrome} */
+  /** @type {ChromeLauncher.LaunchedChrome|undefined} */
   let launchedChrome;
   const shouldGather = flags.gatherMode || flags.gatherMode === flags.auditMode;
   let chromeP = Promise.resolve();

lighthouse-cli/sentry-prompt.js

@@ -18,7 +18,7 @@ const MESSAGE = `${log.reset}We're constantly trying to improve Lighthouse and i
   ` May we anonymously report runtime exceptions to improve the tool over time? `;
 
 /**
- * @return {!Promise<boolean>}
+ * @return {Promise<boolean>}
  */
 function prompt() {
   if (!process.stdout.isTTY || process.env.CI) {
@@ -58,7 +58,7 @@ function prompt() {
 }
 
 /**
- * @return {!Promise<boolean>}
+ * @return {Promise<boolean>}
  */
 function askPermission() {
   return Promise.resolve().then(_ => {

lighthouse-cli/test/cli/run-test.js

@@ -70,10 +70,6 @@ describe('Parsing --chrome-flags', () => {
     assert.deepStrictEqual(parseChromeFlags('--debug=false'), ['--debug=false']);
   });
 
-  it('returns empty when passed undefined', () => {
-    assert.deepStrictEqual(parseChromeFlags(), []);
-  });
-
   it('keeps --no-flags untouched, #3003', () => {
     assert.deepStrictEqual(parseChromeFlags('--no-sandbox'), ['--no-sandbox']);
   });

lighthouse-core/config/config.js

@@ -160,18 +160,18 @@ function assertValidGatherer(gathererInstance, gathererName) {
 /**
  * Creates a settings object from potential flags object by dropping all the properties
  * that don't exist on Config.Settings.
- * TODO(bckenny): fix Flags type
  * @param {Partial<LH.Flags>=} flags
- * @return {Partial<LH.Config.Settings>}
- */
+ * @return {RecursivePartial<LH.Config.Settings>}
+*/
 function cleanFlagsForSettings(flags = {}) {
-  /** @type {Partial<LH.Config.Settings>} */
+  /** @type {RecursivePartial<LH.Config.Settings>} */
   const settings = {};
 
   for (const key of Object.keys(flags)) {
     // @ts-ignore - intentionally testing some keys not on defaultSettings to discard them.
     if (typeof constants.defaultSettings[key] !== 'undefined') {
-      const safekey = /** @type {keyof LH.SharedFlagsSettings} */ (key);
+      // Cast since key now must be able to index both Flags and Settings.
+      const safekey = /** @type {Extract<keyof LH.Flags, keyof LH.Config.Settings>} */ (key);
       settings[safekey] = flags[safekey];
     }
   }

lighthouse-core/index.js

@@ -31,10 +31,7 @@ const Config = require('./config/config');
  * @param {LH.Config.Json=} configJSON
  * @return {Promise<LH.RunnerResult|undefined>}
  */
-async function lighthouse(url, flags, configJSON) {
-  // TODO(bckenny): figure out Flags types.
-  flags = flags || /** @type {LH.Flags} */ ({});
-
+async function lighthouse(url, flags = {}, configJSON) {
   // set logging preferences, assume quiet
   flags.logLevel = flags.logLevel || 'error';
   log.setLevel(flags.logLevel);

lighthouse-extension/app/src/popup.js

@@ -160,8 +160,8 @@ async function onGenerateReportButtonClick(background, settings) {
   feedbackEl.textContent = '';
 
   const {selectedCategories, useDevTools} = settings;
-  // TODO(bckenny): make flags workable as a type.
-  const flags = /** @type {LH.Flags} */ ({throttlingMethod: useDevTools ? 'devtools' : 'simulate'});
+  /** @type {LH.Flags} */
+  const flags = {throttlingMethod: useDevTools ? 'devtools' : 'simulate'};
 
   try {
     await background.runLighthouseInExtension(flags, selectedCategories);

typings/externs.d.ts

@@ -28,6 +28,18 @@ declare global {
     [P in K]+?: T[P]
   }
 
+  /** An object with the keys in the union K mapped to themselves as values. */
+  type SelfMap<K extends string> = {
+    [P in K]: P;
+  };
+
+  /** Make optional all properties on T and any properties on object properties of T. */
+  type RecursivePartial<T> = {
+    [P in keyof T]+?: T[P] extends object ?
+      RecursivePartial<T[P]> :
+      T[P];
+  };
+
   /**
    * Exclude void from T
    */
@@ -77,16 +89,21 @@ declare global {
       onlyAudits?: string[] | null;
       onlyCategories?: string[] | null;
       skipAudits?: string[] | null;
+      extraHeaders?: Crdp.Network.Headers | null; // See extraHeaders TODO in bin.js
     }
 
     export interface Flags extends SharedFlagsSettings {
-      // Used by both core/ and cli/
-      port: number;
-      hostname: string;
-      output: any;
-      logLevel: 'silent'|'error'|'info'|'verbose';
+      port?: number;
+      hostname?: string;
+      logLevel?: 'silent'|'error'|'info'|'verbose';
+      configPath?: string;
+    }
 
-      // Just used by cli/
+    /**
+     * Flags accepted by Lighthouse, plus additional flags just
+     * for controlling the CLI.
+     */
+    export interface CliFlags extends Flags {
       _: string[];
       chromeFlags: string;
       outputPath: string;
@@ -95,11 +112,13 @@ declare global {
       enableErrorReporting: boolean;
       listAllAudits: boolean;
       listTraceCategories: boolean;
-      configPath?: string;
       preset?: 'full'|'mixed-content'|'perf';
       verbose: boolean;
       quiet: boolean;
-      extraHeaders?: string;
+      // following are given defaults in cli-flags, so not optional like in Flags or SharedFlagsSettings
+      output: OutputMode[];
+      port: number;
+      hostname: string;
     }
 
     export interface RunnerResult {
@@ -120,12 +139,6 @@ declare global {
       group: string;
     }
 
-    export interface LaunchedChrome {
-      pid: number;
-      port: number;
-      kill: () => Promise<{}>;
-    }
-
     export interface LighthouseError extends Error {
       friendlyMessage?: string;
     }