core(fr): add basic timespan support #11944
Conversation
lighthouse-core/runner.js
Outdated
| @@ -26,8 +26,9 @@ const LHError = require('./lib/lh-error.js'); | |||
|
|
|||
| class Runner { | |||
| /** | |||
| * @param {(runnerData: {requestedUrl: string, config: Config, driverMock?: Driver}) => Promise<LH.Artifacts>} gatherFn | |||
| * @param {{config: Config, url?: string, driverMock?: Driver}} runOpts | |||
| * @template {LH.Config.Config | LH.Config.FRConfig} TConfig | |||
There was a problem hiding this comment.
is this just some code golf?
There was a problem hiding this comment.
I wouldn't be very good at it if it were ;)
this is replacing Config type requirement with TConfig which can be either the legacy or new fraggle rock config, it doesn't care which one so long as they match
There was a problem hiding this comment.
from how I understand TS they wouldn't have to match, am I missing something that @template does?
(oh, seems you reverted changes to this file, ok)
There was a problem hiding this comment.
they're not reverted, just part of the filtering changes #11941 that disappeared after I fixed some merge conflicts :)
from how I understand TS they wouldn't have to match, am I missing something that @template does?
I think so :)
In my understanding @template {Bar} TFoo ~= <TFoo extends Bar>. Once a invocation parameter constrains the type of TConfig to FRConfig, the other parameter must also be constrained because they're the same type.
| @@ -5,76 +5,10 @@ | |||
| */ | |||
| 'use strict'; | |||
|
|
|||
| const Driver = require('./gather/driver.js'); | |||
There was a problem hiding this comment.
this is just a move, no logic changes here
| * @param {LH.Gatherer.GatherMode} mode | ||
| * @return {LH.Config.FRConfig['artifacts']} | ||
| */ | ||
| function filterArtifactsByGatherMode(artifacts, mode) { |
There was a problem hiding this comment.
this is new, but covered by #11941 and not really part of this PR
| * @param {LH.Config.FRConfig} config | ||
| * @return {LH.BaseArtifacts} | ||
| */ | ||
| function getBaseArtifacts(config) { |
There was a problem hiding this comment.
this is just moved, nothing special here
| */ | ||
| 'use strict'; | ||
|
|
||
| const Driver = require('./driver.js'); |
There was a problem hiding this comment.
this is just moved, no logic changes here
| * @param {{page: import('puppeteer').Page, config?: LH.Config.Json}} options | ||
| * @return {Promise<{endTimespan(): Promise<LH.RunnerResult|undefined>}>} | ||
| */ | ||
| async function startTimespan(options) { |
There was a problem hiding this comment.
this is new and the bulk of the logic to review
patrickhulce
force-pushed
the
fr_timespan_runner
branch
from
85f9dd3 to
f39985e
Compare
Summary
Adds basic timespan support to the Fraggle Rock API.
api.jsintosnapshot-runner.js/base-artifacts.js.timespan-runner.js.beforeTimespan/afterTimespanmethods.ConsoleMessagesgatherer to support Fraggle Rock (easiest and highest impact timespan-only gatherer).Related Issues/PRs
ref #11313
loosely dependent on #11941 (though could be rebased against master if really necessary)