Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Gatherer/artifact cleanup #6747

Closed
5 of 6 tasks
paulirish opened this issue Dec 7, 2018 · 6 comments
Closed
5 of 6 tasks

Gatherer/artifact cleanup #6747

paulirish opened this issue Dec 7, 2018 · 6 comments

Comments

@paulirish
Copy link
Member

paulirish commented Dec 7, 2018

In a plugin world, the gatherers become part of our public API. We know about a few things we're interested in changing. Let's capture them here.

@patrickhulce you had a few in mind, too, right?

@patrickhulce
Copy link
Collaborator

meta and links were the two big ones 👍 I also wanted to cleanup

  • Images into ImageElements
  • AnchorElements

@patrickhulce
Copy link
Collaborator

I like the standardization on *Elements I'll update #6694, my only concern with LinkElements was that it would be easily confused with a elements :) I think it makes sense though when they'll be side-by-side

@paulirish paulirish added the P1 label Jan 3, 2019
@paulirish paulirish changed the title Gatherer cleanup Gatherer/artifact cleanup Jan 3, 2019
@egsweeny
Copy link
Contributor

egsweeny commented Jan 9, 2019

Is this issue comprehensive with what we're wanting to accomplish with artifact cleanup, or are there more that need to be tended to?

If it's not the full list, then can Connor take on fleshing it out?

@patrickhulce
Copy link
Collaborator

I think the primary goal is getting our artifacts in a state ready and useful for plugins. If someone wants to do some brainstorming what audits might need to know about a page and how we should standardize it, that'd be super useful :)

@patrickhulce
Copy link
Collaborator

OK decision time and input needed! Over in #8133 (comment) we were discussing exposing the DevTools-style child-index path on the ScriptElements artifact. This brings up a host of discussions about our public API for artifacts in v5.

My proposal is we only expose *Elements artifacts and the base artifacts to plugins by default. This will give plugins access to network records and the primary elements on the page which should take you pretty far. Everything else we make opt-in via dangerouslyUseExperimentalArtifacts or an Experimental prefix in the artifact name or something.

Most of the artifacts were written for specific audits in mind and by exposing them to arbitrary usage I think we run the risk of folks accidentally using the wrong thing or in a way we didn't intend and coming up with wrong results and/or stuck supporting a use case we never wanted to support. This is somewhat similar policy to the devtools protocol methods, making our BaseArtifacts + Element artifacts our "v1 stable" protocol.

In addition, I think certain properties on our elements should become "private". For example, the only purpose of the devtools-specific child-index node path is for node revealing, so until we support plugins in Devtools (feels a ways off), there is no need to expose that under a generic property name of path and have folks start relying on it for some other purpose.

Thoughts on this proposal? Any objections?

export interface BaseArtifacts {
/** The ISO-8601 timestamp of when the test page was fetched and artifacts collected. */
fetchTime: string;
/** A set of warnings about unexpected things encountered while loading and testing the page. */
LighthouseRunWarnings: string[];
/** Whether the page was loaded on either a real or emulated mobile device. */
TestedAsMobileDevice: boolean;
/** The user agent string of the version of Chrome used. */
HostUserAgent: string;
/** The user agent string that Lighthouse used to load the page. */
NetworkUserAgent: string;
/** The benchmark index that indicates rough device class. */
BenchmarkIndex: number;
/** Parsed version of the page's Web App Manifest, or null if none found. */
WebAppManifest: Artifacts.Manifest | null;
/** Information on detected tech stacks (e.g. JS libraries) used by the page. */
Stacks: Artifacts.DetectedStack[] | null;
/** A set of page-load traces, keyed by passName. */
traces: {[passName: string]: Trace};
/** A set of DevTools debugger protocol records, keyed by passName. */
devtoolsLogs: {[passName: string]: DevtoolsLog};
/** An object containing information about the testing configuration used by Lighthouse. */
settings: Config.Settings;
/** The URL initially requested and the post-redirects URL that was actually loaded. */
URL: {requestedUrl: string, finalUrl: string};
/** The timing instrumentation of the gather portion of a run. */
Timing: Artifacts.MeasureEntry[];
}

@brendankenny
Copy link
Member

This is more or less done for 5.0 in PublicGathererArtifacts (and BaseArtifacts). Growing the public artifacts will be an ongoing project.

See the number of links above for PRs where this was done and plugins.md#available-artifacts for developer information on using the artifacts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants