core(config): remove gatherer options support

lighthouse-core/config/config.js

@@ -16,7 +16,7 @@ const path = require('path');
 const Runner = require('../runner.js');
 const ConfigPlugin = require('./config-plugin.js');
 const Budget = require('./budget.js');
-const {requireAudits, mergeOptionsOfItems, resolveModule} = require('./config-helpers.js');
+const {requireAudits, resolveModule} = require('./config-helpers.js');
 
 /** @typedef {typeof import('../gather/gatherers/gatherer.js')} GathererConstructor */
 /** @typedef {InstanceType<GathererConstructor>} Gatherer */
@@ -374,10 +374,6 @@ class Config {
           gathererDefn.implementation = undefined;
           // @ts-expect-error Breaking the Config.GathererDefn type.
           gathererDefn.instance = undefined;
-          if (Object.keys(gathererDefn.options).length === 0) {
-            // @ts-expect-error Breaking the Config.GathererDefn type.
-            gathererDefn.options = undefined;
-          }
         }
       }
     }
@@ -741,12 +737,11 @@ class Config {
 
   /**
    * @param {string} path
-   * @param {{}|undefined} options
    * @param {Array<string>} coreAuditList
    * @param {string=} configDir
    * @return {LH.Config.GathererDefn}
    */
-  static requireGathererFromPath(path, options, coreAuditList, configDir) {
+  static requireGathererFromPath(path, coreAuditList, configDir) {
     const coreGatherer = coreAuditList.find(a => a === `${path}.js`);
 
     let requirePath = `../gather/gatherers/${path}`;
@@ -761,7 +756,6 @@ class Config {
       instance: new GathererClass(),
       implementation: GathererClass,
       path,
-      options: options || {},
     };
   }
 
@@ -788,29 +782,29 @@ class Config {
             instance: gathererDefn.instance,
             implementation: gathererDefn.implementation,
             path: gathererDefn.path,
-            options: gathererDefn.options || {},
           };
         } else if (gathererDefn.implementation) {
           const GathererClass = gathererDefn.implementation;
           return {
             instance: new GathererClass(),
             implementation: gathererDefn.implementation,
             path: gathererDefn.path,
-            options: gathererDefn.options || {},
           };
         } else if (gathererDefn.path) {
           const path = gathererDefn.path;
-          const options = gathererDefn.options;
-          return Config.requireGathererFromPath(path, options, coreList, configDir);
+          return Config.requireGathererFromPath(path, coreList, configDir);
         } else {
           throw new Error('Invalid expanded Gatherer: ' + JSON.stringify(gathererDefn));
         }
       });
 
-      const mergedDefns = mergeOptionsOfItems(gathererDefns);
-      mergedDefns.forEach(gatherer => assertValidGatherer(gatherer.instance, gatherer.path));
+      // De-dupe gatherers by artifact name because artifact IDs must be unique at runtime.
+      const uniqueDefns = Array.from(
+        new Map(gathererDefns.map(defn => [defn.instance.name, defn])).values()
+      );
+      uniqueDefns.forEach(gatherer => assertValidGatherer(gatherer.instance, gatherer.path));
 
-      return Object.assign(pass, {gatherers: mergedDefns});
+      return Object.assign(pass, {gatherers: uniqueDefns});
     });
     log.timeEnd(status);
     return fullPasses;

lighthouse-core/gather/gather-runner.js

