From d28d14c0415e39ca9e66541ebd1328b662b8838e Mon Sep 17 00:00:00 2001
From: Brendan Kenny <bckenny@gmail.com>
Date: Wed, 10 Apr 2019 20:20:21 -0700
Subject: [PATCH 1/2] core: adjustments to stack-pack comments and types

---
 build/build-bundle.js                         |  3 +-
 .../audits/dobetterweb/js-libraries.js        |  6 +-
 .../dobetterweb/no-vulnerable-libraries.js    |  8 +--
 lighthouse-core/gather/gather-runner.js       | 12 +++-
 ...{gatherer-stacks.js => stack-collector.js} | 67 ++++++++++---------
 lighthouse-core/lib/stack-packs.js            | 52 +++++++-------
 lighthouse-core/runner.js                     |  2 +-
 .../audits/dobetterweb/js-libraries-test.js   | 14 ++--
 .../no-vulnerable-libraries-test.js           |  6 +-
 .../artifacts/empty-artifacts/artifacts.json  |  3 +-
 .../fixtures/artifacts/perflog/artifacts.json |  1 +
 .../test/gather/gather-runner-test.js         |  2 +-
 lighthouse-core/test/runner-test.js           |  2 +-
 package.json                                  |  5 +-
 stack-packs/index.js                          |  2 +-
 stack-packs/package.json                      |  3 +-
 stack-packs/stack-packs.d.ts                  |  6 ++
 types/artifacts.d.ts                          | 15 +++--
 types/html-renderer.d.ts                      | 21 +++---
 types/lhr.d.ts                                | 14 ++--
 yarn.lock                                     |  5 ++
 21 files changed, 143 insertions(+), 106 deletions(-)
 rename lighthouse-core/lib/{gatherer-stacks.js => stack-collector.js} (53%)

