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 17 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
31 changes: 24 additions & 7 deletions lighthouse-cli/cli-flags.js
Expand Up @@ -343,19 +343,28 @@ function getFlags(manualArgv, options = {}) {

// Augmenting yargs type with auto-camelCasing breaks in tsc@4.1.2 and @types/yargs@15.0.11,
// so for now cast to add yarg's camelCase properties to type.
const argv = parser.argv;
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>|undefined}
* @return {Array<string>=}
*/
function splitCommaSeparatedValues(strings) {
if (!strings) return strings;
if (!strings) return undefined;
connorjclark marked this conversation as resolved.
Show resolved Hide resolved

return strings.flatMap(value => value.split(','));
}
Expand All @@ -365,7 +374,9 @@ function splitCommaSeparatedValues(strings) {
* @return {boolean|string|undefined}
*/
function coerceOptionalStringBoolean(value) {
if (typeof value !== 'undefined' && typeof value !== 'string' && typeof value !== 'boolean') {
if (value === undefined) return;

if (typeof value !== 'string' && typeof value !== 'boolean') {
throw new Error('Invalid value: Argument must be a string or a boolean');
}
return value;
Expand Down Expand Up @@ -399,9 +410,11 @@ function coerceOutput(values) {
* allowlist specific locales. Why? So we can support the user who requests 'es-MX' (unsupported)
* and we'll fall back to 'es' (supported).
* @param {unknown} value
* @return {LH.Locale}
* @return {LH.Locale|undefined}
*/
function coerceLocale(value) {
if (value === undefined) return;

if (typeof value !== 'string') throw new Error(`Invalid value: Argument 'locale' must be a string`);
return /** @type {LH.Locale} */ (value);
}
Expand Down Expand Up @@ -431,9 +444,11 @@ function coerceExtraHeaders(value) {
/**
* Take yarg's unchecked object value and ensure it's proper throttling settings.
* @param {unknown} value
* @return {LH.ThrottlingSettings}
* @return {LH.ThrottlingSettings|undefined}
*/
function coerceThrottling(value) {
if (value === undefined) return;

if (!isObjectOfUnknownValues(value)) {
throw new Error(`Invalid value: Argument 'throttling' must be an object, specified per-property ('throttling.rttMs', 'throttling.throughputKbps', etc)`);
}
Expand Down Expand Up @@ -465,9 +480,11 @@ function coerceThrottling(value) {
/**
* Take yarg's unchecked object value and ensure it is a proper LH.screenEmulationSettings.
* @param {unknown} value
* @return {Partial<LH.ScreenEmulationSettings>}
* @return {Partial<LH.ScreenEmulationSettings>|undefined}
*/
function coerceScreenEmulation(value) {
if (value === undefined) return;

if (!isObjectOfUnknownValues(value)) {
throw new Error(`Invalid value: Argument 'screenEmulation' must be an object, specified per-property ('screenEmulation.width', 'screenEmulation.deviceScaleFactor', etc)`);
}
Expand Down
46 changes: 43 additions & 3 deletions lighthouse-cli/test/cli/__snapshots__/cli-flags-test.js.snap
Expand Up @@ -388,6 +388,49 @@ Object {
}
`;

exports[`CLI flags settings are accepted from a file path (inlined budgets) 1`] = `
Object {
"$0": "node_modules/jest/bin/jest.js",
"_": Array [
"http://www.example.com",
],
"budgets": Array [
Object {
"anything": "works",
},
],
"channel": "cli",
"chrome-flags": "",
"chrome-ignore-default-flags": false,
"chromeFlags": "",
"chromeIgnoreDefaultFlags": false,
"cli-flags-path": "__REPLACED__/lighthouse-cli/test/fixtures/cli-flags-path-inline-budgets.json",
"cliFlagsPath": "__REPLACED__/lighthouse-cli/test/fixtures/cli-flags-path-inline-budgets.json",
"enable-error-reporting": undefined,
"enableErrorReporting": undefined,
"fraggle-rock": false,
"fraggleRock": false,
"hostname": "127.0.0.1",
"list-all-audits": false,
"list-locales": false,
"list-trace-categories": false,
"listAllAudits": false,
"listLocales": false,
"listTraceCategories": false,
"output": Array [
"html",
],
"port": 0,
"print-config": false,
"printConfig": false,
"quiet": false,
"save-assets": false,
"saveAssets": false,
"verbose": false,
"view": false,
}
`;

exports[`CLI flags settings are accepted from a file path 1`] = `
Object {
"$0": "node_modules/jest/bin/jest.js",
Expand All @@ -413,9 +456,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
16 changes: 14 additions & 2 deletions lighthouse-cli/test/cli/cli-flags-test.js
Expand Up @@ -46,8 +46,9 @@ describe('CLI flags', function() {
Object.keys(optionGroups).forEach(key => {
allOptions.push(...optionGroups[key]);
});
// @ts-expect-error - getUsageInstance is private
const optionsWithDescriptions = Object.keys(yargs.getUsageInstance().getDescriptions());
const optionsWithDescriptions =
// @ts-expect-error - getUsageInstance is private
Object.keys(yargs.getInternalMethods().getUsageInstance().getDescriptions());

allOptions.forEach(opt => {
assert.ok(optionsWithDescriptions.includes(opt), `cli option '${opt}' has no description`);
Expand Down Expand Up @@ -75,6 +76,17 @@ describe('CLI flags', function() {
snapshot(flags);
});

it('settings are accepted from a file path (inlined budgets)', () => {
const flags = getFlags([
'http://www.example.com',
// eslint-disable-next-line max-len
`--cli-flags-path="${LH_ROOT}/lighthouse-cli/test/fixtures/cli-flags-path-inline-budgets.json"`,
].join(' '));

expect(flags.budgets).toMatchObject([{'anything': 'works'}]);
snapshot(flags);
});

it('array values support csv when appropriate', () => {
const flags = getFlags([
'http://www.example.com',
Expand Down
@@ -0,0 +1,3 @@
{
"budgets": [{"anything": "works"}]
}
3 changes: 2 additions & 1 deletion lighthouse-cli/test/smokehouse/frontends/smokehouse-bin.js
Expand Up @@ -175,7 +175,8 @@ async function begin() {

// Augmenting yargs type with auto-camelCasing breaks in tsc@4.1.2 and @types/yargs@15.0.11,
// so for now cast to add yarg's camelCase properties to type.
const argv = /** @type {typeof rawArgv & CamelCasify<typeof rawArgv>} */ (rawArgv);
const argv =
/** @type {Awaited<typeof rawArgv> & CamelCasify<Awaited<typeof rawArgv>>} */ (rawArgv);

const jobs = Number.isFinite(argv.jobs) ? argv.jobs : undefined;
const retries = Number.isFinite(argv.retries) ? argv.retries : undefined;
Expand Down
17 changes: 17 additions & 0 deletions lighthouse-core/runner.js
Expand Up @@ -259,10 +259,27 @@ class Runner {
output: undefined,
channel: undefined,
budgets: undefined,
skipAudits: undefined,
onlyAudits: undefined,
onlyCategories: undefined,
extraHeaders: undefined,
};
const normalizedGatherSettings = Object.assign({}, artifacts.settings, overrides);
const normalizedAuditSettings = Object.assign({}, settings, overrides);

// First, try each key individually so we can print which key differed.
const keys = new Set([
...Object.keys(normalizedGatherSettings),
...Object.keys(normalizedAuditSettings),
]);
for (const k of keys) {
if (!isDeepEqual(normalizedGatherSettings[k], normalizedAuditSettings[k])) {
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
throw new Error(
`Cannot change settings between gathering and auditing. Difference found at: ${k}`);
}
}

// Call `isDeepEqual` on the entire thing, just in case something was missed.
if (!isDeepEqual(normalizedGatherSettings, normalizedAuditSettings)) {
throw new Error('Cannot change settings between gathering and auditing');
}
Expand Down
3 changes: 2 additions & 1 deletion lighthouse-core/scripts/compare-runs.js
Expand Up @@ -74,7 +74,8 @@ const rawArgv = y

// Augmenting yargs type with auto-camelCasing breaks in tsc@4.1.2 and @types/yargs@15.0.11,
// so for now cast to add yarg's camelCase properties to type.
const argv = /** @type {typeof rawArgv & CamelCasify<typeof rawArgv>} */ (rawArgv);
const argv =
/** @type {Awaited<typeof rawArgv> & CamelCasify<Awaited<typeof rawArgv>>} */ (rawArgv);

const reportExcludeRegex =
argv.reportExclude !== 'none' ? new RegExp(argv.reportExclude, 'i') : null;
Expand Down
3 changes: 2 additions & 1 deletion lighthouse-core/scripts/pptr-run-devtools.js
Expand Up @@ -31,7 +31,7 @@ import yargs from 'yargs';
import * as yargsHelpers from 'yargs/helpers';

const y = yargs(yargsHelpers.hideBin(process.argv));
const argv = y
const argv_ = y
.usage('$0 [url]')
.help('help').alias('help', 'h')
.option('_', {type: 'string'})
Expand All @@ -50,6 +50,7 @@ const argv = y
})
.argv;

const argv = /** @type {Awaited<typeof argv_>} */ (argv_);
/** @type {LH.Config.Json=} */
const config = argv.config ? JSON.parse(argv.config) : undefined;

Expand Down
4 changes: 2 additions & 2 deletions package.json
Expand Up @@ -127,7 +127,7 @@
"@types/tabulator-tables": "^4.9.1",
"@types/update-notifier": "^4.1.0",
"@types/ws": "^7.0.0",
"@types/yargs": "^16.0.0",
"@types/yargs": "^17.0.8",
"@types/yargs-parser": "^15.0.0",
"@typescript-eslint/eslint-plugin": "^5.6.0",
"@typescript-eslint/parser": "^5.6.0",
Expand Down Expand Up @@ -212,7 +212,7 @@
"third-party-web": "^0.12.7",
"update-notifier": "^4.1.0",
"ws": "^7.0.0",
"yargs": "^16.1.1",
"yargs": "^17.3.1",
"yargs-parser": "^20.2.4"
},
"resolutions": {
Expand Down
27 changes: 26 additions & 1 deletion yarn.lock
Expand Up @@ -1741,6 +1741,13 @@
dependencies:
"@types/yargs-parser" "*"

"@types/yargs@^17.0.8":
version "17.0.8"
resolved "https://registry.yarnpkg.com/@types/yargs/-/yargs-17.0.8.tgz#d23a3476fd3da8a0ea44b5494ca7fa677b9dad4c"
integrity sha512-wDeUwiUmem9FzsyysEwRukaEdDNcwbROvQ9QGRKaLI6t+IltNzbn4/i4asmB10auvZGQCzSQ6t0GSczEThlUXw==
dependencies:
"@types/yargs-parser" "*"

"@types/yauzl@^2.9.1":
version "2.9.1"
resolved "https://registry.yarnpkg.com/@types/yauzl/-/yauzl-2.9.1.tgz#d10f69f9f522eef3cf98e30afb684a1e1ec923af"
Expand Down Expand Up @@ -7591,7 +7598,7 @@ string-length@^4.0.1:
char-regex "^1.0.2"
strip-ansi "^6.0.0"

string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0:
string-width@^4.0.0, string-width@^4.1.0, string-width@^4.2.0, string-width@^4.2.3:
version "4.2.3"
resolved "https://registry.yarnpkg.com/string-width/-/string-width-4.2.3.tgz#269c7117d27b05ad2e536830a8ec895ef9c6d010"
integrity sha512-wKyQRQpjJ0sIp62ErSZdGsjMJWsap5oRNihHhu6G7JVO/9jIB6UyevL+tXuOqrng8j/cxKTWyWUwvSTriiZz/g==
Expand Down Expand Up @@ -8486,6 +8493,11 @@ yargs-parser@^20.0.0, yargs-parser@^20.2.2, yargs-parser@^20.2.4:
resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-20.2.4.tgz#b42890f14566796f85ae8e3a25290d205f154a54"
integrity sha512-WOkpgNhPTlE73h4VFAFsOnomJVaovO8VqLDzy5saChRBFQFBoMYirowyW+Q9HB4HFF4Z7VZTiG3iSzJJA29yRA==

yargs-parser@^21.0.0:
version "21.0.0"
resolved "https://registry.yarnpkg.com/yargs-parser/-/yargs-parser-21.0.0.tgz#a485d3966be4317426dd56bdb6a30131b281dc55"
integrity sha512-z9kApYUOCwoeZ78rfRYYWdiU/iNL6mwwYlkkZfJoyMR1xps+NEBX5X7XmRpxkZHhXJ6+Ey00IwKxBBSW9FIjyA==

yargs@^15.0.0:
version "15.4.1"
resolved "https://registry.yarnpkg.com/yargs/-/yargs-15.4.1.tgz#0d87a16de01aee9d8bec2bfbf74f67851730f4f8"
Expand Down Expand Up @@ -8516,6 +8528,19 @@ yargs@^16.0.0, yargs@^16.0.3, yargs@^16.1.1, yargs@^16.2.0:
y18n "^5.0.5"
yargs-parser "^20.2.2"

yargs@^17.3.1:
version "17.3.1"
resolved "https://registry.yarnpkg.com/yargs/-/yargs-17.3.1.tgz#da56b28f32e2fd45aefb402ed9c26f42be4c07b9"
integrity sha512-WUANQeVgjLbNsEmGk20f+nlHgOqzRFpiGWVaBrYGYIGANIIu3lWjoyi0fNlFmJkvfhCZ6BXINe7/W2O2bV4iaA==
dependencies:
cliui "^7.0.2"
escalade "^3.1.1"
get-caller-file "^2.0.5"
require-directory "^2.1.1"
string-width "^4.2.3"
y18n "^5.0.5"
yargs-parser "^21.0.0"

yauzl@^2.10.0:
version "2.10.0"
resolved "https://registry.yarnpkg.com/yauzl/-/yauzl-2.10.0.tgz#c7eb17c93e112cb1086fa6d8e51fb0667b79a5f9"
Expand Down