From ca4fcad5d3a8dbcaa010bdadb690219e689b1dff Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Mon, 7 Jan 2019 17:49:54 -0800 Subject: [PATCH] misc: clean up local names within Config (#6950) --- lighthouse-core/config/config.js | 70 ++++++++++++++++++---- lighthouse-core/runner.js | 46 -------------- lighthouse-core/test/config/config-test.js | 2 +- 3 files changed, 59 insertions(+), 59 deletions(-) diff --git a/lighthouse-core/config/config.js b/lighthouse-core/config/config.js index b7e4b4a41d8c..54cf15322dfb 100644 --- a/lighthouse-core/config/config.js +++ b/lighthouse-core/config/config.js @@ -749,13 +749,13 @@ class Config { /** * Take an array of audits and audit paths and require any paths (possibly - * relative to the optional `configPath`) using `Runner.resolvePlugin`, + * relative to the optional `configDir`) using `Config.resolveModule`, * leaving only an array of AuditDefns. * @param {LH.Config.Json['audits']} audits - * @param {string=} configPath + * @param {string=} configDir * @return {Config['audits']} */ - static requireAudits(audits, configPath) { + static requireAudits(audits, configDir) { const status = {msg: 'Requiring audits', id: 'lh:config:requireAudits'}; log.time(status, 'verbose'); const expandedAudits = Config.expandAuditShorthand(audits); @@ -775,7 +775,7 @@ class Config { let requirePath = `../audits/${audit.path}`; if (!coreAudit) { // Otherwise, attempt to find it elsewhere. This throws if not found. - requirePath = Runner.resolvePlugin(audit.path, configPath, 'audit'); + requirePath = Config.resolveModule(audit.path, configDir, 'audit'); } implementation = /** @type {typeof Audit} */ (require(requirePath)); } @@ -797,16 +797,16 @@ class Config { * @param {string} path * @param {{}=} options * @param {Array} coreAuditList - * @param {string=} configPath + * @param {string=} configDir * @return {LH.Config.GathererDefn} */ - static requireGathererFromPath(path, options, coreAuditList, configPath) { + static requireGathererFromPath(path, options, coreAuditList, configDir) { const coreGatherer = coreAuditList.find(a => a === `${path}.js`); let requirePath = `../gather/gatherers/${path}`; if (!coreGatherer) { // Otherwise, attempt to find it elsewhere. This throws if not found. - requirePath = Runner.resolvePlugin(path, configPath, 'gatherer'); + requirePath = Config.resolveModule(path, configDir, 'gatherer'); } const GathererClass = /** @type {GathererConstructor} */ (require(requirePath)); @@ -821,13 +821,13 @@ class Config { /** * Takes an array of passes with every property now initialized except the - * gatherers and requires them, (relative to the optional `configPath` if - * provided) using `Runner.resolvePlugin`, returning an array of full Passes. + * gatherers and requires them, (relative to the optional `configDir` if + * provided) using `Config.resolveModule`, returning an array of full Passes. * @param {?Array>} passes - * @param {string=} configPath + * @param {string=} configDir * @return {Config['passes']} */ - static requireGatherers(passes, configPath) { + static requireGatherers(passes, configDir) { if (!passes) { return null; } @@ -855,7 +855,7 @@ class Config { } else if (gathererDefn.path) { const path = gathererDefn.path; const options = gathererDefn.options; - return Config.requireGathererFromPath(path, options, coreList, configPath); + return Config.requireGathererFromPath(path, options, coreList, configDir); } else { throw new Error('Invalid expanded Gatherer: ' + JSON.stringify(gathererDefn)); } @@ -869,6 +869,52 @@ class Config { log.timeEnd(status); return fullPasses; } + + /** + * Resolves the location of the specified module and returns an absolute + * string path to the file. Used for loading custom audits and gatherers. + * Throws an error if no module is found. + * @param {string} moduleIdentifier + * @param {string=} configDir The absolute path to the directory of the config file, if there is one. + * @param {string=} category Optional plugin category (e.g. 'audit') for better error messages. + * @return {string} + * @throws {Error} + */ + static resolveModule(moduleIdentifier, configDir, category) { + // First try straight `require()`. Unlikely to be specified relative to this + // file, but adds support for Lighthouse modules from npm since + // `require()` walks up parent directories looking inside any node_modules/ + // present. Also handles absolute paths. + try { + return require.resolve(moduleIdentifier); + } catch (e) {} + + // See if the module resolves relative to the current working directory. + // Most useful to handle the case of invoking Lighthouse as a module, since + // then the config is an object and so has no path. + const cwdPath = path.resolve(process.cwd(), moduleIdentifier); + try { + return require.resolve(cwdPath); + } catch (e) {} + + const errorString = 'Unable to locate ' + + (category ? `${category}: ` : '') + + `${moduleIdentifier} (tried to require() from '${__dirname}' and load from '${cwdPath}'`; + + if (!configDir) { + throw new Error(errorString + ')'); + } + + // Finally, try looking up relative to the config file path. Just like the + // relative path passed to `require()` is found relative to the file it's + // in, this allows module paths to be specified relative to the config file. + const relativePath = path.resolve(configDir, moduleIdentifier); + try { + return require.resolve(relativePath); + } catch (requireError) {} + + throw new Error(errorString + ` and '${relativePath}')`); + } } module.exports = Config; diff --git a/lighthouse-core/runner.js b/lighthouse-core/runner.js index db7c06575eb1..cb9fb5f02740 100644 --- a/lighthouse-core/runner.js +++ b/lighthouse-core/runner.js @@ -399,52 +399,6 @@ class Runner { return fileList.filter(f => /\.js$/.test(f) && f !== 'gatherer.js').sort(); } - /** - * Resolves the location of the specified plugin and returns an absolute - * string path to the file. Used for loading custom audits and gatherers. - * Throws an error if no plugin is found. - * @param {string} plugin - * @param {string=} configDir The absolute path to the directory of the config file, if there is one. - * @param {string=} category Optional plugin category (e.g. 'audit') for better error messages. - * @return {string} - * @throws {Error} - */ - static resolvePlugin(plugin, configDir, category) { - // First try straight `require()`. Unlikely to be specified relative to this - // file, but adds support for Lighthouse plugins in npm modules as - // `require()` walks up parent directories looking inside any node_modules/ - // present. Also handles absolute paths. - try { - return require.resolve(plugin); - } catch (e) {} - - // See if the plugin resolves relative to the current working directory. - // Most useful to handle the case of invoking Lighthouse as a module, since - // then the config is an object and so has no path. - const cwdPath = path.resolve(process.cwd(), plugin); - try { - return require.resolve(cwdPath); - } catch (e) {} - - const errorString = 'Unable to locate ' + - (category ? `${category}: ` : '') + - `${plugin} (tried to require() from '${__dirname}' and load from '${cwdPath}'`; - - if (!configDir) { - throw new Error(errorString + ')'); - } - - // Finally, try looking up relative to the config file path. Just like the - // relative path passed to `require()` is found relative to the file it's - // in, this allows plugin paths to be specified relative to the config file. - const relativePath = path.resolve(configDir, plugin); - try { - return require.resolve(relativePath); - } catch (requireError) {} - - throw new Error(errorString + ` and '${relativePath}')`); - } - /** * Get path to use for -G and -A modes. Defaults to $CWD/latest-run * @param {LH.Config.Settings} settings diff --git a/lighthouse-core/test/config/config-test.js b/lighthouse-core/test/config/config-test.js index 48c08cd1eb32..f3a9d1476bc6 100644 --- a/lighthouse-core/test/config/config-test.js +++ b/lighthouse-core/test/config/config-test.js @@ -975,7 +975,7 @@ describe('Config', () => { }); }); - describe('#getDisplayString', () => { + describe('#getPrintString', () => { it('doesn\'t include empty gatherer/audit options in output', () => { const gOpt = 'gathererOption'; const aOpt = 'auditOption';