Skip to content

Commit

Permalink
misc: clean up local names within Config (#6950)
Browse files Browse the repository at this point in the history
  • Loading branch information
brendankenny committed Jan 8, 2019
1 parent 781201b commit ca4fcad
Show file tree
Hide file tree
Showing 3 changed files with 59 additions and 59 deletions.
70 changes: 58 additions & 12 deletions lighthouse-core/config/config.js
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand All @@ -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));
}
Expand All @@ -797,16 +797,16 @@ class Config {
* @param {string} path
* @param {{}=} options
* @param {Array<string>} 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));
Expand All @@ -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<Required<LH.Config.PassJson>>} passes
* @param {string=} configPath
* @param {string=} configDir
* @return {Config['passes']}
*/
static requireGatherers(passes, configPath) {
static requireGatherers(passes, configDir) {
if (!passes) {
return null;
}
Expand Down Expand Up @@ -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));
}
Expand All @@ -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;
46 changes: 0 additions & 46 deletions lighthouse-core/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
2 changes: 1 addition & 1 deletion lighthouse-core/test/config/config-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -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';
Expand Down

0 comments on commit ca4fcad

Please sign in to comment.