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

misc(tsc): reorganize and expose external interfaces #4788

Merged
merged 3 commits into from
Mar 16, 2018
Merged
Show file tree
Hide file tree
Changes from all 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
1 change: 0 additions & 1 deletion lighthouse-cli/bin.js
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@ const runLighthouse = require('./run').runLighthouse;
const log = require('lighthouse-logger');
// @ts-ignore
const perfOnlyConfig = require('../lighthouse-core/config/perf.json');
// @ts-ignore
const mixedContentConfig = require('../lighthouse-core/config/mixed-content.js');
// @ts-ignore
const pkg = require('../package.json');
Expand Down
76 changes: 10 additions & 66 deletions lighthouse-core/audits/audit.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class Audit {
}

/**
* @return {Audit.ScoringModes}
* @return {LH.Audit.ScoringModes}
*/
static get SCORING_MODES() {
return {
Expand All @@ -35,7 +35,7 @@ class Audit {
}

/**
* @return {Audit.Meta}
* @return {LH.Audit.Meta}
*/
static get meta() {
throw new Error('Audit meta information must be overridden.');
Expand Down Expand Up @@ -66,7 +66,7 @@ class Audit {
/**
* @param {typeof Audit} audit
* @param {string} debugString
* @return {LH.AuditFullResult}
* @return {LH.Audit.Result}
*/
static generateErrorAuditResult(audit, debugString) {
return Audit.generateAuditResult(audit, {
Expand All @@ -77,10 +77,10 @@ class Audit {
}

/**
* @param {Audit.Headings} headings
* @param {Array<LH.Audit.Heading>} headings
* @param {Array<Object<string, string>>} results
* @param {Audit.DetailsRenderer.DetailsSummary} summary
* @return {Audit.DetailsRenderer.DetailsJSON}
* @param {LH.Audit.DetailsRendererDetailsSummary} summary
* @return {LH.Audit.DetailsRendererDetailsJSON}
*/
static makeTableDetails(headings, results, summary) {
if (results.length === 0) {
Expand All @@ -102,8 +102,8 @@ class Audit {

/**
* @param {typeof Audit} audit
* @param {LH.AuditResult} result
* @return {{score: number, scoreDisplayMode: Audit.ScoringModeValues}}
* @param {LH.Audit.Product} result
* @return {{score: number, scoreDisplayMode: LH.Audit.ScoringModeValue}}
*/
static _normalizeAuditScore(audit, result) {
// Cast true/false to 1/0
Expand All @@ -125,8 +125,8 @@ class Audit {

/**
* @param {typeof Audit} audit
* @param {LH.AuditResult} result
* @return {LH.AuditFullResult}
* @param {LH.Audit.Product} result
* @return {LH.Audit.Result}
*/
static generateAuditResult(audit, result) {
if (typeof result.rawValue === 'undefined') {
Expand Down Expand Up @@ -173,59 +173,3 @@ class Audit {
}

module.exports = Audit;

/**
* @typedef {Object} Audit.ScoringModes
* @property {'numeric'} NUMERIC
* @property {'binary'} BINARY
*/

/**
* @typedef {Audit.ScoringModes[keyof Audit.ScoringModes]} Audit.ScoringModeValues
*/

/**
* @typedef {Object} Audit.Meta
* @property {string} name
* @property {string} description
* @property {string} helpText
* @property {Array<string>} requiredArtifacts
* @property {string} [failureDescription]
* @property {boolean} [informative]
* @property {boolean} [manual]
* @property {Audit.ScoringModeValues} [scoreDisplayMode]
*/

/**
* @typedef {Object} Audit.Heading
* @property {string} key
* @property {string} itemType
* @property {string} text
*/

/**
* @typedef {Array<Audit.Heading>} Audit.Headings
*/

/**
* @typedef {Object} Audit.HeadingsResult
* @property {number} results
* @property {Audit.Headings} headings
* @property {boolean} passes
* @property {string} [debugString]
*/

// TODO: placeholder typedefs until Details are typed
/**
* @typedef {void} Audit.DetailsRenderer.DetailsSummary
* @property {number} [wastedMs]
* @property {number} [wastedKb]
*/

/**
* @typedef {object} Audit.DetailsRenderer.DetailsJSON
* @property {'table'} type
* @property {Array<Audit.Heading>} headings
* @property {Array<Object<string, string>>} items
* @property {Audit.DetailsRenderer.DetailsSummary} summary
*/
20 changes: 4 additions & 16 deletions lighthouse-core/gather/gatherers/gatherer.js
Original file line number Diff line number Diff line change
Expand Up @@ -28,15 +28,15 @@ class Gatherer {

/**
* Called before navigation to target url.
* @param {Gatherer.PassContext} passContext
* @param {LH.Gatherer.PassContext} passContext
* @return {*|!Promise<*>}
*/
beforePass(passContext) { }

/**
* Called after target page is loaded. If a trace is enabled for this pass,
* the trace is still being recorded.
* @param {Gatherer.PassContext} passContext
* @param {LH.Gatherer.PassContext} passContext
* @return {*|!Promise<*>}
*/
pass(passContext) { }
Expand All @@ -45,25 +45,13 @@ class Gatherer {
* Called after target page is loaded, all gatherer `pass` methods have been
* executed, and — if generated in this pass — the trace is ended. The trace
* and record of network activity are provided in `loadData`.
* @param {Gatherer.PassContext} passContext
* @param {Gatherer.LoadData} loadData
* @param {LH.Gatherer.PassContext} passContext
* @param {LH.Gatherer.LoadData} loadData
* @return {*|!Promise<*>}
*/
afterPass(passContext, loadData) { }

/* eslint-enable no-unused-vars */
}

/**
* @typedef {Object} Gatherer.PassContext
* @property {object} options
*/

/**
* @typedef {Object} Gatherer.LoadData
* @property {Array<LH.NetworkRequest>} networkRecords
* @property {Array<void>} devtoolsLog
* @property {{traceEvents: Array<LH.TraceEvent>}} trace
*/

module.exports = Gatherer;
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

const INITIAL_CWD = 14 * 1024;

module.exports = class NetworkAnalyzer {
class NetworkAnalyzer {
/**
* @return {string}
*/
Expand Down Expand Up @@ -303,7 +303,9 @@ module.exports = class NetworkAnalyzer {
const estimatesByOrigin = NetworkAnalyzer._estimateResponseTimeByOrigin(records, rttByOrigin);
return NetworkAnalyzer.summarize(estimatesByOrigin);
}
};
}

module.exports = NetworkAnalyzer;

/**
* @typedef NetworkAnalyzer.Summary
Expand Down
4 changes: 3 additions & 1 deletion lighthouse-core/report/v2/renderer/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,9 @@ class Util {
* @return {string}
*/
static getURLDisplayName(parsedUrl, options) {
options = options || {};
// Closure optional properties aren't optional in tsc, so fallback needs undefined values.
options = options || {numPathParts: undefined, preserveQuery: undefined,
preserveHost: undefined};
const numPathParts = options.numPathParts !== undefined ? options.numPathParts : 2;
const preserveQuery = options.preserveQuery !== undefined ? options.preserveQuery : true;
const preserveHost = options.preserveHost || false;
Expand Down
2 changes: 1 addition & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,7 @@
"mocha": "^3.2.0",
"npm-run-posix-or-windows": "^2.0.2",
"sinon": "^2.3.5",
"typescript": "2.7.2",
"typescript": "^2.8.0-rc",
"zone.js": "^0.7.3"
},
"dependencies": {
Expand Down
2 changes: 1 addition & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"lighthouse-core/lib/dependency-graph/**/*.js",
"lighthouse-core/gather/connections/**/*.js",
"lighthouse-core/gather/gatherers/gatherer.js",
"./typings/externs.d.ts"
"./typings/*.d.ts"
],
"exclude": [
"lighthouse-cli/test/**/*.js"
Expand Down
88 changes: 88 additions & 0 deletions typings/audit.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,88 @@
/**
* @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 namespace LH.Audit {
Copy link
Member Author

Choose a reason for hiding this comment

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

global namespaces are merged, so this adds to the LH namespace defined in externs.d.ts

export interface ScoringModes {
Copy link
Collaborator

Choose a reason for hiding this comment

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

enum?

Copy link
Member Author

Choose a reason for hiding this comment

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

it looks like we can't (at least in a d.ts file). Since enum defines a type and a value in ts, it expects you to use the value of the enum anywhere the type is expected. So e.g. the values of Audit.SCORING_MODES would have to be LH.Audit.ScoringModes (not just 'numeric' and 'binary', even though that's what those are), but those don't exist in JS if the enum is defined here.

Copy link
Member Author

Choose a reason for hiding this comment

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

I could be wrong, but I can't make it work. Playground example

Copy link
Collaborator

Choose a reason for hiding this comment

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

you mean you're not a fan of this!?

😆

NUMERIC: 'numeric';
BINARY: 'binary';
}

export type ScoringModeValue = Audit.ScoringModes[keyof Audit.ScoringModes];

export interface Meta {
name: string;
description: string;
helpText: string;
requiredArtifacts: Array<string>;
failureDescription?: string;
informative?: boolean;
manual?: boolean;
scoreDisplayMode?: Audit.ScoringModeValue;
}

export interface Heading {
key: string;
itemType: string;
text: string;
}

export interface HeadingsResult {
results: number;
headings: Array<Audit.Heading>;
passes: boolean;
debugString?: string;
}

// TODO: placeholder typedefs until Details are typed
export interface DetailsRendererDetailsSummary {
wastedMs?: number;
wastedBytes?: number;
}

// TODO: placeholder typedefs until Details are typed
export interface DetailsRendererDetailsJSON {
type: 'table';
headings: Array<Audit.Heading>;
items: Array<{[x: string]: string}>;
summary: DetailsRendererDetailsSummary;
}

// Type returned by Audit.audit(). Only rawValue is required.
export interface Product {
rawValue: boolean | number | null;
displayValue?: string;
debugString?: string;
score?: number;
extendedInfo?: {value: string};
notApplicable?: boolean;
error?: boolean;
// TODO: define details
details?: object;
}

/* Audit result returned in Lighthouse report. All audits offer a description and score of 0-1 */
export interface Result {
rawValue: boolean | number | null;
displayValue: string;
debugString?: string;
score: number;
scoreDisplayMode: ScoringModeValue;
description: string;
extendedInfo?: {value: string};
notApplicable?: boolean;
error?: boolean;
name: string;
helpText?: string;
informative?: boolean;
manual?: boolean;
// TODO: define details
details?: object;
}

export interface Results {
[metric: string]: Result;
}
}
33 changes: 1 addition & 32 deletions typings/externs.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,40 +33,9 @@ declare namespace LH {

export interface Config {}

export interface AuditResult {
rawValue: boolean | number | null;
displayValue?: string;
debugString?: string;
score?: number;
extendedInfo?: {value: string};
notApplicable?: boolean;
error?: boolean;
// TODO: define details
details?: object;
}

export interface AuditResults {
[metric: string]: AuditResult;
}

export interface AuditFullResult extends AuditResult {
displayValue: string;
score: number;
scoreDisplayMode: 'numeric' | 'binary';
description: string;
name: string;
helpText?: string;
informative?: boolean;
manual?: boolean;
}

export interface AuditFullResults {
[metric: string]: AuditFullResult;
}

export interface Results {
url: string;
audits: AuditFullResults;
audits: LH.Audit.Results;
lighthouseVersion: string;
artifacts?: Object;
initialUrl: string;
Expand Down
17 changes: 17 additions & 0 deletions typings/gatherer.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,17 @@
/**
* @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 namespace LH.Gatherer {
export interface PassContext {
options: object;
}

export interface LoadData {
networkRecords: Array<LH.NetworkRequest>;
devtoolsLog: Array<void>;
trace: {trraceEvents: Array<LH.TraceEvent>}
}
}
6 changes: 3 additions & 3 deletions yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -4097,9 +4097,9 @@ typedarray@^0.0.6:
version "0.0.6"
resolved "https://registry.yarnpkg.com/typedarray/-/typedarray-0.0.6.tgz#867ac74e3864187b1d3d47d996a78ec5c8830777"

typescript@2.7.2:
version "2.7.2"
resolved "https://registry.yarnpkg.com/typescript/-/typescript-2.7.2.tgz#2d615a1ef4aee4f574425cdff7026edf81919836"
typescript@^2.8.0-rc:
version "2.8.0-rc"
resolved "https://registry.yarnpkg.com/typescript/-/typescript-2.8.0-rc.tgz#a0256b7d1d39fb7493ba0403f55e95d31e8bc374"

uglify-js@^2.6:
version "2.7.3"
Expand Down