GoogleChrome / lighthouse

Automated auditing, performance metrics, and best practices for the web.
https://developer.chrome.com/docs/lighthouse/overview/
Apache License 2.0
28.01k stars 9.31k forks source link

Proposal: Allow Gatherers to Opt-out of Lighthouse Filtering #16002

Closed alenaksu closed 4 weeks ago

alenaksu commented 1 month ago

Feature request summary

Lighthouse currently filters gatherers during execution, running only those explicitly required by audits. However, there are existing use cases within the Lighthouse codebase where gatherers need to run regardless of audits (e.g., Stacks, NetworkUserAgent, FullPageScreenshot). These gatherers often provide data used for various purposes beyond specific audits.

What is the motivation or use case for changing this?

Introduce a new meta property named alwaysRun. This property would be a boolean flag, that can be set to true whenever it is needed to run the gatherer even if audits do not request it.

import { Gatherer } from 'lighthouse';

class MyGatherer extends Gatherer { 
    meta = {
        alwaysRun: true,
    supportedModes: ['navigation']
    }

    async getArtifact() {
        // ...
    }
}

How is this beneficial to Lighthouse?

This functionality can be beneficial for various scenarios beyond the current examples in the Lighthouse codebase: developers can explicitly mark gatherers for always-on execution, even if not required by specific audits. For instance, a gatherer collecting custom page information could benefit from this to ensure data collection occurs regardless of the audits.

connorjclark commented 1 month ago

I believe we landed on this because it avoided landing a non-reversible change to the gatherer implementation that affects filtering, which is already kinda complex. But this doesn't help for custom gatherers.

We also have --disable-full-page-screenshot to exclude this logic for the FPS audit ... so a gatherer option called alwaysRun: true would be more like alwaysRunUnlessToldOtherwise :p Maybe runOnlyWhenRequested or runOnlyIfNeeded: false or supplementary: true ...

Is it an option for you to always include (via --only-audits) a hidden audit that depends on your custom artifact?

alenaksu commented 1 month ago

Yeah that's a valid point, adding such new property might end up making things even more complex.

In regards to the hidden audit, yes, that's what I tried when I found out that gatherers are filtered.

At first, I didn't get why it was not working, then I realised I needed to include the gatherer in an audit. So I implemented a fake audit, like the one in the example below, but I felt like it was not the best way to do it and I was wondering whether could be feasible to, in a way, decouple gatherers from audits.

class MyAudit extends Audit {
    static get meta() {
        return {
            id: 'my-audit',
            title: 'My Audit',
            failureTitle: 'Dummy failure title',
            description: 'Dummy description',
            requiredArtifacts: ['MyGatherer'],
        };
    }

    static audit() {
        return {
            score: 1
        };
    }
}

Thanks for your feedback, happy to discuss this further but feel free to close the issue.