core(fr): replace configContext with flags
#14050
Conversation
types/externs.d.ts
Outdated
| /** Run the specified plugins. */ | ||
| plugins?: string[]; | ||
| /** If set to true, will skip the initial navigation to about:blank. This option is ignored when using the legacy navigation runner. */ | ||
| skipAboutBlank?: boolean; |
There was a problem hiding this comment.
This is the only option in the FR context that isn't present on the legacy flags. @paulirish I remember when we added this you were concerned about exposing it to the user, but I'm not sure how else to do it without creating another options object for internal stuff.
There was a problem hiding this comment.
All good. I'm fine with it moving here
There was a problem hiding this comment.
if skipAboutBlank is going to be exposed, it should just be a setting (and put on SharedFlagsSettings). It makes more sense as a config-level setting than a module-level one anyways.
That shouldn't change much about the implementation here, though it'll need a default in constants.js, etc. The internalOptions stuff could then be deleted, too.
types/externs.d.ts
Outdated
| /** Run the specified plugins. */ | ||
| plugins?: string[]; | ||
| /** If set to true, will skip the initial navigation to about:blank. This option is ignored when using the legacy navigation runner. */ | ||
| skipAboutBlank?: boolean; |
There was a problem hiding this comment.
All good. I'm fine with it moving here
Without this change, the following PR would break our flags usage: GoogleChrome/lighthouse#14050 Bug: None Change-Id: If5d05982fcd6a4fdd379e2de1406b976338cdf56 Reviewed-on: https://chromium-review.googlesource.com/c/devtools/devtools-frontend/+/3774533 Reviewed-by: Paul Irish <[email protected]> Commit-Queue: Paul Irish <[email protected]>
brendankenny
left a comment
brendankenny
left a comment
There was a problem hiding this comment.
LGTM with a skipAboutBlank type move.
types/externs.d.ts
Outdated
| /** Run the specified plugins. */ | ||
| plugins?: string[]; | ||
| /** If set to true, will skip the initial navigation to about:blank. This option is ignored when using the legacy navigation runner. */ | ||
| skipAboutBlank?: boolean; |
There was a problem hiding this comment.
if skipAboutBlank is going to be exposed, it should just be a setting (and put on SharedFlagsSettings). It makes more sense as a config-level setting than a module-level one anyways.
That shouldn't change much about the implementation here, though it'll need a default in constants.js, etc. The internalOptions stuff could then be deleted, too.
4 participants
First item in #14049