core(fr): update flow API for 10.0 #14388
Conversation
|
TODO: Docs need updating Rest of this should be reviewable though. |
|
|
||
| /** | ||
| * @param {LH.Artifacts} artifacts | ||
| * @return {string} |
There was a problem hiding this comment.
It's surprising to me that TS agrees this is string instead of string=. I know the switch covers all enumerations of gather mode, but ... javascript 🙃
wdyt of 1) throwing an error in a default case or 2) removing the switch and capitalizing the gather mode to get the desired string?
There was a problem hiding this comment.
I prefer option 1. Option 2 doesn't cover i18n and I don't want to support custom gather modes.
There was a problem hiding this comment.
Wait, how does the current code cover i18n? should this function be a report UIString? (if so, this should be a followup)
There was a problem hiding this comment.
It doesn't support i18n, we should do it in a followup.
clients/devtools/devtools-entry.js
Outdated
| @@ -119,10 +119,16 @@ if (typeof self !== 'undefined') { | |||
| // @ts-expect-error | |||
| self.runLighthouseNavigation = runLighthouseNavigation; | |||
| // @ts-expect-error | |||
| self.runLighthouseNavigation2 = navigation; | |||
There was a problem hiding this comment.
I was actually thinking self.navigation, etc.
There was a problem hiding this comment.
Also @paulirish might be changing this to be just exports soon?
Part of #14049
pageas a top level parameter rather than stuffing it into{page, config, flags}StepFlags extends LH.Flagswhich contains the step name instead of doing{stepName, flags}stepName->name