diff --git a/build/build-bundle.js b/build/build-bundle.js
index 2426210f48f3..ac590e9c2869 100644
--- a/build/build-bundle.js
+++ b/build/build-bundle.js
@@ -53,7 +53,7 @@ async function browserifyFile(entryPath, distPath) {
 
   bundle
     // Transform the fs.readFile etc into inline strings.
-    .transform('brfs', {global: true, parserOpts: {ecmaVersion: 9}})
+    .transform('brfs', {global: true, parserOpts: {ecmaVersion: 10}})
     // Strip everything out of package.json includes except for the version.
     .transform('package-json-versionify');
 
@@ -121,6 +121,7 @@ function minifyScript(filePath) {
     shouldPrintComment: () => false, // Don't include @license or @preserve comments either
     plugins: [
       'syntax-object-rest-spread',
+      'syntax-async-generators',
     ],
     // sourceMaps: 'both'
   };
diff --git a/lighthouse-core/audits/dobetterweb/js-libraries.js b/lighthouse-core/audits/dobetterweb/js-libraries.js
index 8ca0176e4563..2f4d7521f38f 100644
--- a/lighthouse-core/audits/dobetterweb/js-libraries.js
+++ b/lighthouse-core/audits/dobetterweb/js-libraries.js
@@ -30,12 +30,12 @@ class JsLibrariesAudit extends Audit {
    * @return {LH.Audit.Product}
    */
   static audit(artifacts) {
-    const libDetails = (artifacts.Stacks || [])
+    const libDetails = artifacts.Stacks
       .filter(stack => stack.detector === 'js')
       .map(stack => ({
         name: stack.name,
-        version: stack.version || undefined, // null if not detected
-        npm: stack.npm || undefined, // ~70% of libs come with this field
+        version: stack.version,
+        npm: stack.npm,
       }));
 
     /** @type {LH.Audit.Details.Table['headings']} */
diff --git a/lighthouse-core/audits/dobetterweb/no-vulnerable-libraries.js b/lighthouse-core/audits/dobetterweb/no-vulnerable-libraries.js
index 42d5d0a52bc3..fb779308b528 100644
--- a/lighthouse-core/audits/dobetterweb/no-vulnerable-libraries.js
+++ b/lighthouse-core/audits/dobetterweb/no-vulnerable-libraries.js
@@ -60,8 +60,8 @@ class NoVulnerableLibrariesAudit extends Audit {
 
   /**
    * Attempts to normalize the version.
-   * @param {?string} version
-   * @return {?string}
+   * @param {string|undefined} version
+   * @return {string|undefined}
    */
   static normalizeVersion(version) {
     if (!version) return version;
@@ -78,7 +78,7 @@ class NoVulnerableLibrariesAudit extends Audit {
 
   /**
    * @param {string} normalizedVersion
-   * @param {{name: string, version: string, npm?: string}} lib
+   * @param {LH.Artifacts.DetectedStack} lib
    * @param {SnykDB} snykDB
    * @return {Array<Vulnerability>}
    */
@@ -135,7 +135,7 @@ class NoVulnerableLibrariesAudit extends Audit {
    * @return {LH.Audit.Product}
    */
   static audit(artifacts) {
-    const foundLibraries = (artifacts.Stacks || []).filter(stack => stack.detector === 'js');
+    const foundLibraries = artifacts.Stacks.filter(stack => stack.detector === 'js');
     const snykDB = NoVulnerableLibrariesAudit.snykDB;
 
     if (!foundLibraries.length) {
diff --git a/lighthouse-core/gather/gather-runner.js b/lighthouse-core/gather/gather-runner.js
index 4a268c99f595..033a9a8bf317 100644
--- a/lighthouse-core/gather/gather-runner.js
+++ b/lighthouse-core/gather/gather-runner.js
@@ -7,7 +7,7 @@
 
 const log = require('lighthouse-logger');
 const manifestParser = require('../lib/manifest-parser.js');
-const stacksGatherer = require('../lib/gatherer-stacks.js');
+const stacksGatherer = require('../lib/stack-collector.js');
 const LHError = require('../lib/lh-error.js');
 const URL = require('../lib/url-shim.js');
 const NetworkRecorder = require('../lib/network-recorder.js');
@@ -412,7 +412,7 @@ class GatherRunner {
       NetworkUserAgent: '', // updated later
       BenchmarkIndex: 0, // updated later
       WebAppManifest: null, // updated later
-      Stacks: null, // updated later
+      Stacks: [], // updated later
       traces: {},
       devtoolsLogs: {},
       settings: options.settings,
@@ -480,12 +480,18 @@ class GatherRunner {
         }
         await GatherRunner.beforePass(passContext, gathererResults);
         await GatherRunner.pass(passContext, gathererResults);
+
         if (isFirstPass) {
+          // Fetch the manifest, if it exists. Currently must be fetched before gatherers' `afterPass`.
           baseArtifacts.WebAppManifest = await GatherRunner.getWebAppManifest(passContext);
-          baseArtifacts.Stacks = await stacksGatherer(passContext);
         }
+
         const passData = await GatherRunner.afterPass(passContext, gathererResults);
 
+        if (isFirstPass) {
+          baseArtifacts.Stacks = await stacksGatherer(passContext);
+        }
+
         // Save devtoolsLog, but networkRecords are discarded and not added onto artifacts.
         baseArtifacts.devtoolsLogs[passConfig.passName] = passData.devtoolsLog;
 
diff --git a/lighthouse-core/lib/gatherer-stacks.js b/lighthouse-core/lib/stack-collector.js
similarity index 53%
rename from lighthouse-core/lib/gatherer-stacks.js
rename to lighthouse-core/lib/stack-collector.js
index e743158a3ee5..4903bad8f94b 100644
--- a/lighthouse-core/lib/gatherer-stacks.js
+++ b/lighthouse-core/lib/stack-collector.js
@@ -15,45 +15,51 @@
 
 const fs = require('fs');
 const libDetectorSource = fs.readFileSync(
-  require.resolve('js-library-detector/library/libraries.js'),
-  'utf8'
-);
+  require.resolve('js-library-detector/library/libraries.js'), 'utf8');
+
+/**
+ * @typedef JSLibraryDetectorTest
+ * @property {string} icon Essentially an id, useful if no npm name is detected.
+ * @property {string} url
+ * @property {string|null} npm npm module name, if applicable to library.
+ * @property {function(Window): false|{version: string|null} | Promise<false|{version: string|null}>} test Returns false if library is not present, otherwise returns an object that contains the library version (set to null if the version is not detected).
+ */
 
 /**
  * @typedef JSLibrary
  * @property {string} name
- * @property {string} version
- * @property {string} npm
- * @property {string} iconName
+ * @property {string} icon
+ * @property {string|null} version
+ * @property {string|null} npm
  */
 
 /**
  * Obtains a list of detected JS libraries and their versions.
  */
 /* istanbul ignore next */
-function detectLibraries() {
+async function detectLibraries() {
   /** @type {JSLibrary[]} */
   const libraries = [];
 
   // d41d8cd98f00b204e9800998ecf8427e_ is a consistent prefix used by the detect libraries
   // see https://github.com/HTTPArchive/httparchive/issues/77#issuecomment-291320900
+  /** @type {Record<string, JSLibraryDetectorTest>} */
   // @ts-ignore - injected libDetectorSource var
-  Object.entries(d41d8cd98f00b204e9800998ecf8427e_LibraryDetectorTests).forEach(
-    async ([name, lib]) => {
-      // eslint-disable-line max-len
-      try {
-        const result = await lib.test(window);
-        if (result) {
-          libraries.push({
-            name: name,
-            version: result.version,
-            npm: lib.npm,
-            iconName: lib.icon,
-          });
-        }
-      } catch (e) {}
-    }
-  );
+  const libraryDetectorTests = d41d8cd98f00b204e9800998ecf8427e_LibraryDetectorTests; // eslint-disable-line 
+
+  for await (const [name, lib] of Object.entries(libraryDetectorTests)) {
+    try {
+      const result = await lib.test(window);
+      if (result) {
+        libraries.push({
+          name: name,
+          icon: lib.icon,
+          version: result.version,
+          npm: lib.npm,
+        });
+      }
+    } catch (e) {}
+  }
 
   return libraries;
 }
@@ -62,23 +68,22 @@ function detectLibraries() {
  * @param {LH.Gatherer.PassContext} passContext
  * @return {Promise<LH.Artifacts['Stacks']>}
  */
-async function getStacks(passContext) {
+async function collectStacks(passContext) {
   const expression = `(function () {
     ${libDetectorSource};
     return (${detectLibraries.toString()}());
   })()`;
 
-  const jsLibraries = /** @type {JSLibrary[]} */ (await passContext.driver.evaluateAsync(
-    expression
-  ));
+  /** @type {JSLibrary[]} */
+  const jsLibraries = await passContext.driver.evaluateAsync(expression);
 
   return jsLibraries.map(lib => ({
     detector: /** @type {'js'} */ ('js'),
-    id: lib.npm || lib.iconName,
+    id: lib.npm || lib.icon,
     name: lib.name,
-    version: lib.version,
-    npm: lib.npm,
+    version: lib.version || undefined,
+    npm: lib.npm || undefined,
   }));
 }
 
-module.exports = getStacks;
+module.exports = collectStacks;
diff --git a/lighthouse-core/lib/stack-packs.js b/lighthouse-core/lib/stack-packs.js
index ae9d8ea9c786..bdf2b3707862 100644
--- a/lighthouse-core/lib/stack-packs.js
+++ b/lighthouse-core/lib/stack-packs.js
@@ -6,42 +6,48 @@
 'use strict';
 
 const stackPacks = require('@lighthouse/stack-packs');
+const log = require('lighthouse-logger');
 
+/**
+ * Pairs consisting of a stack pack's ID and the set of stacks needed to be
+ * detected in a page to display that pack's advice.
+ * @type {Array<{packId: string, requiredStacks: Array<string>}>}
+ */
 const stackPacksToInclude = [{
   packId: 'wordpress',
   requiredStacks: ['js:wordpress'],
 }];
 
 /**
- * @param {LH.Artifacts} artifacts
+ * Returns all packs that match the stacks found in the page.
+ * @param {LH.Artifacts['Stacks']} pageStacks
  * @return {Array<LH.Result.StackPack>}
  */
-function getStackPacks(artifacts) {
+function getStackPacks(pageStacks) {
   /** @type {Array<LH.Result.StackPack>} */
   const packs = [];
 
-  if (artifacts.Stacks) {
-    for (const pageStack of artifacts.Stacks) {
-      const stackPackToIncl = stackPacksToInclude.find(stackPackToIncl =>
-        stackPackToIncl.requiredStacks.includes(`${pageStack.detector}:${pageStack.id}`));
-      if (!stackPackToIncl) {
-        continue;
-      }
-
-      // Grab the full pack definition
-      const matchedPack = stackPacks.find(pack => pack.id === stackPackToIncl.packId);
-      if (!matchedPack) {
-        // we couldn't find a pack that's in our inclusion list, this is weird.
-        continue;
-      }
-
-      packs.push({
-        id: matchedPack.id,
-        title: matchedPack.title,
-        iconDataURL: matchedPack.iconDataURL,
-        descriptions: matchedPack.descriptions,
-      });
+  for (const pageStack of pageStacks) {
+    const stackPackToIncl = stackPacksToInclude.find(stackPackToIncl =>
+      stackPackToIncl.requiredStacks.includes(`${pageStack.detector}:${pageStack.id}`));
+    if (!stackPackToIncl) {
+      continue;
     }
+
+    // Grab the full pack definition
+    const matchedPack = stackPacks.find(pack => pack.id === stackPackToIncl.packId);
+    if (!matchedPack) {
+      log.warn('StackPacks',
+        `'${stackPackToIncl.packId}' stack pack was matched but is not found in stack-packs lib`);
+      continue;
+    }
+
+    packs.push({
+      id: matchedPack.id,
+      title: matchedPack.title,
+      iconDataURL: matchedPack.iconDataURL,
+      descriptions: matchedPack.descriptions,
+    });
   }
 
   return packs;
diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js
index 8b86e3d60dd4..1cbbf12ce1cd 100644
--- a/lighthouse-core/runner.js
+++ b/lighthouse-core/runner.js
@@ -148,7 +148,7 @@ class Runner {
           rendererFormattedStrings: i18n.getRendererFormattedStrings(settings.locale),
           icuMessagePaths: {},
         },
-        stackPacks: stackPacks.getStackPacks(artifacts),
+        stackPacks: stackPacks.getStackPacks(artifacts.Stacks),
       };
 
       // Replace ICU message references with localized strings; save replaced paths in lhr.
diff --git a/lighthouse-core/test/audits/dobetterweb/js-libraries-test.js b/lighthouse-core/test/audits/dobetterweb/js-libraries-test.js
index 90888c3c8c1d..20583bdf7577 100644
--- a/lighthouse-core/test/audits/dobetterweb/js-libraries-test.js
+++ b/lighthouse-core/test/audits/dobetterweb/js-libraries-test.js
@@ -21,7 +21,7 @@ describe('Returns detected front-end JavaScript libraries', () => {
     const auditResult2 = JsLibrariesAudit.audit({
       Stacks: [
         {detector: 'js', name: 'lib1', version: '3.10.1', npm: 'lib1'},
-        {detector: 'js', name: 'lib2', version: null, npm: 'lib2'},
+        {detector: 'js', name: 'lib2', version: undefined, npm: 'lib2'},
       ],
     });
     assert.equal(auditResult2.rawValue, true);
@@ -29,11 +29,11 @@ describe('Returns detected front-end JavaScript libraries', () => {
     // LOTS of frontend libs
     const auditResult3 = JsLibrariesAudit.audit({
       Stacks: [
-        {detector: 'js', name: 'React', version: null, npm: 'react'},
-        {detector: 'js', name: 'Polymer', version: null, npm: 'polymer-core'},
-        {detector: 'js', name: 'Preact', version: null, npm: 'preact'},
-        {detector: 'js', name: 'Angular', version: null, npm: 'angular'},
-        {detector: 'js', name: 'jQuery', version: null, npm: 'jquery'},
+        {detector: 'js', name: 'React', version: undefined, npm: 'react'},
+        {detector: 'js', name: 'Polymer', version: undefined, npm: 'polymer-core'},
+        {detector: 'js', name: 'Preact', version: undefined, npm: 'preact'},
+        {detector: 'js', name: 'Angular', version: undefined, npm: 'angular'},
+        {detector: 'js', name: 'jQuery', version: undefined, npm: 'jquery'},
       ],
     });
     assert.equal(auditResult3.rawValue, true);
@@ -43,7 +43,7 @@ describe('Returns detected front-end JavaScript libraries', () => {
     const auditResult = JsLibrariesAudit.audit({
       Stacks: [
         {detector: 'js', name: 'lib1', version: '3.10.1', npm: 'lib1'},
-        {detector: 'js', name: 'lib2', version: null, npm: 'lib2'},
+        {detector: 'js', name: 'lib2', version: undefined, npm: 'lib2'},
       ],
     });
     const expected = [
diff --git a/lighthouse-core/test/audits/dobetterweb/no-vulnerable-libraries-test.js b/lighthouse-core/test/audits/dobetterweb/no-vulnerable-libraries-test.js
index 7bb58ce13b87..7580648d2fa5 100644
--- a/lighthouse-core/test/audits/dobetterweb/no-vulnerable-libraries-test.js
+++ b/lighthouse-core/test/audits/dobetterweb/no-vulnerable-libraries-test.js
@@ -13,7 +13,7 @@ const assert = require('assert');
 describe('Avoids front-end JavaScript libraries with known vulnerabilities', () => {
   describe('#normalizeVersion', () => {
     it('should leave valid and unsavable versions untouched', () => {
-      assert.equal(NoVulnerableLibrariesAudit.normalizeVersion(null), null);
+      assert.equal(NoVulnerableLibrariesAudit.normalizeVersion(undefined), undefined);
       assert.equal(NoVulnerableLibrariesAudit.normalizeVersion('52.1.13'), '52.1.13');
       assert.equal(NoVulnerableLibrariesAudit.normalizeVersion('52.1.13-rc.1'), '52.1.13-rc.1');
       assert.equal(NoVulnerableLibrariesAudit.normalizeVersion('c0ab71056b936'), 'c0ab71056b936');
@@ -31,7 +31,7 @@ describe('Avoids front-end JavaScript libraries with known vulnerabilities', ()
       Stacks: [
         {detector: 'js', name: 'lib1', version: '1.0.0', npm: 'lib1'},
         {detector: 'js', name: 'angular', version: '1.1.4', npm: 'angular'},
-        {detector: 'js', name: 'lib3', version: null, npm: 'lib3'},
+        {detector: 'js', name: 'lib3', version: undefined, npm: 'lib3'},
       ],
     });
     assert.equal(auditResult.rawValue, false);
@@ -89,7 +89,7 @@ Array [
     const auditResult = NoVulnerableLibrariesAudit.audit({
       Stacks: [
         {detector: 'js', name: 'lib1', version: '3.10.1', npm: 'lib1'},
-        {detector: 'js', name: 'lib2', version: null, npm: 'lib2'},
+        {detector: 'js', name: 'lib2', version: undefined, npm: 'lib2'},
       ],
     });
     assert.equal(auditResult.rawValue, true);
diff --git a/lighthouse-core/test/fixtures/artifacts/empty-artifacts/artifacts.json b/lighthouse-core/test/fixtures/artifacts/empty-artifacts/artifacts.json
index 00d0ee379892..c80f8395dab8 100644
--- a/lighthouse-core/test/fixtures/artifacts/empty-artifacts/artifacts.json
+++ b/lighthouse-core/test/fixtures/artifacts/empty-artifacts/artifacts.json
@@ -2,5 +2,6 @@
   "URL": {
     "requestedUrl": "https://example.com/",
     "finalUrl": "https://example.com/"
-  }
+  },
+  "Stacks": []
 }
diff --git a/lighthouse-core/test/fixtures/artifacts/perflog/artifacts.json b/lighthouse-core/test/fixtures/artifacts/perflog/artifacts.json
index 2a08553da892..219b2cf783b2 100644
--- a/lighthouse-core/test/fixtures/artifacts/perflog/artifacts.json
+++ b/lighthouse-core/test/fixtures/artifacts/perflog/artifacts.json
@@ -9,6 +9,7 @@
     "requestedUrl": "https://www.reddit.com/r/nba",
     "finalUrl": "https://www.reddit.com/r/nba"
   },
+  "Stacks": [],
   "MetaElements": [],
   "ViewportDimensions": {
     "innerWidth": 412,
diff --git a/lighthouse-core/test/gather/gather-runner-test.js b/lighthouse-core/test/gather/gather-runner-test.js
index 252af3a02cc7..684d33fe8d05 100644
--- a/lighthouse-core/test/gather/gather-runner-test.js
+++ b/lighthouse-core/test/gather/gather-runner-test.js
@@ -14,7 +14,7 @@ const Config = require('../../config/config');
 const unresolvedPerfLog = require('./../fixtures/unresolved-perflog.json');
 const NetworkRequest = require('../../lib/network-request.js');
 
-jest.mock('../../lib/gatherer-stacks.js', () => () => Promise.resolve([]));
+jest.mock('../../lib/stack-collector.js', () => () => Promise.resolve([]));
 
 class TestGatherer extends Gatherer {
   constructor() {
diff --git a/lighthouse-core/test/runner-test.js b/lighthouse-core/test/runner-test.js
index 72edb833eb0c..0a49a79dcc22 100644
--- a/lighthouse-core/test/runner-test.js
+++ b/lighthouse-core/test/runner-test.js
@@ -20,7 +20,7 @@ const LHError = require('../lib/lh-error.js');
 
 /* eslint-env jest */
 
-jest.mock('../lib/gatherer-stacks.js', () => () => Promise.resolve([]));
+jest.mock('../lib/stack-collector.js', () => () => Promise.resolve([]));
 
 describe('Runner', () => {
   /** @type {jest.Mock} */
diff --git a/package.json b/package.json
index 0a8f1fc8fa3a..fc8485f48038 100644
--- a/package.json
+++ b/package.json
@@ -93,6 +93,7 @@
     "angular": "^1.7.4",
     "archiver": "^3.0.0",
     "babel-core": "^6.26.0",
+    "babel-plugin-syntax-async-generators": "^6.13.0",
     "babel-plugin-syntax-object-rest-spread": "^6.13.0",
     "brfs": "^1.6.1",
     "browserify": "^16.2.3",
@@ -130,6 +131,7 @@
     "zone.js": "^0.7.3"
   },
   "dependencies": {
+    "@lighthouse/stack-packs": "file:./stack-packs",
     "axe-core": "3.1.2",
     "chrome-launcher": "^0.10.5",
     "configstore": "^3.1.1",
@@ -158,8 +160,7 @@
     "update-notifier": "^2.5.0",
     "ws": "3.3.2",
     "yargs": "3.32.0",
-    "yargs-parser": "7.0.0",
-    "@lighthouse/stack-packs": "file:./stack-packs"
+    "yargs-parser": "7.0.0"
   },
   "resolutions": {
     "chrome-launcher/@types/node": "*"
diff --git a/stack-packs/index.js b/stack-packs/index.js
index 38509bec1827..a8da0cd10ece 100644
--- a/stack-packs/index.js
+++ b/stack-packs/index.js
@@ -6,7 +6,7 @@
 
 'use strict';
 
-/** @type {import('./stack-packs').Pack[]} */
+/** @type {typeof import('./stack-packs')} */
 const stackPacks = [
   require('./packs/wordpress.js'),
 ];
diff --git a/stack-packs/package.json b/stack-packs/package.json
index e60682c216c6..a397db221d47 100644
--- a/stack-packs/package.json
+++ b/stack-packs/package.json
@@ -7,7 +7,8 @@
   "keywords": [
     "google",
     "chrome",
-    "devtools"
+    "devtools",
+    "lighthouse"
   ],
   "author": "The Chromium Authors",
   "license": "Apache-2.0",
diff --git a/stack-packs/stack-packs.d.ts b/stack-packs/stack-packs.d.ts
index 51d70596f00b..7d99e5f07fd3 100644
--- a/stack-packs/stack-packs.d.ts
+++ b/stack-packs/stack-packs.d.ts
@@ -1,3 +1,9 @@
+/**
+ * @license Copyright 2019 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.
+ */
+
 interface Pack {
   id: string;
   iconDataURL: string;
diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts
index 424e995f2a69..5f743b89104b 100644
--- a/types/artifacts.d.ts
+++ b/types/artifacts.d.ts
@@ -34,7 +34,7 @@ declare global {
       /** Parsed version of the page's Web App Manifest, or null if none found. */
       WebAppManifest: Artifacts.Manifest | null;
       /** Information on detected tech stacks (e.g. JS libraries) used by the page. */
-      Stacks: Artifacts.DetectedStack[] | null;
+      Stacks: Artifacts.DetectedStack[];
       /** A set of page-load traces, keyed by passName. */
       traces: {[passName: string]: Trace};
       /** A set of DevTools debugger protocol records, keyed by passName. */
@@ -447,16 +447,17 @@ declare global {
         fmpFellBack: boolean;
       }
 
+      /** Information on a tech stack (e.g. a JS library) used by the page. */
       export interface DetectedStack {
-        /** The identifier on how this stack got detected */
+        /** The identifier for how this stack was detected. */
         detector: 'js';
-        /** The unique name of the stack stripped of special characters */
+        /** The unique string ID for the stack. */
         id: string;
-        /** The name of the stack */
+        /** The name of the stack. */
         name: string;
-        /** The version of the stack we found */
-        version: string;
-        /** The package name on NPM if it exists */
+        /** The version of the stack, if it could be detected. */
+        version?: string;
+        /** The package name on NPM, if it exists. */
         npm?: string;
       }
     }
diff --git a/types/html-renderer.d.ts b/types/html-renderer.d.ts
index 4641421bfca8..de49d00d12d2 100644
--- a/types/html-renderer.d.ts
+++ b/types/html-renderer.d.ts
@@ -45,15 +45,6 @@ declare global {
     prepareLabData: typeof _prepareLabData;
   }
 
-  interface StackPackRef {
-    /** Title of the stackpack, used as an alt text */
-    title: string;
-    /** Base64 url of the current stackpack */
-    iconDataURL: string;
-    /** Description of the current audit of the stackpack */
-    description: string;
-  }
-
   module LH {
     // During report generation, the LHR object is transformed a bit for convenience
     // Primarily, the auditResult is added as .result onto the auditRef.
@@ -66,8 +57,18 @@ declare global {
       export interface Category extends Result.Category {
         auditRefs: Array<AuditRef>
       }
+
       export interface AuditRef extends Result.AuditRef {
-        result: Audit.Result & { stackPacks?: StackPackRef[] }
+        result: Audit.Result & { stackPacks?: StackPackDescription[] }
+      }
+
+      export interface StackPackDescription {
+         /** The title of the stack pack. */
+        title: string;
+        /** A base64 data url to be used as the stack pack's icon. */
+        iconDataURL: string;
+        /** The stack-specific description for this audit. */
+        description: string;
       }
     }
   }
diff --git a/types/lhr.d.ts b/types/lhr.d.ts
index 6948d0f862de..288565172c44 100644
--- a/types/lhr.d.ts
+++ b/types/lhr.d.ts
@@ -60,7 +60,7 @@ declare global {
       timing: Result.Timing;
       /** The record of all formatted string locations in the LHR and their corresponding source values. */
       i18n: {rendererFormattedStrings: I18NRendererStrings, icuMessagePaths: I18NMessages};
-      /** An array containing the result of all stackpacks */
+      /** An array containing the result of all stack packs. */
       stackPacks: Result.StackPack[];
     }
 
@@ -103,16 +103,18 @@ declare global {
       }
 
       /**
-       * A description of a stack pack
+       * A pack of secondary audit descriptions to be used when a page uses a
+       * specific technology stack, giving stack-specific advice for some of
+       * Lighthouse's audits.
        */
       export interface StackPack {
-        /** The unique stackpack's name stripped from special characters */
+        /** The unique string ID for this stack pack. */
         id: string;
-        /** The name of the stack pack which we display on our report */
+        /** The title of the stack pack, to be displayed in the report. */
         title: string;
-        /** The base64 url of the icon  */
+        /** A base64 data url to be used as the stack pack's icon. */
         iconDataURL: string;
-        /** An object containing the descriptions of the audits, keyed by the audits' `id` identifier. */
+        /** A set of descriptions for some of Lighthouse's audits, keyed by the audits' `id`. */
         descriptions: Record<string, string>;
       }
     }
diff --git a/yarn.lock b/yarn.lock
index 1f680283db5f..363e9e303e9c 100644
--- a/yarn.lock
+++ b/yarn.lock
@@ -1292,6 +1292,11 @@ babel-plugin-jest-hoist@^24.3.0:
   dependencies:
     "@types/babel__traverse" "^7.0.6"
 
+babel-plugin-syntax-async-generators@^6.13.0:
+  version "6.13.0"
+  resolved "https://registry.yarnpkg.com/babel-plugin-syntax-async-generators/-/babel-plugin-syntax-async-generators-6.13.0.tgz#6bc963ebb16eccbae6b92b596eb7f35c342a8b9a"
+  integrity sha1-a8lj67FuzLrmuStZbrfzXDQqi5o=
+
 babel-plugin-syntax-object-rest-spread@^6.13.0:
   version "6.13.0"
   resolved "https://registry.yarnpkg.com/babel-plugin-syntax-object-rest-spread/-/babel-plugin-syntax-object-rest-spread-6.13.0.tgz#fd6536f2bce13836ffa3a5458c4903a597bb3bf5"

From da14c5257b696b124732f24d0c508c0e51372589 Mon Sep 17 00:00:00 2001
From: Brendan Kenny <bckenny@gmail.com>
Date: Thu, 11 Apr 2019 13:10:52 -0700
Subject: [PATCH 2/2] feedback

---
 lighthouse-core/lib/stack-collector.js | 5 +++--
 types/lhr.d.ts                         | 2 +-
 2 files changed, 4 insertions(+), 3 deletions(-)

diff --git a/lighthouse-core/lib/stack-collector.js b/lighthouse-core/lib/stack-collector.js
index 4903bad8f94b..984d8b1dca60 100644
--- a/lighthouse-core/lib/stack-collector.js
+++ b/lighthouse-core/lib/stack-collector.js
@@ -17,12 +17,13 @@ const fs = require('fs');
 const libDetectorSource = fs.readFileSync(
   require.resolve('js-library-detector/library/libraries.js'), 'utf8');
 
+/** @typedef {false | {version: string|null}} JSLibraryDetectorTestResult */
 /**
  * @typedef JSLibraryDetectorTest
  * @property {string} icon Essentially an id, useful if no npm name is detected.
  * @property {string} url
  * @property {string|null} npm npm module name, if applicable to library.
- * @property {function(Window): false|{version: string|null} | Promise<false|{version: string|null}>} test Returns false if library is not present, otherwise returns an object that contains the library version (set to null if the version is not detected).
+ * @property {function(Window): JSLibraryDetectorTestResult | Promise<JSLibraryDetectorTestResult>} test Returns false if library is not present, otherwise returns an object that contains the library version (set to null if the version is not detected).
  */
 
 /**
@@ -47,7 +48,7 @@ async function detectLibraries() {
   // @ts-ignore - injected libDetectorSource var
   const libraryDetectorTests = d41d8cd98f00b204e9800998ecf8427e_LibraryDetectorTests; // eslint-disable-line 
 
-  for await (const [name, lib] of Object.entries(libraryDetectorTests)) {
+  for (const [name, lib] of Object.entries(libraryDetectorTests)) {
     try {
       const result = await lib.test(window);
       if (result) {
diff --git a/types/lhr.d.ts b/types/lhr.d.ts
index 288565172c44..f78312b5c292 100644
--- a/types/lhr.d.ts
+++ b/types/lhr.d.ts
@@ -114,7 +114,7 @@ declare global {
         title: string;
         /** A base64 data url to be used as the stack pack's icon. */
         iconDataURL: string;
-        /** A set of descriptions for some of Lighthouse's audits, keyed by the audits' `id`. */
+        /** A set of descriptions for some of Lighthouse's audits, keyed by audit `id`. */
         descriptions: Record<string, string>;
       }
     }