@@ -382,8 +382,6 @@ class GatherRunner {
 
     for (const gathererDefn of passContext.passConfig.gatherers) {
       const gatherer = gathererDefn.instance;
-      // Abuse the passContext to pass through gatherer options
-      passContext.options = gathererDefn.options || {};
       const status = {
         msg: `Gathering setup: ${gatherer.name}`,
         id: `lh:gather:beforePass:${gatherer.name}`,
@@ -412,8 +410,6 @@ class GatherRunner {
 
     for (const gathererDefn of gatherers) {
       const gatherer = gathererDefn.instance;
-      // Abuse the passContext to pass through gatherer options
-      passContext.options = gathererDefn.options || {};
       const status = {
         msg: `Gathering in-page: ${gatherer.name}`,
         id: `lh:gather:pass:${gatherer.name}`,
@@ -457,8 +453,6 @@ class GatherRunner {
       };
       log.time(status);
 
-      // Add gatherer options to the passContext.
-      passContext.options = gathererDefn.options || {};
       const artifactPromise = Promise.resolve()
         .then(_ => gatherer.afterPass(passContext, loadData));
 

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

@@ -64,6 +64,11 @@ describe('Config', () => {
         super();
         this.secret = secretVal;
       }
+
+      get name() {
+        // Use unique artifact name per instance so gatherers aren't deduplicated.
+        return `MyGatherer${this.secret}`;
+      }
     }
     const myGatherer1 = new MyGatherer(1729);
     const myGatherer2 = new MyGatherer(6);
@@ -1206,19 +1211,18 @@ describe('Config', () => {
   });
 
   describe('#requireGatherers', () => {
-    it('should merge gatherers', () => {
+    it('should deduplicate gatherers', () => {
       const gatherers = [
         'viewport-dimensions',
-        {path: 'viewport-dimensions', options: {x: 1}},
-        {path: 'viewport-dimensions', options: {y: 1}},
+        {path: 'viewport-dimensions'},
       ];
 
       const merged = Config.requireGatherers([{gatherers}]);
       // Round-trip through JSON to drop live 'instance'/'implementation' props.
       const mergedJson = JSON.parse(JSON.stringify(merged));
 
       assert.deepEqual(mergedJson[0].gatherers,
-        [{path: 'viewport-dimensions', options: {x: 1, y: 1}, instance: {}}]);
+        [{path: 'viewport-dimensions', instance: {}}]);
     });
 
     function loadGatherer(gathererEntry) {
@@ -1325,16 +1329,14 @@ describe('Config', () => {
   });
 
   describe('#getPrintString', () => {
-    it('doesn\'t include empty gatherer/audit options in output', () => {
-      const gOpt = 'gathererOption';
+    it('doesn\'t include empty audit options in output', () => {
       const aOpt = 'auditOption';
       const configJson = {
         extends: 'lighthouse:default',
         passes: [{
           passName: 'defaultPass',
           gatherers: [
-            // `options` merged into default `script-elements` gatherer.
-            {path: 'script-elements', options: {gOpt}},
+            {path: 'script-elements'},
           ],
         }],
         audits: [
@@ -1347,20 +1349,9 @@ describe('Config', () => {
       const printedConfig = JSON.parse(printed);
 
       // Check that options weren't completely eliminated.
-      const scriptsGatherer = printedConfig.passes[0].gatherers
-        .find(g => g.path === 'script-elements');
-      assert.strictEqual(scriptsGatherer.options.gOpt, gOpt);
       const metricsAudit = printedConfig.audits.find(a => a.path === 'metrics');
       assert.strictEqual(metricsAudit.options.aOpt, aOpt);
 
-      for (const pass of printedConfig.passes) {
-        for (const gatherer of pass.gatherers) {
-          if (gatherer.options) {
-            assert.ok(Object.keys(gatherer.options).length > 0);
-          }
-        }
-      }
-
       for (const audit of printedConfig.audits) {
         if (audit.options) {
           assert.ok(Object.keys(audit.options).length > 0);

lighthouse-core/test/gather/gather-runner-test.js

@@ -1464,64 +1464,6 @@ describe('GatherRunner', function() {
       });
     });
 
-    it('passes gatherer options', async () => {
-      /** @type {Record<string, any[]>} */
-      const calls = {beforePass: [], pass: [], afterPass: []};
-      /** @param {string} name */
-      const makeEavesdropGatherer = name => {
-        const C = class extends Gatherer {};
-        Object.defineProperty(C, 'name', {value: name});
-        return Object.assign(new C, {
-          /** @param {LH.Gatherer.PassContext} context */
-          beforePass(context) {
-            calls.beforePass.push(context.options);
-          },
-          /** @param {LH.Gatherer.PassContext} context */
-          pass(context) {
-            calls.pass.push(context.options);
-          },
-          /** @param {LH.Gatherer.PassContext} context */
-          afterPass(context) {
-            calls.afterPass.push(context.options);
-            // @ts-expect-error
-            return context.options.x || 'none';
-          },
-        });
-      };
-
-      const gatherers = [
-        {instance: makeEavesdropGatherer('EavesdropGatherer1'), options: {x: 1}},
-        {instance: makeEavesdropGatherer('EavesdropGatherer2'), options: {x: 2}},
-        {instance: makeEavesdropGatherer('EavesdropGatherer3')},
-      ];
-
-      const config = makeConfig({
-        passes: [{
-          passName: 'defaultPass',
-          gatherers,
-        }],
-      });
-
-      /** @type {any} Using Test-only gatherers. */
-      const artifacts = await GatherRunner.run(config.passes, {
-        driver: fakeDriver,
-        requestedUrl: 'https://example.com',
-        settings: config.settings,
-      });
-
-      assert.equal(artifacts.EavesdropGatherer1, 1);
-      assert.equal(artifacts.EavesdropGatherer2, 2);
-      assert.equal(artifacts.EavesdropGatherer3, 'none');
-
-      // assert that all three phases received the gatherer options expected
-      const expectedOptions = [{x: 1}, {x: 2}, {}];
-      for (let i = 0; i < 3; i++) {
-        assert.deepEqual(calls.beforePass[i], expectedOptions[i]);
-        assert.deepEqual(calls.pass[i], expectedOptions[i]);
-        assert.deepEqual(calls.afterPass[i], expectedOptions[i]);
-      }
-    });
-
     it('uses the last not-undefined phase result as artifact', async () => {
       const recoverableError = new Error('My recoverable error');
       const someOtherError = new Error('Bad, bad error.');

types/config.d.ts

@@ -90,7 +90,6 @@ declare global {
         implementation?: typeof Gatherer;
         instance: InstanceType<typeof Gatherer>;
         path?: string;
-        options: {};
       }
 
       export interface AuditDefn {

types/gatherer.d.ts

@@ -39,7 +39,6 @@ declare global {
       disableJavaScript?: boolean;
       passConfig: Config.Pass
       settings: Config.Settings;
-      options?: object;
       /** Gatherers can push to this array to add top-level warnings to the LHR. */
       LighthouseRunWarnings: Array<string | IcuMessage>;
       baseArtifacts: BaseArtifacts;