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

core(lhr): lhr-lite type declaration #4983

Merged
merged 6 commits into from
Apr 23, 2018
Merged
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
132 changes: 132 additions & 0 deletions typings/lhr-lite.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,132 @@
/**
* @license Copyright 2018 Google Inc. All Rights Reserved.
* Licensed under the Apache License, Version 2.0 (the "License"); you may not use this file except in compliance with the License. You may obtain a copy of the License at http://www.apache.org/licenses/LICENSE-2.0
* Unless required by applicable law or agreed to in writing, software distributed under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the License for the specific language governing permissions and limitations under the License.
*/

declare global {
module LH {
/**
* The lightweight version of Lighthouse results.
*/
export interface ResultLite {
/** The URL that was supplied to Lighthouse and initially navigated to. */
requestedUrl: string;
/** The post-redirects URL that Lighthouse loaded. */
finalUrl: string;
/** The ISO-8601 timestamp of when the results were generated. */
fetchedAt: string;
/** The version of Lighthouse with which these results were generated. */
lighthouseVersion: string;
/** An object containing the results of the audits. */
audits: Record<string, ResultLite.Audit>;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

since we don't have any references to what auditId really is how do you feel about aliases like type AuditId = string?

/** An object containing the top-level categories, their overall scores, and reference to member audits. */
categories?: Record<string, ResultLite.Category>;
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be required, even if an empty object (API responses should always have categories, so no need to make people jump through hoops checking for categories existence every time)

/** Descriptions of the audit groups referenced by AuditRefs. */
categoryGroups?: Record<string, ResultLite.CategoryGroup>;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did we decide of groups for this? I don't remember at this point 😆 or auditGroups aka "groups of related audits"

}

// ResultLite namespace
export module ResultLite {
export interface Category {
/** The human-friendly name of the category. */
title: string;
/** A description of what this category is about (e.g. these help you validate your PWA). */
description: string;
/** The overall score of the category, the weighted average of all its audits. */
score: number;
/** An array of references to all the audit members of this category. */
auditRefs: AuditRef[];
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😬

/** An optional description for manual audits within this category. */
manualDescription?: string;
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

😬

}

/**
* A reference to an audit result, with weighting and grouping information
* for its place in this category.
*/
export interface AuditRef {
/** Matches a key in the top-level `audits` object. */
auditId: string;
/** The weight of the audit's score in the overall category score. */
weight: number;
/** Optional grouping within the category. Matches the key in the top-level `categoryGroups` object. */
groupId?: string;
}

export interface CategoryGroup {
/** The human-friendly name of the category. */
title: string;
/** A brief description of the purpose of the display group. */
description: string;
}

export interface Audit {
/** The brief description of the audit. The text can change depending on if the audit passed or failed. */
title: string;
/** A more detailed description that describes why the audit is important and links to Lighthouse documentation on the audit; markdown links supported. */
description: string;
/** The scored value determined by the audit, in the range `0-1`. */
score: number;
/**
* A string identifying how the score should be interpreted:
* 'binary': pass/fail audit (0 and 1 are only possible scores).
* 'numeric': scores of 0-1 (map to displayed scores of 0-100).
* 'informative': the audit is an FYI only, and can't be interpreted as pass/fail. Ignore the score.
* 'notApplicable': the audit turned out to not apply to the page. Ignore the score.
* 'manual': The audit exists only to tell you to review something yourself. Ignore the score.
*/
scoreDisplayMode: 'binary' | 'numeric' | 'informative' | 'notApplicable' | 'manual';
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we have an 'error' scoreDisplayMode? That brings us (basically) back to a boolean and a string, which isn't ideal, but it also makes sense as it affects interpretation of the score in the same way that 'informative' 'notApplicable', and 'manual' do (you should ignore whatever is in score) and you won't have to look in multiple places to see how you should interpret score (like you do currently)

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I like it 👍

/** Extra information provided by some types of audits. */
details?: AuditDetails;
/** Error message from any exception thrown while running this audit. */
errorMessage?: string;
}

export type AuditDetails = AuditMetricDetails | AuditOpportunityDetails | AuditTableDetails;

export interface AuditMetricDetails {
type: 'metric';
/** The value of the metric expressed in milliseconds. */
timespanMs?: number;
}

export interface AuditOpportunityDetails {
type: 'opportunity';
wastedMs: number
wastedBytes?: number
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should we rename these totalWastedMs/totalWastedBytes? To make it clear what they are

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Crazy idea. since these are opportunities, we could stay on the optimistic side.. something like.. "estimatedSavingsMs" ?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

estimatedSavingsMs

I'm on board with that @patrickhulce

headings: AuditColumnHeading[];
rows: AuditTableRow[];
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renaming items to rows, eh? :)

I prefer items personally because it doesn't imply these table semantics. Table-ness is a visual/report thing, which i don't think is relevant to most consumers of the LHR

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

renaming items to rows, eh? :)

hehe, caught me. I can switch back, but "headings" doesn't really make sense without some matrix/table-ness structure imposed on the data anyways

}

export interface AuditTableDetails {
type: 'table';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

do we actually have any of these that we want to expose? seems like we're only wanting to expose the opportunities which all use the table headings/rows types

headings: AuditColumnHeading[];
rows: AuditTableRow[];
}

export interface AuditColumnHeading {
key: string;
label: string;
valueType: 'url' | 'text' | 'link' | 'timespanMs';
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

also have bytes and duration

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

duration -> timespanMs I thought? Also do we actually have text or links if it's just WastedBytesDetailsItem or WastedTimeDetailsItem?

}

export type AuditTableRow = WastedBytesDetailsRow | WastedTimeDetailsRow;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can u inline this into the two uses above?


export interface WastedBytesDetailsRow {
url: string;
wastedBytes?: number;
totalBytes?: number;
}

export interface WastedTimeDetailsRow {
url: string;
wastedMs: number;
totalBytes?: number;
}
}
}
}

// empty export to keep file a module
export {}