feat: allow extension of default config#1731
Conversation
|
This looks useful. I dreamed of similar things in #887. Our single gigantic config is being unwieldy. This will also help consumers of LH more easily create custom configs. |
| return value; | ||
| } else { | ||
| return extension; | ||
| } |
There was a problem hiding this comment.
nit: ditch else and just return extension;?
| configJSON.audits = Array.from(inputConfig.audits); | ||
| } | ||
|
|
||
| // Extend the default config is specified |
There was a problem hiding this comment.
// Extend the default config if specified.
| validatePasses(configJSON.passes, this._audits, this._configDir); | ||
| } | ||
|
|
||
| static extendConfigJSON(baseJSON, extendJSON) { |
There was a problem hiding this comment.
might want to add jsdocs, even though it's fairly obvious what everything is.
|
Can you add documentation to the README? In the "creating custom configs" section. |
paulirish
left a comment
There was a problem hiding this comment.
yup generally like this approach and think itll be more flexible than passing options from CLI for everything. nice.
| return artifacts; | ||
| } | ||
|
|
||
| function merge(value, extension) { |
There was a problem hiding this comment.
how about s/value/base/. communicates a little better which is being mutated, and in line with your extendConfigJSON below
| if (!Array.isArray(value)) throw new TypeError(`Expected array but got ${typeof value}`); | ||
| return value.concat(extension); | ||
| } else if (typeof extension === 'object') { | ||
| if (typeof value !== 'object') throw new TypeError(`Expected array but got ${typeof value}`); |
| delete extendJSON.passes; | ||
| } | ||
|
|
||
| Object.keys(extendJSON).forEach(key => { |
There was a problem hiding this comment.
couldn't these three lines be replaced with merge(baseJSON, extendJSON) ?
There was a problem hiding this comment.
yep! artifact of earlier implementation, done
|
@ebidel added a readme entry PTAL :) |
would address part of #1730, it's rather out there so comments and thoughts welcome on other possible approaches
usage