deps(yargs): upgrade to 17.3.1 #13590
Conversation
|
Looks like we need to handle |
|
ok let's try to get master node lighthouse-cli/index.js https://www.example.com{
_: [ 'https://www.example.com' ],
verbose: false,
quiet: false,
'save-assets': false,
saveAssets: false,
'list-all-audits': false,
listAllAudits: false,
'list-locales': false,
listLocales: false,
'list-trace-categories': false,
listTraceCategories: false,
'print-config': false,
printConfig: false,
'fraggle-rock': false,
fraggleRock: false,
'chrome-flags': '',
chromeFlags: '',
port: 0,
hostname: '127.0.0.1',
'enable-error-reporting': undefined,
enableErrorReporting: undefined,
output: [ 'html' ],
view: false,
channel: 'cli',
'chrome-ignore-default-flags': false,
chromeIgnoreDefaultFlags: false,
'$0': 'lighthouse-cli/index.js'
}new yargs, no changes to cli-flags Invalid value: Argument 'screenEmulation' must be an object, specified per-property ('screenEmulation.width', 'screenEmulation.deviceScaleFactor', etc)
Specify --help for available optionsnew yargs, current PR state {
_: [ 'https://www.example.com' ],
verbose: false,
quiet: false,
'save-assets': false,
saveAssets: false,
'list-all-audits': false,
listAllAudits: false,
'list-locales': false,
listLocales: false,
'list-trace-categories': false,
listTraceCategories: false,
'print-config': false,
printConfig: false,
'fraggle-rock': false,
fraggleRock: false,
'chrome-flags': '',
chromeFlags: '',
port: 0,
hostname: '127.0.0.1',
'enable-error-reporting': undefined,
enableErrorReporting: undefined,
output: [ 'html' ],
view: false,
channel: 'cli',
'chrome-ignore-default-flags': false,
chromeIgnoreDefaultFlags: false,
'$0': 'lighthouse-cli/index.js',
screenEmulation: undefined,
'screen-emulation': undefined,
emulatedUserAgent: undefined,
'emulated-user-agent': undefined,
'gather-mode': undefined,
G: undefined,
gatherMode: undefined,
'audit-mode': undefined,
A: undefined,
auditMode: undefined,
'only-audits': null,
onlyAudits: null,
'only-categories': null,
onlyCategories: null,
'skip-audits': null,
skipAudits: null,
locale: undefined,
throttling: undefined,
'extra-headers': null,
extraHeaders: null,
plugins: null
} |
must have been this yargs/yargs#2063
we should also update our dep on yargs-parser ... we only pass it strings for |
| @@ -413,9 +456,6 @@ Object { | |||
| "extraHeaders": Object { | |||
| "X-Men": "wolverine", | |||
| }, | |||
| "extraheaders": Object { | |||
| "xMen": "wolverine", | |||
| }, | |||
There was a problem hiding this comment.
seems the older yargs was overzealous in how it interprets provided config files.
connorjclark
changed the title
lighthouse-cli/cli-flags.js
Outdated
| // explicitly set to undefined, delete them from the flags object. | ||
| for (const [k, v] of Object.entries(cliFlags)) { | ||
| // This property is meant to possibly be explicitly undefined. | ||
| if (k === 'enable-error-reporting' || k === 'enableErrorReporting') continue; |
There was a problem hiding this comment.
Can you add tests for the special handling of this flag?
There was a problem hiding this comment.
That's the snapshot tests tho. I don't understand the comment in this flag's options anyway, @brendankenny may know.
There was a problem hiding this comment.
Hmm seems like it isn't necessary https://github.com/GoogleChrome/lighthouse/pull/11794/files#r539003671
Fixes #13589
Not sure on the root cause, but I noticed that the bug didn't happen with the latest yargs.
https://github.com/yargs/yargs/blob/main/CHANGELOG.md
Only necessary changes were for:
Which required having coerce fns return
null(orundefined, based on what the settings type expected) to prevent some fields likeskipAudits,extraHeadersfrom beingundefinedinstead ofnull... which was fine, but was an unnecessary change to the settings normalization.