extension: access core through module instead of Runner #5855
Conversation
| * @return {Promise<LH.RunnerResult|undefined>} | ||
| */ | ||
| async function lighthouse(url, flags = {}, configJSON) { | ||
| async function lighthouse(url, flags = {}, configJSON, connection) { |
There was a problem hiding this comment.
if we feel like this is going to become a never-ending list of optional arguments, we could put connection inside an object (overrides? or...something).
OTOH we don't want to go back to flags, settings, and options objects passed into every function :)
There was a problem hiding this comment.
4️⃣. 0️⃣
ha. Just to be clear, ambiguous smiley face, this should be fine since it's just an addition to our API, not a change, right?
There was a problem hiding this comment.
oh, well since all of them are optional and you didn't like the idea of flags + options (I mostly agree), I thought you meant collect all of the arguments into a single argument options object which I like for 4.0
in the interim, I see no huge win from making connection an object with just connection since we'd like to make as much configurable from the config as possible, so this seems fine as-is to me
| @@ -131,6 +131,9 @@ gulp.task('browserify-lighthouse', () => { | |||
| .ignore('rimraf') | |||
| .ignore('pako/lib/zlib/inflate.js'); | |||
|
|
|||
| // Don't include the desktop protocol connection. | |||
| bundle.ignore(require.resolve('../lighthouse-core/gather/connections/cri.js')); | |||
| let runnerResult; | ||
| try { | ||
| runnerResult = await lighthouse(url, flags, config, connection); | ||
| } finally { |
|
this is so great. |
Makes the extension (and devtools/psi) access lighthouse-core through the module interface instead of directly calling
Runner.run(). With the last few simplifications to flags/settings/config, this refactor is now really simple and means we have a single core interface for all our platforms.Works by allowing a
Connectionto be passed intolighthouse()instead of always using a desktopChromeProtocolconnection. This is something we have previously discussed with the puppeteer team to make #4403 easy, so it seems like a good move.@paulirish I think this will also ease other refactors you've talked about for all the
runLighthouseIn*methods