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(tsc): remove more reliance on implicit index signatures #5874

Merged
merged 2 commits into from
Aug 21, 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: 1 addition & 0 deletions lighthouse-cli/bin.js
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ function run() {
}
})
.then(_ => {
// @ts-ignore TODO(bckenny): Sentry type checking
Sentry.init({
url,
flags: cliFlags,
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/audits/dobetterweb/dom-size.js
Original file line number Diff line number Diff line change
Expand Up @@ -101,6 +101,7 @@ class DOMSize extends Audit {
{key: 'width', itemType: 'text', text: str_(UIStrings.columnDOMWidth)},
];

/** @type {Array<Object<string, LH.Audit.DetailsItem>>} */
const items = [
{
totalNodes: Util.formatNumber(stats.totalDOMNodes),
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/audits/user-timings.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ class UserTimings extends Audit {
static filterTrace(tabTrace) {
/** @type {Array<MarkEvent|MeasureEvent>} */
const userTimings = [];
/** @type {Record<string, number>} */
const measuresStartTimes = {};

// Get all blink.user_timing events
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/gather/computed/metrics/lantern-metric.js
Original file line number Diff line number Diff line change
Expand Up @@ -85,6 +85,7 @@ class LanternMetricArtifact extends ComputedArtifact {
const optimisticGraph = this.getOptimisticGraph(graph, traceOfTab);
const pessimisticGraph = this.getPessimisticGraph(graph, traceOfTab);

/** @type {{flexibleOrdering?: boolean, label?: string}} */
let simulateOptions = {label: `optimistic${metricName}`};
const optimisticSimulation = simulator.simulate(optimisticGraph, simulateOptions);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,11 @@ const NetworkRequest = require('../../network-request');
// Assume that 40% of TTFB was server response time by default for static assets
const DEFAULT_SERVER_RESPONSE_PERCENTAGE = 0.4;

// For certain resource types, server response time takes up a greater percentage of TTFB (dynamic
// assets like HTML documents, XHR/API calls, etc)
/**
* For certain resource types, server response time takes up a greater percentage of TTFB (dynamic
* assets like HTML documents, XHR/API calls, etc)
* @type {Partial<Record<LH.Crdp.Page.ResourceType, number>>}
*/
const SERVER_RESPONSE_PERCENTAGE_OF_TTFB = {
Document: 0.9,
XHR: 0.9,
Expand Down Expand Up @@ -73,9 +76,11 @@ class NetworkAnalyzer {
return summaryByKey;
}

/** @typedef {{record: LH.Artifacts.NetworkRequest, timing: LH.Crdp.Network.ResourceTiming, connectionReused?: boolean}} RequestInfo */
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 apologize for the terrible name here, @patrickhulce :)

Copy link
Collaborator

Choose a reason for hiding this comment

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

heh, works for me :)


/**
* @param {LH.Artifacts.NetworkRequest[]} records
* @param {function(any):any} iteratee
* @param {(e: RequestInfo) => number | number[] | undefined} iteratee
* @return {Map<string, number[]>}
*/
static _estimateValueByOrigin(records, iteratee) {
Expand Down Expand Up @@ -190,6 +195,7 @@ class NetworkAnalyzer {
static _estimateRTTByOriginViaHeadersEndTiming(records) {
return NetworkAnalyzer._estimateValueByOrigin(records, ({record, timing, connectionReused}) => {
if (!Number.isFinite(timing.receiveHeadersEnd) || timing.receiveHeadersEnd < 0) return;
if (!record.resourceType) return;

const serverResponseTimePercentage = SERVER_RESPONSE_PERCENTAGE_OF_TTFB[record.resourceType]
|| DEFAULT_SERVER_RESPONSE_PERCENTAGE;
Expand Down
3 changes: 3 additions & 0 deletions lighthouse-core/lib/i18n.js
Original file line number Diff line number Diff line change
Expand Up @@ -175,6 +175,7 @@ function _formatPathAsString(pathInLHR) {
*/
function getRendererFormattedStrings(locale) {
const icuMessageIds = Object.keys(LOCALES[locale]).filter(f => f.includes('core/report/html/'));
/** @type {LH.I18NRendererStrings} */
const strings = {};
for (const icuMessageId of icuMessageIds) {
const [filename, varName] = icuMessageId.split(' | ');
Expand All @@ -190,6 +191,7 @@ function getRendererFormattedStrings(locale) {
* @param {Record<string, string>} fileStrings
*/
function createMessageInstanceIdFn(filename, fileStrings) {
/** @type {Record<string, string>} */
const mergedStrings = {...UIStrings, ...fileStrings};

/** @param {string} icuMessage @param {*} [values] */
Expand Down Expand Up @@ -284,6 +286,7 @@ function replaceIcuMessageInstanceIds(lhr, locale) {
}
}

/** @type {LH.I18NMessages} */
const icuMessagePaths = {};
replaceInObject(lhr, icuMessagePaths);
return icuMessagePaths;
Expand Down
3 changes: 2 additions & 1 deletion lighthouse-core/report/html/renderer/report-ui-features.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,7 +90,7 @@ class ReportUIFeatures {
this._setupHeaderAnimation();
this._resetUIState();
this._document.addEventListener('keydown', this.printShortCutDetect);
// @ts-ignore - tsc thinks document can't listen for `copy`
// @ts-ignore - TODO(bckenny): tsc thinks document can't listen for `copy`. Remove ignore in 3.1.
this._document.addEventListener('copy', this.onCopy);
}

Expand Down Expand Up @@ -447,6 +447,7 @@ class ReportUIFeatures {
*/
getReportHtml() {
this._resetUIState();
// @ts-ignore - technically documentElement can be null, but that's dumb - https://dom.spec.whatwg.org/#document-element
return this._document.documentElement.outerHTML;
}

Expand Down
5 changes: 5 additions & 0 deletions lighthouse-core/report/html/renderer/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -223,6 +223,7 @@ class Util {
* @return {string}
*/
static formatDateTime(date) {
/** @type {Intl.DateTimeFormatOptions} */
const options = {
month: 'short', day: 'numeric', year: 'numeric',
hour: 'numeric', minute: 'numeric', timeZoneName: 'short',
Expand Down Expand Up @@ -444,6 +445,10 @@ class Util {
*/
Util.numberDateLocale = 'en';

/**
* Report-renderer-specific strings.
* @type {LH.I18NRendererStrings}
*/
Util.UIStrings = {
/** Disclaimer shown to users below the metric values (First Contentful Paint, Time to Interactive, etc) to warn them that the numbers they see will likely change slightly the next time they run Lighthouse. */
varianceDisclaimer: 'Values are estimated and may vary.',
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/runner.js
Original file line number Diff line number Diff line change
Expand Up @@ -368,6 +368,7 @@ class Runner {
const ArtifactClass = require('./gather/computed/' + filename);
const artifact = new ArtifactClass(computedArtifacts);
// define the request* function that will be exposed on `artifacts`
// @ts-ignore - doesn't have an index signature, so can't be set dynamically.
computedArtifacts['request' + artifact.name] = artifact.request.bind(artifact);
Copy link
Member Author

Choose a reason for hiding this comment

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

artifact and ArtifactClass are already any here, and computedArtifacts already has a TODO to refactor above this method, so it seems fine to just ignore and keep the issues contained here than to make computedArtifacts have a generic index signature and have it leak into audits, etc.

});

Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/scoring.js
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,7 @@ class ReportScoring {
* @return {Object<string, LH.Result.Category>}
*/
static scoreAllCategories(configCategories, resultsByAuditId) {
/** @type {Record<string, LH.Result.Category>} */
const scoredCategories = {};

for (const [categoryId, configCategory] of Object.entries(configCategories)) {
Expand Down
1 change: 1 addition & 0 deletions lighthouse-core/scripts/i18n/collect-strings.js
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,7 @@ function collectAllStringsInDir(dir, strings = {}) {
*/
function writeStringsToLocaleFormat(locale, strings) {
const fullPath = path.join(LH_ROOT, `lighthouse-core/lib/locales/${locale}.json`);
/** @type {Record<string, ICUMessageDefn>} */
const output = {};
const sortedEntries = Object.entries(strings).sort(([keyA], [keyB]) => keyA.localeCompare(keyB));
for (const [key, defn] of sortedEntries) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -170,7 +170,7 @@ describe('DependencyGraph/Simulator/NetworkAnalyzer', () => {

it('should infer from TTFB when available', () => {
const timing = {receiveHeadersEnd: 1000};
const record = createRecord({startTime: 0, endTime: 1, timing});
const record = createRecord({startTime: 0, endTime: 1, timing, resourceType: 'Other'});
const result = NetworkAnalyzer.estimateRTTByOrigin([record], {
coarseEstimateMultiplier: 1,
});
Expand Down
3 changes: 3 additions & 0 deletions lighthouse-extension/app/src/lighthouse-ext-background.js
Original file line number Diff line number Diff line change
Expand Up @@ -122,7 +122,9 @@ function createReportPageAsBlob(reportHtml) {
*/
function saveSettings(settings) {
const storage = {
/** @type {Record<string, boolean>} */
[STORAGE_KEY]: {},
/** @type {Record<string, boolean>} */
[SETTINGS_KEY]: {},
};

Expand All @@ -149,6 +151,7 @@ function loadSettings() {
chrome.storage.local.get([STORAGE_KEY, SETTINGS_KEY], result => {
// Start with list of all default categories set to true so list is
// always up to date.
/** @type {Record<string, boolean>} */
const defaultCategories = {};
background.getDefaultCategories().forEach(category => {
defaultCategories[category.id] = true;
Expand Down