From ed14c8e0f98519de6aa9efa202d2cccc17881e85 Mon Sep 17 00:00:00 2001 From: Brendan Kenny Date: Wed, 2 May 2018 13:35:57 -0700 Subject: [PATCH] core(tsc): more type checking of top-level audits (#5089) --- lighthouse-core/audits/multi-check-audit.js | 21 +++++++++------- lighthouse-core/audits/redirects-http.js | 7 +++--- .../audits/screenshot-thumbnails.js | 16 +++++++++---- lighthouse-core/audits/service-worker.js | 6 ++--- lighthouse-core/audits/speed-index.js | 6 ++--- lighthouse-core/audits/splash-screen.js | 14 ++++++++--- lighthouse-core/audits/themed-omnibox.js | 19 ++++++++++++--- lighthouse-core/audits/time-to-first-byte.js | 12 +++++++--- lighthouse-core/audits/viewport.js | 6 ++--- .../audits/webapp-install-banner.js | 24 ++++++++++++++++--- lighthouse-core/audits/without-javascript.js | 6 ++--- lighthouse-core/audits/works-offline.js | 6 ++--- .../test/audits/time-to-first-byte-test.js | 21 ++++++++++++++++ package.json | 3 ++- tsconfig.json | 12 ++++++++++ typings/metaviewport-parser/index.d.ts | 13 ++++++++++ yarn.lock | 12 +++++++--- 17 files changed, 156 insertions(+), 48 deletions(-) create mode 100644 typings/metaviewport-parser/index.d.ts diff --git a/lighthouse-core/audits/multi-check-audit.js b/lighthouse-core/audits/multi-check-audit.js index 3db25030358a..a5dd9d943802 100644 --- a/lighthouse-core/audits/multi-check-audit.js +++ b/lighthouse-core/audits/multi-check-audit.js @@ -13,18 +13,18 @@ const Audit = require('./audit'); class MultiCheckAudit extends Audit { /** - * @param {!Artifacts} artifacts - * @return {!AuditResult} + * @param {LH.Artifacts} artifacts + * @return {Promise} */ static audit(artifacts) { - return Promise.resolve(this.audit_(artifacts)).then(result => this.createAuditResult(result)); + return Promise.resolve(this.audit_(artifacts)).then(result => this.createAuditProduct(result)); } /** - * @param {!{failures: !Array, themeColor: ?string, manifestValues: ?Object, }} result - * @return {!AuditResult} + * @param {{failures: Array, warnings?: Array, manifestValues?: LH.Artifacts.ManifestValues}} result + * @return {LH.Audit.Product} */ - static createAuditResult(result) { + static createAuditProduct(result) { const extendedInfo = { value: result, }; @@ -51,12 +51,17 @@ class MultiCheckAudit extends Audit { }; } + /* eslint-disable no-unused-vars */ + /** - * @param {!Artifacts} artifacts + * @param {LH.Artifacts} artifacts + * @return {Promise<{failures: Array, warnings?: Array, manifestValues?: LH.Artifacts.ManifestValues}>} */ - static audit_() { + static audit_(artifacts) { throw new Error('audit_ unimplemented'); } + + /* eslint-enable no-unused-vars */ } module.exports = MultiCheckAudit; diff --git a/lighthouse-core/audits/redirects-http.js b/lighthouse-core/audits/redirects-http.js index 04ab8871f12a..d70ca41a738e 100644 --- a/lighthouse-core/audits/redirects-http.js +++ b/lighthouse-core/audits/redirects-http.js @@ -9,7 +9,7 @@ const Audit = require('./audit'); class RedirectsHTTP extends Audit { /** - * @return {!AuditMeta} + * @return {LH.Audit.Meta} */ static get meta() { return { @@ -23,13 +23,12 @@ class RedirectsHTTP extends Audit { } /** - * @param {!Artifacts} artifacts - * @return {!AuditResult} + * @param {LH.Artifacts} artifacts + * @return {LH.Audit.Product} */ static audit(artifacts) { return { rawValue: artifacts.HTTPRedirect.value, - debugString: artifacts.HTTPRedirect.debugString, }; } } diff --git a/lighthouse-core/audits/screenshot-thumbnails.js b/lighthouse-core/audits/screenshot-thumbnails.js index 961cd5753f54..0511dcea4ed9 100644 --- a/lighthouse-core/audits/screenshot-thumbnails.js +++ b/lighthouse-core/audits/screenshot-thumbnails.js @@ -12,9 +12,11 @@ const jpeg = require('jpeg-js'); const NUMBER_OF_THUMBNAILS = 10; const THUMBNAIL_WIDTH = 120; +/** @typedef {LH.Artifacts.Speedline['frames'][0]} SpeedlineFrame */ + class ScreenshotThumbnails extends Audit { /** - * @return {!AuditMeta} + * @return {LH.Audit.Meta} */ static get meta() { return { @@ -30,8 +32,8 @@ class ScreenshotThumbnails extends Audit { * Scales down an image to THUMBNAIL_WIDTH using nearest neighbor for speed, maintains aspect * ratio of the original thumbnail. * - * @param {{width: number, height: number, data: !Array}} imageData - * @return {{width: number, height: number, data: !Array}} + * @param {ReturnType} imageData + * @return {{width: number, height: number, data: Uint8Array}} */ static scaleImageToThumbnail(imageData) { const scaledWidth = THUMBNAIL_WIDTH; @@ -63,11 +65,13 @@ class ScreenshotThumbnails extends Audit { } /** - * @param {!Artifacts} artifacts - * @return {!AuditResult} + * @param {LH.Artifacts} artifacts + * @param {LH.Audit.Context} context + * @return {Promise} */ static async audit(artifacts, context) { const trace = artifacts.traces[Audit.DEFAULT_PASS]; + /** @type {Map} */ const cachedThumbnails = new Map(); const speedline = await artifacts.requestSpeedline(trace); @@ -99,6 +103,8 @@ class ScreenshotThumbnails extends Audit { for (let i = 1; i <= NUMBER_OF_THUMBNAILS; i++) { const targetTimestamp = speedline.beginning + timelineEnd * i / NUMBER_OF_THUMBNAILS; + /** @type {SpeedlineFrame} */ + // @ts-ignore - there will always be at least one frame by this point. TODO: use nonnullable assertion in TS2.9 let frameForTimestamp = null; if (i === NUMBER_OF_THUMBNAILS) { frameForTimestamp = analyzedFrames[analyzedFrames.length - 1]; diff --git a/lighthouse-core/audits/service-worker.js b/lighthouse-core/audits/service-worker.js index 360a2c057ac2..187ef5f31c71 100644 --- a/lighthouse-core/audits/service-worker.js +++ b/lighthouse-core/audits/service-worker.js @@ -10,7 +10,7 @@ const Audit = require('./audit'); class ServiceWorker extends Audit { /** - * @return {!AuditMeta} + * @return {LH.Audit.Meta} */ static get meta() { return { @@ -25,8 +25,8 @@ class ServiceWorker extends Audit { } /** - * @param {!Artifacts} artifacts - * @return {!AuditResult} + * @param {LH.Artifacts} artifacts + * @return {LH.Audit.Product} */ static audit(artifacts) { // Find active service worker for this URL. Match against diff --git a/lighthouse-core/audits/speed-index.js b/lighthouse-core/audits/speed-index.js index a3e28fbb319d..bf19ed23e323 100644 --- a/lighthouse-core/audits/speed-index.js +++ b/lighthouse-core/audits/speed-index.js @@ -10,7 +10,7 @@ const Util = require('../report/html/renderer/util'); class SpeedIndex extends Audit { /** - * @return {!AuditMeta} + * @return {LH.Audit.Meta} */ static get meta() { return { @@ -39,9 +39,9 @@ class SpeedIndex extends Audit { /** * Audits the page to give a score for the Speed Index. * @see https://github.com/GoogleChrome/lighthouse/issues/197 - * @param {Artifacts} artifacts The artifacts from the gather phase. + * @param {LH.Artifacts} artifacts The artifacts from the gather phase. * @param {LH.Audit.Context} context - * @return {Promise} + * @return {Promise} */ static async audit(artifacts, context) { const trace = artifacts.traces[Audit.DEFAULT_PASS]; diff --git a/lighthouse-core/audits/splash-screen.js b/lighthouse-core/audits/splash-screen.js index 4307c67a81ce..f8b25ebf20f4 100644 --- a/lighthouse-core/audits/splash-screen.js +++ b/lighthouse-core/audits/splash-screen.js @@ -22,7 +22,7 @@ const MultiCheckAudit = require('./multi-check-audit'); class SplashScreen extends MultiCheckAudit { /** - * @return {!AuditMeta} + * @return {LH.Audit.Meta} */ static get meta() { return { @@ -36,8 +36,12 @@ class SplashScreen extends MultiCheckAudit { }; } + /** + * @param {LH.Artifacts.ManifestValues} manifestValues + * @param {Array} failures + */ static assessManifest(manifestValues, failures) { - if (manifestValues.isParseFailure) { + if (manifestValues.isParseFailure && manifestValues.parseFailureReason) { failures.push(manifestValues.parseFailureReason); return; } @@ -58,8 +62,12 @@ class SplashScreen extends MultiCheckAudit { }); } - + /** + * @param {LH.Artifacts} artifacts + * @return {Promise<{failures: Array, manifestValues: LH.Artifacts.ManifestValues}>} + */ static audit_(artifacts) { + /** @type {Array} */ const failures = []; return artifacts.requestManifestValues(artifacts.Manifest).then(manifestValues => { diff --git a/lighthouse-core/audits/themed-omnibox.js b/lighthouse-core/audits/themed-omnibox.js index a0fcfa29b230..cf5b158a0b90 100644 --- a/lighthouse-core/audits/themed-omnibox.js +++ b/lighthouse-core/audits/themed-omnibox.js @@ -20,7 +20,7 @@ const validColor = require('../lib/web-inspector').Color.parse; class ThemedOmnibox extends MultiCheckAudit { /** - * @return {!AuditMeta} + * @return {LH.Audit.Meta} */ static get meta() { return { @@ -33,6 +33,10 @@ class ThemedOmnibox extends MultiCheckAudit { }; } + /** + * @param {LH.Artifacts['ThemeColor']} themeColorMeta + * @param {Array} failures + */ static assessMetaThemecolor(themeColorMeta, failures) { if (themeColorMeta === null) { failures.push('No `` tag found'); @@ -41,19 +45,28 @@ class ThemedOmnibox extends MultiCheckAudit { } } + /** + * @param {LH.Artifacts.ManifestValues} manifestValues + * @param {Array} failures + */ static assessManifest(manifestValues, failures) { - if (manifestValues.isParseFailure) { + if (manifestValues.isParseFailure && manifestValues.parseFailureReason) { failures.push(manifestValues.parseFailureReason); return; } const themeColorCheck = manifestValues.allChecks.find(i => i.id === 'hasThemeColor'); - if (!themeColorCheck.passing) { + if (themeColorCheck && !themeColorCheck.passing) { failures.push(themeColorCheck.failureText); } } + /** + * @param {LH.Artifacts} artifacts + * @return {Promise<{failures: Array, manifestValues: LH.Artifacts.ManifestValues, themeColor: ?string}>} + */ static audit_(artifacts) { + /** @type {Array} */ const failures = []; return artifacts.requestManifestValues(artifacts.Manifest).then(manifestValues => { diff --git a/lighthouse-core/audits/time-to-first-byte.js b/lighthouse-core/audits/time-to-first-byte.js index cbf9f3e76567..c325f3cc7afa 100644 --- a/lighthouse-core/audits/time-to-first-byte.js +++ b/lighthouse-core/audits/time-to-first-byte.js @@ -12,7 +12,7 @@ const TTFB_THRESHOLD = 600; class TTFBMetric extends Audit { /** - * @return {!AuditMeta} + * @return {LH.Audit.Meta} */ static get meta() { return { @@ -25,6 +25,9 @@ class TTFBMetric extends Audit { }; } + /** + * @param {LH.WebInspector.NetworkRequest} record + */ static caclulateTTFB(record) { const timing = record._timing; @@ -32,8 +35,8 @@ class TTFBMetric extends Audit { } /** - * @param {!Artifacts} artifacts - * @return {!AuditResult} + * @param {LH.Artifacts} artifacts + * @return {Promise} */ static audit(artifacts) { const devtoolsLogs = artifacts.devtoolsLogs[Audit.DEFAULT_PASS]; @@ -44,6 +47,9 @@ class TTFBMetric extends Audit { const finalUrl = artifacts.URL.finalUrl; const finalUrlRequest = networkRecords.find(record => record._url === finalUrl); + if (!finalUrlRequest) { + throw new Error(`finalUrl '${finalUrl} not found in network records.`); + } const ttfb = TTFBMetric.caclulateTTFB(finalUrlRequest); const passed = ttfb < TTFB_THRESHOLD; diff --git a/lighthouse-core/audits/viewport.js b/lighthouse-core/audits/viewport.js index 8404150f4d7d..eafd66b72823 100644 --- a/lighthouse-core/audits/viewport.js +++ b/lighthouse-core/audits/viewport.js @@ -10,7 +10,7 @@ const Parser = require('metaviewport-parser'); class Viewport extends Audit { /** - * @return {!AuditMeta} + * @return {LH.Audit.Meta} */ static get meta() { return { @@ -25,8 +25,8 @@ class Viewport extends Audit { } /** - * @param {!Artifacts} artifacts - * @return {!AuditResult} + * @param {LH.Artifacts} artifacts + * @return {LH.Audit.Product} */ static audit(artifacts) { if (artifacts.Viewport === null) { diff --git a/lighthouse-core/audits/webapp-install-banner.js b/lighthouse-core/audits/webapp-install-banner.js index f8d5dc0c0804..ec2babf19b20 100644 --- a/lighthouse-core/audits/webapp-install-banner.js +++ b/lighthouse-core/audits/webapp-install-banner.js @@ -30,7 +30,7 @@ const SWAudit = require('./service-worker'); class WebappInstallBanner extends MultiCheckAudit { /** - * @return {!AuditMeta} + * @return {LH.Audit.Meta} */ static get meta() { return { @@ -44,11 +44,16 @@ class WebappInstallBanner extends MultiCheckAudit { }; } + /** + * @param {LH.Artifacts.ManifestValues} manifestValues + * @return {Array} + */ static assessManifest(manifestValues) { - if (manifestValues.isParseFailure) { + if (manifestValues.isParseFailure && manifestValues.parseFailureReason) { return [manifestValues.parseFailureReason]; } + /** @type {Array} */ const failures = []; const bannerCheckIds = [ 'hasName', @@ -68,7 +73,10 @@ class WebappInstallBanner extends MultiCheckAudit { return failures; } - + /** + * @param {LH.Artifacts} artifacts + * @return {Array} + */ static assessServiceWorker(artifacts) { const failures = []; const hasServiceWorker = SWAudit.audit(artifacts).rawValue; @@ -79,6 +87,10 @@ class WebappInstallBanner extends MultiCheckAudit { return failures; } + /** + * @param {LH.Artifacts} artifacts + * @return {{failures: Array, warnings: Array}} + */ static assessOfflineStartUrl(artifacts) { const failures = []; const warnings = []; @@ -98,8 +110,14 @@ class WebappInstallBanner extends MultiCheckAudit { return {failures, warnings}; } + /** + * @param {LH.Artifacts} artifacts + * @return {Promise<{failures: Array, warnings: Array, manifestValues: LH.Artifacts.ManifestValues}>} + */ static audit_(artifacts) { + /** @type {Array} */ let offlineFailures = []; + /** @type {Array} */ let offlineWarnings = []; return artifacts.requestManifestValues(artifacts.Manifest).then(manifestValues => { diff --git a/lighthouse-core/audits/without-javascript.js b/lighthouse-core/audits/without-javascript.js index 962398ac7dee..3c58cffb0507 100644 --- a/lighthouse-core/audits/without-javascript.js +++ b/lighthouse-core/audits/without-javascript.js @@ -9,7 +9,7 @@ const Audit = require('./audit'); class WithoutJavaScript extends Audit { /** - * @return {!AuditMeta} + * @return {LH.Audit.Meta} */ static get meta() { return { @@ -24,8 +24,8 @@ class WithoutJavaScript extends Audit { } /** - * @param {!Artifacts} artifacts - * @return {!AuditResult} + * @param {LH.Artifacts} artifacts + * @return {LH.Audit.Product} */ static audit(artifacts) { const artifact = artifacts.HTMLWithoutJavaScript; diff --git a/lighthouse-core/audits/works-offline.js b/lighthouse-core/audits/works-offline.js index 98c751bfc222..4a5371384b88 100644 --- a/lighthouse-core/audits/works-offline.js +++ b/lighthouse-core/audits/works-offline.js @@ -10,7 +10,7 @@ const Audit = require('./audit'); class WorksOffline extends Audit { /** - * @return {!AuditMeta} + * @return {LH.Audit.Meta} */ static get meta() { return { @@ -25,8 +25,8 @@ class WorksOffline extends Audit { } /** - * @param {!Artifacts} artifacts - * @return {!AuditResult} + * @param {LH.Artifacts} artifacts + * @return {LH.Audit.Product} */ static audit(artifacts) { let debugString; diff --git a/lighthouse-core/test/audits/time-to-first-byte-test.js b/lighthouse-core/test/audits/time-to-first-byte-test.js index c4fe77a8cf9c..69f14b856192 100644 --- a/lighthouse-core/test/audits/time-to-first-byte-test.js +++ b/lighthouse-core/test/audits/time-to-first-byte-test.js @@ -45,4 +45,25 @@ describe('Performance: time-to-first-byte audit', () => { assert.strictEqual(result.score, 1); }); }); + + it('throws when somehow finalUrl is not in network records', async () => { + const networkRecords = [ + {_url: 'https://example.com/', _requestId: '0', _timing: {receiveHeadersEnd: 400, sendEnd: 200}}, + {_url: 'https://google.com/styles.css', _requestId: '1', _timing: {receiveHeadersEnd: 850, sendEnd: 200}}, + {_url: 'https://google.com/image.jpg', _requestId: '2', _timing: {receiveHeadersEnd: 1000, sendEnd: 400}}, + ]; + const badFinalUrl = 'https://badexample.com/'; + const artifacts = { + devtoolsLogs: {[TimeToFirstByte.DEFAULT_PASS]: []}, + requestNetworkRecords: () => Promise.resolve(networkRecords), + URL: {finalUrl: badFinalUrl}, + }; + + try { + await TimeToFirstByte.audit(artifacts); + throw new Error('TimeToFirstByte did not throw'); + } catch (e) { + assert.ok(e.message.includes(badFinalUrl)); + } + }); }); diff --git a/package.json b/package.json index e34f9751d3b2..c6a201369cd1 100644 --- a/package.json +++ b/package.json @@ -70,6 +70,7 @@ "@types/configstore": "^2.1.1", "@types/css-font-loading-module": "^0.0.2", "@types/inquirer": "^0.0.35", + "@types/jpeg-js": "^0.3.0", "@types/lodash.isequal": "^4.5.2", "@types/node": "*", "@types/opn": "^3.0.28", @@ -123,7 +124,7 @@ "rimraf": "^2.6.1", "robots-parser": "^2.0.1", "semver": "^5.3.0", - "speedline": "1.3.2", + "speedline": "1.3.3", "update-notifier": "^2.1.0", "ws": "3.3.2", "yargs": "3.32.0", diff --git a/tsconfig.json b/tsconfig.json index 60bdb283a7da..9429fc2c9db8 100644 --- a/tsconfig.json +++ b/tsconfig.json @@ -34,6 +34,18 @@ "lighthouse-core/audits/metrics.js", "lighthouse-core/audits/mixed-content.js", "lighthouse-core/audits/network-requests.js", + "lighthouse-core/audits/multi-check-audit.js", + "lighthouse-core/audits/redirects-http.js", + "lighthouse-core/audits/screenshot-thumbnails.js", + "lighthouse-core/audits/service-worker.js", + "lighthouse-core/audits/speed-index.js", + "lighthouse-core/audits/splash-screen.js", + "lighthouse-core/audits/themed-omnibox.js", + "lighthouse-core/audits/time-to-first-byte.js", + "lighthouse-core/audits/viewport.js", + "lighthouse-core/audits/webapp-install-banner.js", + "lighthouse-core/audits/without-javascript.js", + "lighthouse-core/audits/works-offline.js", "lighthouse-core/audits/accessibility/**/*.js", "lighthouse-core/audits/dobetterweb/**/*.js", "lighthouse-core/audits/byte-efficiency/**/*.js", diff --git a/typings/metaviewport-parser/index.d.ts b/typings/metaviewport-parser/index.d.ts new file mode 100644 index 000000000000..f10008efce8c --- /dev/null +++ b/typings/metaviewport-parser/index.d.ts @@ -0,0 +1,13 @@ +/** + * @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 module 'metaviewport-parser' { + export function parseMetaViewPortContent(S: string): { + validProperties: {[p: string]: number | string}, + unknownProperties: {[p: string]: number | string}, + invalidValues: {[p: string]: number | string}; + }; +} diff --git a/yarn.lock b/yarn.lock index 95cbc3589c80..8b41c72a135c 100644 --- a/yarn.lock +++ b/yarn.lock @@ -41,6 +41,12 @@ "@types/rx" "*" "@types/through" "*" +"@types/jpeg-js@^0.3.0": + version "0.3.0" + resolved "https://registry.yarnpkg.com/@types/jpeg-js/-/jpeg-js-0.3.0.tgz#5971f0aa50900194e9565711c265c10027385e89" + dependencies: + "@types/node" "*" + "@types/lodash.isequal@^4.5.2": version "4.5.2" resolved "https://registry.yarnpkg.com/@types/lodash.isequal/-/lodash.isequal-4.5.2.tgz#adbdff67f7c956ed703009e5466a34eeddb0b712" @@ -4649,9 +4655,9 @@ spdx-license-ids@^1.0.2: version "1.2.2" resolved "https://registry.yarnpkg.com/spdx-license-ids/-/spdx-license-ids-1.2.2.tgz#c9df7a3424594ade6bd11900d596696dc06bac57" -speedline@1.3.2: - version "1.3.2" - resolved "https://registry.yarnpkg.com/speedline/-/speedline-1.3.2.tgz#b48bf899568d917bbb14c8397453b14c4c0d0ed0" +speedline@1.3.3: + version "1.3.3" + resolved "https://registry.yarnpkg.com/speedline/-/speedline-1.3.3.tgz#c5b38f7ba81a1e1cc34ed3e9cc0ac9b6260a20ed" dependencies: "@types/node" "*" babar "0.2.0"