filterPasses -> validatePasses #608
Conversation
| .filter(p => p.gatherers.length > 0); | ||
| return filteredPasses; | ||
| }); | ||
| return passes; |
There was a problem hiding this comment.
shouldn't return passes anymore
|
looks good. The only downside to not putting a |
| @@ -106,29 +106,22 @@ function cleanTrace(trace) { | |||
| return trace; | |||
| } | |||
|
|
|||
| function filterPasses(passes, audits, rootPath) { | |||
| function validatePasses(passes, audits, rootPath) { | |||
| if (!Array.isArray(passes)) { | |||
There was a problem hiding this comment.
why wouldnt it be an array?
There was a problem hiding this comment.
oh, if it's null? it seems confusing to do the check in here instead of in getGatherersNeededByAudits
There was a problem hiding this comment.
I forgot this isn't returning anything, just logging now, so that's fine, actually
|
I think this still messes up the config object if it's kept around externally to lighthouse, but there should really be a test for that sort of thing :) |
|
should note for those following along, this PR now also does:
|
|
LGTM |
|
Thx. updated the PR description. |
This PR:
The current filterPass behavior implies that the user can leave all default core gatherers in the config and we'll remove the ones that aren't needed.
This is a bit odd as audits and aggregation sections must be correct, so we shouldn't make gatherers in particular be so forgiving.
Furthermore, the custom perf config no longer requires any core gatherers. It only needs trace & network records, which, without this PR, will end up filtering out the only pass in the config.
Now instead of changing the config, config.js will issue warnings when gatherers are specified in the config that appear to be unnecessary. The resolution is adjusting the config.