Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

deps(yargs): upgrade to 17.3.1 #13590

Merged
merged 21 commits into from
Jan 26, 2022
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
update
  • Loading branch information
connorjclark committed Jan 25, 2022
commit 90a07d11cf1795c150ef301cad6b704bdaa44347
15 changes: 12 additions & 3 deletions lighthouse-cli/cli-flags.js
Original file line number Diff line number Diff line change
Expand Up @@ -346,16 +346,25 @@ function getFlags(manualArgv, options = {}) {
const argv = /** @type {Awaited<typeof parser.argv>} */ (parser.argv);
const cliFlags = /** @type {typeof argv & CamelCasify<typeof argv>} */ (argv);

// yargs will return `undefined` for options that have a `coerce` function but
// are not actually present in the user input. Instead of passing properties
// 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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add tests for the special handling of this flag?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That's the snapshot tests tho. I don't understand the comment in this flag's options anyway, @brendankenny may know.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

if (v === undefined) delete cliFlags[k];
}

return cliFlags;
}

/**
* Support comma-separated values for some array flags by splitting on any ',' found.
* @param {Array<string>=} strings
* @return {Array<string>|null}
* @return {Array<string>=}
*/
function splitCommaSeparatedValues(strings) {
if (!strings) return null;
if (!strings) return undefined;

return strings.flatMap(value => value.split(','));
}
Expand Down Expand Up @@ -418,7 +427,7 @@ function coerceLocale(value) {
*/
function coerceExtraHeaders(value) {
// TODO: this function does not actually verify the object type.
if (value === undefined) return null;
if (value === undefined) return value;
if (typeof value === 'object') return /** @type {LH.SharedFlagsSettings['extraHeaders']} */ (value);
if (typeof value !== 'string') {
throw new Error(`Invalid value: Argument 'extra-headers' must be a string`);
Expand Down
3 changes: 0 additions & 3 deletions lighthouse-cli/test/cli/__snapshots__/cli-flags-test.js.snap
Original file line number Diff line number Diff line change
Expand Up @@ -413,9 +413,6 @@ Object {
"extraHeaders": Object {
"X-Men": "wolverine",
},
"extraheaders": Object {
"xMen": "wolverine",
},
Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems the older yargs was overzealous in how it interprets provided config files.

"fraggle-rock": false,
"fraggleRock": false,
"hostname": "127.0.0.1",
Expand Down