Repository: GoogleChrome/lighthouse
Pull request #12622 (2021-06-04 19:43:23)

core(fr): more precise AnyFRInterface types
**Summary**
Fixes the issue raised in https://github.com/GoogleChrome/lighthouse/issues/12619

The type violations were in fact valid and stemmed from my abuse of `Interface<AllPossibleKeys>` to be a substitute for `Interface<Key1> | Interface<Key2> | Interface<Key3>`. I introduced `AnyInterface` types to more accurately reflect when a function accepts or returns *any possible* artifact/gatherer/dependency object instead. This still has the limitation that it doesn't programmatically generate the combinatorial versions, but I haven't bumped into a scenario where the handling differs/matters yet. 

Solves @adamraine 's original point (see screenshot)

![image](https://user-images.githubusercontent.com/2301202/120854423-c6dfc800-c542-11eb-9866-73d766964017.png)

**Related Issues/PRs**
fixes https://github.com/GoogleChrome/lighthouse/issues/12619
ref #11313 



Patch:
diff --git a/lighthouse-core/fraggle-rock/config/config.js b/lighthouse-core/fraggle-rock/config/config.js
index feabb311d..db07e6a28 100644
--- a/lighthouse-core/fraggle-rock/config/config.js
+++ b/lighthouse-core/fraggle-rock/config/config.js
@@ -57,9 +57,9 @@ function resolveWorkingCopy(configJSON, context) {
  * Looks up the required artifact IDs for each dependency, throwing if no earlier artifact satisfies the dependency.
  *
  * @param {LH.Config.ArtifactJson} artifact
- * @param {LH.Config.FRGathererDefn} gatherer
- * @param {Map<Symbol, LH.Config.ArtifactDefn>} artifactDefnsBySymbol
- * @return {LH.Config.ArtifactDefn['dependencies']}
+ * @param {LH.Config.AnyFRGathererDefn} gatherer
+ * @param {Map<Symbol, LH.Config.AnyArtifactDefn>} artifactDefnsBySymbol
+ * @return {LH.Config.AnyArtifactDefn['dependencies']}
  */
 function resolveArtifactDependencies(artifact, gatherer, artifactDefnsBySymbol) {
   if (!('dependencies' in gatherer.instance.meta)) return undefined;
@@ -86,7 +86,7 @@ function resolveArtifactDependencies(artifact, gatherer, artifactDefnsBySymbol)
  *
  * @param {LH.Config.ArtifactJson[]|null|undefined} artifacts
  * @param {string|undefined} configDir
- * @return {LH.Config.ArtifactDefn[] | null}
+ * @return {LH.Config.AnyArtifactDefn[] | null}
  */
 function resolveArtifactsToDefns(artifacts, configDir) {
   if (!artifacts) return null;
@@ -94,7 +94,7 @@ function resolveArtifactsToDefns(artifacts, configDir) {
   const status = {msg: 'Resolve artifact definitions', id: 'lh:config:resolveArtifactsToDefns'};
   log.time(status, 'verbose');
 
-  /** @type {Map<Symbol, LH.Config.ArtifactDefn>} */
+  /** @type {Map<Symbol, LH.Config.AnyArtifactDefn>} */
   const artifactDefnsBySymbol = new Map();
 
   const coreGathererList = Runner.getGathererList();
@@ -108,7 +108,9 @@ function resolveArtifactsToDefns(artifacts, configDir) {
       throw new Error(`${gatherer.instance.name} gatherer does not support Fraggle Rock`);
     }
 
-    /** @type {LH.Config.ArtifactDefn<LH.Gatherer.DependencyKey>} */
+    /** @type {LH.Config.AnyArtifactDefn} */
+    // @ts-expect-error - Typescript can't validate the gatherer and dependencies match
+    // even though it knows that they're each valid on their own.
     const artifact = {
       id: artifactJson.id,
       gatherer,
@@ -127,7 +129,7 @@ function resolveArtifactsToDefns(artifacts, configDir) {
 /**
  *
  * @param {LH.Config.NavigationJson[]|null|undefined} navigations
- * @param {LH.Config.ArtifactDefn[]|null|undefined} artifactDefns
+ * @param {LH.Config.AnyArtifactDefn[]|null|undefined} artifactDefns
  * @return {LH.Config.NavigationDefn[] | null}
  */
 function resolveNavigationsToDefns(navigations, artifactDefns) {
diff --git a/lighthouse-core/fraggle-rock/config/default-config.js b/lighthouse-core/fraggle-rock/config/default-config.js
index ef55b08ca..c5a601593 100644
--- a/lighthouse-core/fraggle-rock/config/default-config.js
+++ b/lighthouse-core/fraggle-rock/config/default-config.js
@@ -40,6 +40,7 @@ const artifacts = {
   PasswordInputsWithPreventedPaste: '',
   ResponseCompression: '',
   RobotsTxt: '',
+  ScriptElements: '',
   SourceMaps: '',
   Stacks: '',
   TagsBlockingFirstPaint: '',
@@ -91,6 +92,7 @@ const defaultConfig = {
     {id: artifacts.PasswordInputsWithPreventedPaste, gatherer: 'dobetterweb/password-inputs-with-prevented-paste'},
     {id: artifacts.ResponseCompression, gatherer: 'dobetterweb/response-compression'},
     {id: artifacts.RobotsTxt, gatherer: 'seo/robots-txt'},
+    {id: artifacts.ScriptElements, gatherer: 'script-elements'},
     {id: artifacts.SourceMaps, gatherer: 'source-maps'},
     {id: artifacts.Stacks, gatherer: 'stacks'},
     {id: artifacts.TagsBlockingFirstPaint, gatherer: 'dobetterweb/tags-blocking-first-paint'},
@@ -144,6 +146,7 @@ const defaultConfig = {
         artifacts.PasswordInputsWithPreventedPaste,
         artifacts.ResponseCompression,
         artifacts.RobotsTxt,
+        artifacts.ScriptElements,
         artifacts.SourceMaps,
         artifacts.Stacks,
         artifacts.TagsBlockingFirstPaint,
diff --git a/lighthouse-core/fraggle-rock/config/filters.js b/lighthouse-core/fraggle-rock/config/filters.js
index 7e88699dd..40d9c1b9d 100644
--- a/lighthouse-core/fraggle-rock/config/filters.js
+++ b/lighthouse-core/fraggle-rock/config/filters.js
@@ -38,7 +38,7 @@ function filterArtifactsByGatherMode(artifacts, mode) {
  * Filters an array of audits down to the set that can be computed using only the specified artifacts.
  *
  * @param {LH.Config.FRConfig['audits']} audits
- * @param {Array<LH.Config.ArtifactDefn>} availableArtifacts
+ * @param {Array<LH.Config.AnyArtifactDefn>} availableArtifacts
  * @return {LH.Config.FRConfig['audits']}
  */
 function filterAuditsByAvailableArtifacts(audits, availableArtifacts) {
diff --git a/lighthouse-core/fraggle-rock/config/validation.js b/lighthouse-core/fraggle-rock/config/validation.js
index 91b4e4fdc..5d284eda6 100644
--- a/lighthouse-core/fraggle-rock/config/validation.js
+++ b/lighthouse-core/fraggle-rock/config/validation.js
@@ -6,8 +6,8 @@
 'use strict';
 
 /**
- * @param {LH.Config.GathererDefn | LH.Config.FRGathererDefn} gathererDefn
- * @return {gathererDefn is LH.Config.FRGathererDefn}
+ * @param {LH.Config.GathererDefn | LH.Config.AnyFRGathererDefn} gathererDefn
+ * @return {gathererDefn is LH.Config.AnyFRGathererDefn}
  */
 function isFRGathererDefn(gathererDefn) {
   return 'meta' in gathererDefn.instance;
@@ -17,8 +17,8 @@ function isFRGathererDefn(gathererDefn) {
  * Determines if the artifact dependency direction is valid. The dependency's minimum supported mode
  * must be less than or equal to the dependent's.
  *
- * @param {LH.Config.FRGathererDefn} dependent The artifact that depends on the other.
- * @param {LH.Config.FRGathererDefn} dependency The artifact that is being depended on by the other.
+ * @param {LH.Config.AnyFRGathererDefn} dependent The artifact that depends on the other.
+ * @param {LH.Config.AnyFRGathererDefn} dependency The artifact that is being depended on by the other.
  * @return {boolean}
  */
 function isValidArtifactDependency(dependent, dependency) {
diff --git a/lighthouse-core/fraggle-rock/gather/runner-helpers.js b/lighthouse-core/fraggle-rock/gather/runner-helpers.js
index 07fd927c0..2f5986dad 100644
--- a/lighthouse-core/fraggle-rock/gather/runner-helpers.js
+++ b/lighthouse-core/fraggle-rock/gather/runner-helpers.js
@@ -8,7 +8,7 @@
 /**
  * @typedef CollectPhaseArtifactOptions
  * @property {import('./driver.js')} driver
- * @property {Array<LH.Config.ArtifactDefn>} artifactDefinitions
+ * @property {Array<LH.Config.AnyArtifactDefn>} artifactDefinitions
  * @property {ArtifactState} artifactState
  * @property {LH.Gatherer.FRGatherPhase} phase
  * @property {LH.Gatherer.GatherMode} gatherMode
@@ -99,7 +99,7 @@ async function collectPhaseArtifacts(options) {
 }
 
 /**
- * @param {LH.Config.ArtifactDefn} artifact
+ * @param {LH.Config.AnyArtifactDefn} artifact
  * @param {Record<string, LH.Gatherer.PhaseResult>} artifactsById
  * @return {Promise<Dependencies>}
  */
diff --git a/lighthouse-core/gather/gatherers/script-elements.js b/lighthouse-core/gather/gatherers/script-elements.js
index a373f40c7..f399cfe4e 100644
--- a/lighthouse-core/gather/gatherers/script-elements.js
+++ b/lighthouse-core/gather/gatherers/script-elements.js
@@ -5,10 +5,14 @@
  */
 'use strict';
 
-const Gatherer = require('./gatherer.js');
+const FRGatherer = require('../../fraggle-rock/gather/base-gatherer.js');
+const NetworkRecords = require('../../computed/network-records.js');
 const NetworkAnalyzer = require('../../lib/dependency-graph/simulator/network-analyzer.js');
 const NetworkRequest = require('../../lib/network-request.js');
 const pageFunctions = require('../../lib/page-functions.js');
+const {fetchResponseBodyFromCache} = require('../driver/network.js');
+const DevtoolsLog = require('./devtools-log.js');
+const HostFormFactor = require('./host-form-factor.js');
 
 /* global getNodeDetails */
 
@@ -62,17 +66,25 @@ async function runInSeriesOrParallel(values, promiseMapper, runInSeries) {
 /**
  * @fileoverview Gets JavaScript file contents.
  */
-class ScriptElements extends Gatherer {
+class ScriptElements extends FRGatherer {
+  /** @type {LH.Gatherer.GathererMeta<'DevtoolsLog'|'HostFormFactor'>} */
+  meta = {
+    supportedModes: ['timespan', 'navigation'],
+    dependencies: {DevtoolsLog: DevtoolsLog.symbol, HostFormFactor: HostFormFactor.symbol},
+  }
+
   /**
-   * @param {LH.Gatherer.PassContext} passContext
-   * @param {LH.Gatherer.LoadData} loadData
+   * @param {LH.Gatherer.FRTransitionalContext} context
+   * @param {LH.Artifacts.NetworkRequest[]} networkRecords
+   * @param {LH.Artifacts['HostFormFactor']} formFactor
    * @return {Promise<LH.Artifacts['ScriptElements']>}
    */
-  async afterPass(passContext, loadData) {
-    const driver = passContext.driver;
-    const mainResource = NetworkAnalyzer.findMainDocument(loadData.networkRecords, passContext.url);
+  async _getArtifact(context, networkRecords, formFactor) {
+    const session = context.driver.defaultSession;
+    const executionContext = context.driver.executionContext;
+    const mainResource = NetworkAnalyzer.findMainDocument(networkRecords, context.url);
 
-    const scripts = await driver.executionContext.evaluate(collectAllScriptElements, {
+    const scripts = await executionContext.evaluate(collectAllScriptElements, {
       args: [],
       useIsolation: true,
       deps: [
@@ -85,7 +97,7 @@ class ScriptElements extends Gatherer {
       if (script.content) script.requestId = mainResource.requestId;
     }
 
-    const scriptRecords = loadData.networkRecords
+    const scriptRecords = networkRecords
       // Ignore records from OOPIFs
       .filter(record => !record.sessionId)
       // Only get the content of script requests
@@ -95,8 +107,9 @@ class ScriptElements extends Gatherer {
     // record at a time.
     const scriptRecordContents = await runInSeriesOrParallel(
       scriptRecords,
-      record => driver.getRequestContent(record.requestId).catch(() => ''),
-      passContext.baseArtifacts.HostFormFactor === 'mobile' /* runInSeries*/ );
+      record => fetchResponseBodyFromCache(session, record.requestId).catch(() => ''),
+      formFactor === 'mobile' /* runInSeries */
+    );
 
     for (let i = 0; i < scriptRecords.length; i++) {
       const record = scriptRecords[i];
@@ -123,6 +136,26 @@ class ScriptElements extends Gatherer {
     }
     return scripts;
   }
+
+  /**
+   * @param {LH.Gatherer.FRTransitionalContext<'DevtoolsLog'|'HostFormFactor'>} context
+   */
+  async getArtifact(context) {
+    const devtoolsLog = context.dependencies.DevtoolsLog;
+    const formFactor = context.dependencies.HostFormFactor;
+    const networkRecords = await NetworkRecords.request(devtoolsLog, context);
+    return this._getArtifact(context, networkRecords, formFactor);
+  }
+
+  /**
+   * @param {LH.Gatherer.PassContext} passContext
+   * @param {LH.Gatherer.LoadData} loadData
+   */
+  async afterPass(passContext, loadData) {
+    const networkRecords = loadData.networkRecords;
+    const formFactor = passContext.baseArtifacts.HostFormFactor;
+    return this._getArtifact({...passContext, dependencies: {}}, networkRecords, formFactor);
+  }
 }
 
 module.exports = ScriptElements;
diff --git a/types/artifacts.d.ts b/types/artifacts.d.ts
index 09fc5ba0e..230a1fc6c 100644
--- a/types/artifacts.d.ts
+++ b/types/artifacts.d.ts
@@ -24,7 +24,6 @@ declare global {
       | 'HTTPRedirect'
       | 'Manifest'
       | 'MixedContent'
-      | 'ScriptElements'
       | 'ServiceWorker'
       | keyof FRBaseArtifacts
     >;
diff --git a/types/config.d.ts b/types/config.d.ts
index 6814557f4..d5038979c 100644
--- a/types/config.d.ts
+++ b/types/config.d.ts
@@ -49,7 +49,7 @@ declare global {
        */
       export interface FRConfig {
         settings: Settings;
-        artifacts: ArtifactDefn[] | null;
+        artifacts: AnyArtifactDefn[] | null;
         navigations: NavigationDefn[] | null;
         audits: AuditDefn[] | null;
         categories: Record<string, Category> | null;
@@ -160,7 +160,7 @@ declare global {
       }
 
       export interface NavigationDefn extends Omit<Required<NavigationJson>, 'artifacts'> {
-        artifacts: ArtifactDefn[];
+        artifacts: AnyArtifactDefn[];
       }
 
       export interface ArtifactDefn<TDependencies extends Gatherer.DependencyKey = Gatherer.DependencyKey> {
@@ -171,12 +171,26 @@ declare global {
           Record<Exclude<TDependencies, Gatherer.DefaultDependenciesKey>, {id: string}>;
       }
 
+      type ArtifactDefnExpander<TDependencies extends Gatherer.DependencyKey> =
+        // Lack of brackets intentional here to convert to the union of all individual dependencies.
+        TDependencies extends Gatherer.DefaultDependenciesKey ?
+          ArtifactDefn<Gatherer.DefaultDependenciesKey> :
+          ArtifactDefn<TDependencies>
+      export type AnyArtifactDefn = ArtifactDefnExpander<Gatherer.DefaultDependenciesKey>|ArtifactDefnExpander<Gatherer.DependencyKey>
+
       export interface FRGathererDefn<TDependencies extends Gatherer.DependencyKey = Gatherer.DependencyKey> {
         implementation?: ClassOf<Gatherer.FRGathererInstance<TDependencies>>;
         instance: Gatherer.FRGathererInstance<TDependencies>;
         path?: string;
       }
 
+      type FRGathererDefnExpander<TDependencies extends Gatherer.DependencyKey> =
+        // Lack of brackets intentional here to convert to the union of all individual dependencies.
+        TDependencies extends Gatherer.DefaultDependenciesKey ?
+          FRGathererDefn<Gatherer.DefaultDependenciesKey> :
+          FRGathererDefn<TDependencies>
+      export type AnyFRGathererDefn = FRGathererDefnExpander<Gatherer.DefaultDependenciesKey>|FRGathererDefnExpander<Gatherer.DependencyKey>
+
       export interface GathererDefn {
         implementation?: ClassOf<Gatherer.GathererInstance>;
         instance: Gatherer.GathererInstance;
diff --git a/types/gatherer.d.ts b/types/gatherer.d.ts
index 382a49960..f6e3a316c 100644
--- a/types/gatherer.d.ts
+++ b/types/gatherer.d.ts
@@ -87,7 +87,7 @@ declare global {
     }
 
     interface GathererMetaWithDependencies<
-      TDependencies extends DependencyKey = DefaultDependenciesKey
+      TDependencies extends Exclude<DependencyKey, DefaultDependenciesKey>
     > extends GathererMetaNoDependencies {
       /**
        * The set of required dependencies that this gatherer needs before it can compute its results.
@@ -96,9 +96,9 @@ declare global {
     }
 
     export type GathererMeta<TDependencies extends DependencyKey = DefaultDependenciesKey> =
-      TDependencies extends DefaultDependenciesKey ?
+      [TDependencies] extends [DefaultDependenciesKey] ?
         GathererMetaNoDependencies :
-        GathererMetaWithDependencies<TDependencies>;
+        GathererMetaWithDependencies<Exclude<TDependencies, DefaultDependenciesKey>>;
 
     export interface GathererInstance {
       name: keyof LH.GathererArtifacts;
@@ -114,11 +114,18 @@ declare global {
       meta: GathererMeta<TDependencies>;
       startInstrumentation(context: FRTransitionalContext<DefaultDependenciesKey>): Promise<void>|void;
       startSensitiveInstrumentation(context: FRTransitionalContext<DefaultDependenciesKey>): Promise<void>|void;
-      stopSensitiveInstrumentation(context: FRTransitionalContext<TDependencies>): Promise<void>|void;
-      stopInstrumentation(context: FRTransitionalContext<TDependencies>): Promise<void>|void;
+      stopSensitiveInstrumentation(context: FRTransitionalContext<DefaultDependenciesKey>): Promise<void>|void;
+      stopInstrumentation(context: FRTransitionalContext<DefaultDependenciesKey>): Promise<void>|void;
       getArtifact(context: FRTransitionalContext<TDependencies>): PhaseResult;
     }
 
+    type FRGathererInstanceExpander<TDependencies extends Gatherer.DependencyKey> =
+      // Lack of brackets intentional here to convert to the union of all individual dependencies.
+      TDependencies extends Gatherer.DefaultDependenciesKey ?
+        FRGathererInstance<Gatherer.DefaultDependenciesKey> :
+        FRGathererInstance<Exclude<TDependencies, DefaultDependenciesKey>>
+    export type AnyFRGathererInstance = FRGathererInstanceExpander<Gatherer.DependencyKey>
+
     namespace Simulation {
       export type GraphNode = import('../lighthouse-core/lib/dependency-graph/base-node').Node;
       export type GraphNetworkNode = _NetworkNode;


Test patch:
diff --git a/lighthouse-core/test/fraggle-rock/api-test-pptr.js b/lighthouse-core/test/fraggle-rock/api-test-pptr.js
index ddcad87fd..7a6f08ba1 100644
--- a/lighthouse-core/test/fraggle-rock/api-test-pptr.js
+++ b/lighthouse-core/test/fraggle-rock/api-test-pptr.js
@@ -121,7 +121,7 @@ describe('Fraggle Rock API', () => {
 
       const {auditResults, erroredAudits, failedAudits} = getAuditsBreakdown(lhr);
       // TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached.
-      expect(auditResults.length).toMatchInlineSnapshot(`35`);
+      expect(auditResults.length).toMatchInlineSnapshot(`36`);
 
       expect(erroredAudits).toHaveLength(0);
       expect(failedAudits.map(audit => audit.id)).toContain('errors-in-console');
@@ -159,7 +159,7 @@ describe('Fraggle Rock API', () => {
       const {lhr} = result;
       const {auditResults, failedAudits, erroredAudits} = getAuditsBreakdown(lhr);
       // TODO(FR-COMPAT): This assertion can be removed when full compatibility is reached.
-      expect(auditResults.length).toMatchInlineSnapshot(`145`);
+      expect(auditResults.length).toMatchInlineSnapshot(`151`);
       expect(erroredAudits).toHaveLength(1); // FIXME: MainDocumentContent broken from merge
 
       const failedAuditIds = failedAudits.map(audit => audit.id);
diff --git a/lighthouse-core/test/fraggle-rock/config/config-test.js b/lighthouse-core/test/fraggle-rock/config/config-test.js
index 4d4437657..9e3af077f 100644
--- a/lighthouse-core/test/fraggle-rock/config/config-test.js
+++ b/lighthouse-core/test/fraggle-rock/config/config-test.js
@@ -63,8 +63,8 @@ describe('Fraggle Rock Config', () => {
   });
 
   it('should throw on invalid artifact definitions', () => {
-    const configJson = {artifacts: [{id: 'ScriptElements', gatherer: 'script-elements'}]};
-    expect(() => initializeConfig(configJson, {gatherMode})).toThrow(/ScriptElements gatherer/);
+    const configJson = {artifacts: [{id: 'ServiceWorker', gatherer: 'service-worker'}]};
+    expect(() => initializeConfig(configJson, {gatherMode})).toThrow(/ServiceWorker gatherer/);
   });
 
   it('should resolve navigation definitions', () => {
diff --git a/lighthouse-core/test/fraggle-rock/gather/mock-driver.js b/lighthouse-core/test/fraggle-rock/gather/mock-driver.js
index faa9aaf51..333f94535 100644
--- a/lighthouse-core/test/fraggle-rock/gather/mock-driver.js
+++ b/lighthouse-core/test/fraggle-rock/gather/mock-driver.js
@@ -40,7 +40,7 @@ function createMockSession() {
 }
 
 /**
- * @param {LH.Gatherer.FRGathererInstance<LH.Gatherer.DependencyKey>['meta']} meta
+ * @param {LH.Gatherer.AnyFRGathererInstance['meta']} meta
  */
 function createMockGathererInstance(meta) {
   return {
@@ -51,7 +51,7 @@ function createMockGathererInstance(meta) {
     stopSensitiveInstrumentation: jest.fn(),
     getArtifact: jest.fn(),
 
-    /** @return {LH.Gatherer.FRGathererInstance} */
+    /** @return {LH.Gatherer.AnyFRGathererInstance} */
     asGatherer() {
       // @ts-expect-error - We'll rely on the tests passing to know this matches.
       return this;
diff --git a/lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js b/lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js
index 93002fb1d..4a4ef5f54 100644
--- a/lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js
+++ b/lighthouse-core/test/fraggle-rock/gather/navigation-runner-test.js
@@ -31,7 +31,7 @@ describe('NavigationRunner', () => {
   /** @type {Map<string, LH.ArbitraryEqualityMap>} */
   let computedCache;
 
-  /** @return {LH.Config.FRGathererDefn} */
+  /** @return {LH.Config.AnyFRGathererDefn} */
   function createGathererDefn() {
     return {
       instance: {
diff --git a/lighthouse-core/test/fraggle-rock/gather/runner-helpers-test.js b/lighthouse-core/test/fraggle-rock/gather/runner-helpers-test.js
index 3feaf1f38..ea14353d8 100644
--- a/lighthouse-core/test/fraggle-rock/gather/runner-helpers-test.js
+++ b/lighthouse-core/test/fraggle-rock/gather/runner-helpers-test.js
@@ -13,15 +13,19 @@ const {createMockDriver, createMockGathererInstance} = require('./mock-driver.js
 /* eslint-env jest */
 
 describe('collectArtifactDependencies', () => {
-  /** @type {LH.Config.ArtifactDefn} */
+  /** @type {LH.Config.AnyArtifactDefn} */
   let artifact;
   /** @type {Record<string, any>} */
   let artifactStateById;
 
   beforeEach(() => {
+    class GathererWithDependency extends Gatherer {
+      meta = {...new Gatherer().meta, dependencies: {ImageElements: Symbol('')}};
+    }
+
     artifact = {
       id: 'Artifact',
-      gatherer: {instance: new Gatherer()},
+      gatherer: {instance: new GathererWithDependency()},
       dependencies: {ImageElements: {id: 'Dependency'}},
     };
     artifactStateById = {
@@ -163,10 +167,19 @@ describe('collectPhaseArtifacts', () => {
 
   it('should pass dependencies to gatherers', async () => {
     const {artifactDefinitions: artifacts_, gatherers} = createGathererSet();
-    const gatherer = artifacts_[1].gatherer;
+    const gatherer =
+      /** @type {{instance: LH.Gatherer.FRGathererInstance}} */
+      (artifacts_[1].gatherer);
+    const imageElementsGatherer =
+      /** @type {{instance: LH.Gatherer.FRGathererInstance<'ImageElements'>}} */
+      (artifacts_[1].gatherer);
     const artifactDefinitions = [
       {id: 'Dependency', gatherer},
-      {id: 'Snapshot', gatherer, dependencies: {ImageElements: {id: 'Dependency'}}},
+      {
+        id: 'Snapshot',
+        gatherer: imageElementsGatherer,
+        dependencies: {ImageElements: {id: 'Dependency'}},
+      },
     ];
 
     await helpers.collectPhaseArtifacts({
diff --git a/lighthouse-core/test/gather/gatherers/script-elements-test.js b/lighthouse-core/test/gather/gatherers/script-elements-test.js
new file mode 100644
index 000000000..973b6435d
--- /dev/null
+++ b/lighthouse-core/test/gather/gatherers/script-elements-test.js
@@ -0,0 +1,169 @@
+/**
+ * @license Copyright 2021 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';
+
+/* eslint-env jest */
+
+const {
+  createMockContext,
+  mockDriverSubmodules,
+} = require('../../fraggle-rock/gather/mock-driver.js');
+const mocks = mockDriverSubmodules();
+const ScriptElements = require('../../../gather/gatherers/script-elements.js');
+const NetworkRequest = require('../../../lib/network-request.js');
+
+/**
+ * @param {Partial<LH.Artifacts.NetworkRequest>=} partial
+ * @return {LH.Artifacts.NetworkRequest}
+ */
+function mockRecord(partial) {
+  const request = new NetworkRequest();
+  request.resourceType = NetworkRequest.TYPES.Script;
+  return Object.assign(request, partial);
+}
+
+/**
+ * @param {Partial<LH.Artifacts.ScriptElement>=} partial
+ * @return {LH.Artifacts.ScriptElement}
+ */
+function mockElement(partial) {
+  return {
+    type: null,
+    src: null,
+    id: null,
+    async: false,
+    defer: false,
+    source: 'head',
+    content: null,
+    requestId: null,
+    node: null,
+    ...partial,
+  };
+}
+
+describe('_getArtifact', () => {
+  let mockContext = createMockContext();
+  /** @type {ScriptElements} */
+  let gatherer;
+  /** @type {LH.Artifacts.ScriptElement[]} */
+  let scriptElements;
+  /** @type {LH.Artifacts.NetworkRequest[]} */
+  let networkRecords;
+  /** @type {Record<string, string>} */
+  let scriptRecordContents;
+  /** @type {LH.Artifacts.NetworkRequest} */
+  let mainDocument;
+
+  beforeEach(() => {
+    mocks.reset();
+    mockContext = createMockContext();
+    gatherer = new ScriptElements();
+    scriptElements = [];
+    mainDocument = mockRecord({resourceType: NetworkRequest.TYPES.Document, requestId: '0'});
+    networkRecords = [mainDocument];
+    scriptRecordContents = {};
+    mockContext.driver._executionContext.evaluate.mockImplementation(() => scriptElements);
+    mocks.networkMock.fetchResponseBodyFromCache
+      .mockImplementation((_, id) => Promise.resolve(scriptRecordContents[id]));
+  });
+
+  it('collects script elements', async () => {
+    networkRecords = [
+      mainDocument,
+      mockRecord({url: 'https://example.com/script.js', requestId: '1'}),
+    ];
+    scriptRecordContents = {
+      '1': '// SOURCED',
+    };
+    scriptElements = [
+      mockElement({src: 'https://example.com/script.js'}),
+      mockElement({content: '// INLINE'}),
+    ];
+
+    const artifact = await gatherer._getArtifact(mockContext.asContext(), networkRecords, 'mobile');
+
+    expect(artifact).toEqual([
+      mockElement({src: 'https://example.com/script.js', requestId: '1', content: '// SOURCED'}),
+      mockElement({content: '// INLINE', requestId: '0'}),
+    ]);
+  });
+
+  it('ignore OOPIF records', async () => {
+    networkRecords = [
+      mainDocument,
+      mockRecord({url: 'https://example.com/script.js', requestId: '1'}),
+      mockRecord({url: 'https://oopif.com/script.js', requestId: '2', sessionId: 'OOPIF'}),
+    ];
+    scriptRecordContents = {
+      '1': '// SOURCED',
+      '2': '// OOPIF',
+    };
+    // OOPIF would not produce script element
+    scriptElements = [
+      mockElement({src: 'https://example.com/script.js'}),
+      mockElement({content: '// INLINE'}),
+    ];
+
+    const artifact = await gatherer._getArtifact(mockContext.asContext(), networkRecords, 'mobile');
+
+    expect(artifact).toEqual([
+      mockElement({src: 'https://example.com/script.js', requestId: '1', content: '// SOURCED'}),
+      mockElement({content: '// INLINE', requestId: '0'}),
+    ]);
+  });
+
+  it('null content for sourced script with empty content', async () => {
+    networkRecords = [
+      mainDocument,
+      mockRecord({url: 'https://example.com/empty.js', requestId: '1'}),
+    ];
+    scriptRecordContents = {
+      '1': '',
+    };
+    scriptElements = [
+      mockElement({src: 'https://example.com/empty.js'}),
+    ];
+
+    const artifact = await gatherer._getArtifact(mockContext.asContext(), networkRecords, 'mobile');
+
+    expect(artifact).toEqual([
+      mockElement({src: 'https://example.com/empty.js'}),
+    ]);
+  });
+
+  it('handle erroneous network content', async () => {
+    networkRecords = [
+      mainDocument,
+      mockRecord({url: 'https://example.com/script.js', requestId: '1'}),
+    ];
+    mocks.networkMock.fetchResponseBodyFromCache.mockRejectedValue('Error');
+    scriptElements = [
+      mockElement({src: 'https://example.com/script.js'}),
+    ];
+
+    const artifact = await gatherer._getArtifact(mockContext.asContext(), networkRecords, 'mobile');
+
+    expect(artifact).toEqual([
+      mockElement({src: 'https://example.com/script.js'}),
+    ]);
+  });
+
+  it('create element if none found', async () => {
+    networkRecords = [
+      mainDocument,
+      mockRecord({url: 'https://example.com/script.js', requestId: '1'}),
+    ];
+    scriptRecordContents = {
+      '1': '// SOURCED',
+    };
+
+    const artifact = await gatherer._getArtifact(mockContext.asContext(), networkRecords, 'mobile');
+
+    expect(artifact).toEqual([
+      mockElement({src: 'https://example.com/script.js', requestId: '1', content: '// SOURCED', source: 'network'}),
+    ]);
+  });
+});