Skip to content
This repository has been archived by the owner on Nov 16, 2023. It is now read-only.

Add the resource timings pane #5

Merged
merged 23 commits into from
Jan 17, 2018
Merged
Show file tree
Hide file tree
Changes from 19 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
3 changes: 2 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -3,4 +3,5 @@ dist/
*.js
*.js.map
*.d.ts
!injectDemoToolbar.user.js
!injectDemoToolbar.user.js
!commontypes.d.ts
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg Jan 12, 2018

Choose a reason for hiding this comment

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

This is part of your source, so it shouldn't be gitignored. #ByDesign

Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg Jan 12, 2018

Choose a reason for hiding this comment

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

(if you're doing something fancy about not changing the API, document it) #WontFix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hence the ! in front. :-)


In reply to: 161152364 [](ancestors = 161152364)

33 changes: 26 additions & 7 deletions index.html
Original file line number Diff line number Diff line change
Expand Up @@ -2,42 +2,53 @@
<html>
<head>
<title>Demo for WebPerfToolbar</title>
<style>
body { background-color: lightgray; }
</style>
<style>
/** TODO: These need to be moved into JS (for ease of usage in an app). They're inline in the demo for ease of development. */

#PTB_buttons {
position:fixed;
bottom: 0;
left: 50px;
border:1px solid black;
border-bottom: none;

list-style:none;
padding:0;
margin:0;

z-index: 2147483647; /* we're on top */
}
#PTB_buttons li {
display:inline-block;
line-height:1.6em;
margin-left:0.5em;
padding:0.2em;
cursor:pointer;

border:1px solid black;
border-bottom: none;
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg Jan 12, 2018

Choose a reason for hiding this comment

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

Nit: some styles have a space after the colon; some don't. #WontFix

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Won't fix because this is getting moved to JavaScript soon. I'll standardize then.


In reply to: 161152446 [](ancestors = 161152446)

}
#PTB_buttons li:first-child {
margin-left:0;
}

#PTB_frame {
position:fixed;
width:30%;
min-width:300px;
right:0;
left:0;
top:0;

width:100%;
height:100%;
border-left:1px solid black;
overflow:auto;

padding:0.5em;
background:rgba(255, 255, 255, 0.95);
z-index:2147483646; /* we're one layer below the top */
}

#PTB_frame table {
margin-top:0.5em;
border-collapse: collapse;
border-spacing: 0;
border: 1px solid black;
Expand All @@ -50,6 +61,10 @@
border: 1px solid black;
padding:0.2em;
}

#PTB_frame .numeric {
text-align: right;
}
</style>
</head>
<body>
Expand All @@ -67,11 +82,15 @@
(new PerfToolbar.Toolbar([
/** Configure this to include the panels you need */
{
panel: PerfToolbar.NavigationTimingsPanel,
panelConstructor: PerfToolbar.NavigationTimingsPanel,
config: {
goalMs: 25
}
},
{
panelConstructor: PerfToolbar.ResourceTimingsPanel,
config: {}
}
/** End configuration */
])).render();
}
Expand Down
8 changes: 6 additions & 2 deletions injectDemoToolbar.user.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@

var initFunc = function() {
var style = document.createElement("style");
style.innerHTML = "#PTB_buttons {position:fixed;bottom: 0;left: 50px;border:1px solid black;border-bottom: none;list-style:none;padding:0;margin:0;}#PTB_buttons li {display:inline-block;line-height:1.6em;margin-left:0.5em;padding:0.2em;cursor:pointer;}#PTB_buttons li:first-child {margin-left:0;}#PTB_frame {position:fixed;width:30%;min-width:300px;right:0;top:0;height:100%;border-left:1px solid black;background:white;z-index:99999;overflow:scroll;}#PTB_frame table {border-collapse: collapse;border-spacing: 0;border: 1px solid black;}#PTB_frame th {font-weight: bold;}#PTB_frame th,#PTB_frame td {border: 1px solid black;padding:0.2em;}";
style.innerHTML = "#PTB_buttons {position:fixed;bottom: 0;left: 50px;list-style:none;padding:0;margin:0;z-index: 2147483647; /* we're on top */}#PTB_buttons li {display:inline-block;line-height:1.6em;margin-left:0.5em;padding:0.2em;cursor:pointer;border:1px solid black;border-bottom: none;}#PTB_buttons li:first-child {margin-left:0;}#PTB_frame {position:fixed;left:0;top:0;width:100%;height:100%;overflow:auto;padding:0.5em;background:rgba(255, 255, 255, 0.95);z-index:2147483646; /* we're one layer below the top */}#PTB_frame table {margin-top:0.5em;border-collapse: collapse;border-spacing: 0;border: 1px solid black;}#PTB_frame th {font-weight: bold;}#PTB_frame th,#PTB_frame td {border: 1px solid black;padding:0.2em;}#PTB_frame .numeric {text-align: right;}";
document.body.appendChild(style);

var s = document.createElement("script");
Expand All @@ -27,10 +27,14 @@
(new PerfToolbar.Toolbar([
/** Configure this to include the panels you need */
{
panel: PerfToolbar.NavigationTimingsPanel,
panelConstructor: PerfToolbar.NavigationTimingsPanel,
config: {
goalMs: 25
}
},
{
panelConstructor: PerfToolbar.ResourceTimingsPanel,
config: {}
}
/** End configuration */
])).render();
Expand Down
10 changes: 8 additions & 2 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,17 @@
"description": "A toolbar for monitoring web performance.",
"main": "dist/bundle.js",
"scripts": {
"test": "karma start karma.conf.js",
"build": "webpack",
"test": "karma start karma.conf.js",
"demo": "npm run build && http-server -o -c-1",

"tslint": "tslint -p .",
"tsc-verbose": "tsc -p tsconfig.json --traceResolution true",
"tslint": "tslint -p ."

"test-tslint": "tslint -p test/.",
"test-tsc-verbose": "tsc -p test/tsconfig.json --traceResolution true",

"check": "npm run tslint && npm run test-tslint && npm run build"
},
"keywords": [],
"author": "",
Expand Down
8 changes: 8 additions & 0 deletions src/button.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,9 @@ export interface IButtonConfiguration {
/** The panel that owns this button. */
parent?: IPanel;

/** The name of the button, exposed in the title attribute for the button. */
title?: string;

/** Gets the background color for the button. */
getColor?(): string;

Expand All @@ -31,10 +34,14 @@ export class Button {
/** The panel that provides this button. */
public readonly parent: IPanel | undefined;

/** The name of the button, exposed in the title attribute for the button. */
public readonly title: string;

/**
* Create the button.
*/
public constructor(config: IButtonConfiguration = {}) {
this.title = config.title !== undefined ? config.title : "";
this.emoji = config.emoji !== undefined ? config.emoji : "";
this.parent = config.parent;
/* tslint:disable no-unbound-method */
Expand All @@ -50,6 +57,7 @@ export class Button {
public render(container: HTMLElement): void {
const li: HTMLLIElement = document.createElement("li");
li.setAttribute("style", `background-color:${this.getColor()}`);
li.setAttribute("title", this.title);
li.innerText = `${this.emoji} ${this.getValue()}`;

li.addEventListener("click", () => {
Expand Down
25 changes: 25 additions & 0 deletions src/commontypes.d.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
// begin types from https://github.com/Microsoft/TypeScript/issues/15012

type Required<T> = {
[P in Purify<keyof T>]: NonNullable<T[P]>;
};
type Purify<T extends string> = {[P in T]: T; }[T];
type NonNullable<T> = T & {};

// end types from https://github.com/Microsoft/TypeScript/issues/15012

/**
* The types we expect from entry.initiatorType.
* Values from https://w3c.github.io/resource-timing/#dom-performanceresourcetiming-initiatortype
*/
type InitiatorType = "link" | "script" | "img" | "iframe" | "css" | "navigation" | "xmlhttprequest" | "fetch" | "beacon" | "other";

/**
* Finish the typings for a file gotten by `performance.getEntriesByType("resource")`
*/
interface IResourcePerformanceEntry extends PerformanceEntry, PerformanceResourceTiming {
decodedBodySize: number;
encodedBodySize: number;
initiatorType: InitiatorType;
transferSize: number;
}
70 changes: 67 additions & 3 deletions src/formatter.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
/** The level of precision we want to see for numbers */
export const DECIMAL_PLACES: number = 2;

/** The maximum length of a file name */
export const MAX_FILE_NAME_LENGTH: number = 60;

/**
* Formats a duration for output. Makes sure the numbers are valid and only returns a certain number of decimal places.
* Invalid input returns a dash.
Expand All @@ -9,10 +12,71 @@ export const DECIMAL_PLACES: number = 2;
* @param decimalPlaces The number of decimal places to show.
*/
export const duration: (end: number, start: number, decimalPlaces?: number) => string =
(end: number, start: number, decimalPlaces: number = DECIMAL_PLACES): string => {
if (isNaN(end) || isNaN(start)) {
(end, start, decimalPlaces = DECIMAL_PLACES) => {
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg Jan 16, 2018

Choose a reason for hiding this comment

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

As mentioned in person: it's driving me crazy that you have duplicate type declarations here (one explicit, one implicit).

Instead of:

export const duration: (end: number, start: number, decimalPlaces?: number) => string =
     (end, start, decimalPlaces = DECIMAL_PLACES) => { /* ... */ };

...it would be simpler to write:

export const duration = (end: number, start: number, decimalPlaces?: number): string => { /* ... */ };
``` #Resolved

if (isNaN(end) ||
isNaN(start) ||
(end - start < 0) ||
(end === 0 && start === 0)) {
return "-";
}

return (end - start).toFixed(decimalPlaces);
return (end - start).toLocaleString(undefined, {
minimumFractionDigits: decimalPlaces,
maximumFractionDigits: decimalPlaces,
});
};

/**
* Formats a path into a filename.
* @param path The file name to be formatted.
* @param maxLength The maximum length of the returned file name, plus three characters for periods.
*/
export const pathToFilename: (path: string, maxLength?: number) => string =
(path, maxLength = MAX_FILE_NAME_LENGTH) => {
let trimmed: string = path.substring(path.lastIndexOf("/") + 1);

if (trimmed.length > maxLength) {
trimmed = `${trimmed.substring(0, maxLength)}...`;
}

return trimmed;
};

enum FileSizeUnits {
b,
Kb,
Mb,
}

Copy link
Contributor

@Dadstart Dadstart Jan 12, 2018

Choose a reason for hiding this comment

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

enum #Closed

/**
* Simple object for passing to toLocaleString to configure the number of decimal places to display.
*/
const LOCALE_STRING_DECIMAL_PLACES: { maximumFractionDigits: number; minimumFractionDigits: number } = {
minimumFractionDigits: DECIMAL_PLACES,
maximumFractionDigits: DECIMAL_PLACES,
};

/**
* Converts a size in bytes to another size, with a unit.
* @param bytes The size in bytes.
* @param unit The desired unit to conver to.
*/
export const sizeToString: (bytes: number, unit?: keyof typeof FileSizeUnits) => string =
(bytes, unit = "Kb"): string => {
const twoExpTen: number = 1024;

if (bytes === 0) {
return "-";
}

switch (unit) {
case "b":
return `${bytes.toLocaleString(undefined, LOCALE_STRING_DECIMAL_PLACES)} b`;
Copy link
Contributor

@Dadstart Dadstart Jan 17, 2018

Choose a reason for hiding this comment

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

case FileSizeUnits[FileSizeUnits.b]
rather than duplicating the string val #Closed

case "Kb":
return `${(bytes / twoExpTen).toLocaleString(undefined, LOCALE_STRING_DECIMAL_PLACES)} Kb`;
case "Mb":
return `${(bytes / (twoExpTen * twoExpTen)).toLocaleString(undefined, LOCALE_STRING_DECIMAL_PLACES)} Mb`;
default:
throw new Error("unknown unit");
Copy link
Contributor

@JoshuaKGoldberg JoshuaKGoldberg Jan 12, 2018

Choose a reason for hiding this comment

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

Feels like you should turn off the TSLint rule or find/make a config that allows for exhaustive switches. This code should never be hit, and is just wasted bytes. #ByDesign

Copy link
Contributor Author

Choose a reason for hiding this comment

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

By Design, since the value can be passed in from JS code that wouldn't know the type limitations.


In reply to: 161152748 [](ancestors = 161152748)

}
};
1 change: 1 addition & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
@@ -1,3 +1,4 @@
export { Toolbar } from "./toolbar";
export { Button } from "./button";
export { NavigationTimingsPanel } from "./panels/navigation-timing";
export { ResourceTimingsPanel } from "./panels/resource-timing";
9 changes: 2 additions & 7 deletions src/ipanel.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
import { Button } from "./button";
import { PanelFrame } from "./panelframe";

export interface IPanelWithConfiguration<C, P> {
export interface IPanelWithConfiguration<C extends IPanelConfig, P extends IPanel> {
config: C;
panel: IPanelConstructor<C, P>;
panelConstructor: IPanelConstructor<C, P>;
}

export interface IPanelConstructor<C, P> {
Expand All @@ -17,11 +17,6 @@ export interface IPanelConfig {

/** Describes a panel within the opened toolbar. */
export interface IPanel {
/**
* The name of the panel.
*/
name: string;

/**
* Gets the buttons provided by this panel to be displayed in the collapsed toolbar.
*/
Expand Down
16 changes: 12 additions & 4 deletions src/panels/navigation-timing.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,8 +5,14 @@ import { PanelFrame } from "../panelframe";

/** Describes the configuration options available for the network panel */
export interface INavigationTimingsPanelConfig extends IPanelConfig {
/** The emoji for the button */
buttonEmoji: string;
/** The title for the button */
buttonTitle: string;
/** The goal for the load duration */
goalMs: number;
/** The name of the panel */
panelName: string;
/** The performance timing object, can be included in the config to enable injection of a mock object for testing */
timings: PerformanceTiming;
}
Expand All @@ -15,14 +21,15 @@ export interface INavigationTimingsPanelConfig extends IPanelConfig {
const navigationTimingsPanelDefaultConfig: INavigationTimingsPanelConfig = {
goalMs: 500,
timings: performance.timing,
panelName: "Navigation Timings",
buttonTitle: "Duration from navigation to end of load event",
buttonEmoji: "⏱️",
};

/**
* Provides a panel that shows the navigation timings for a page
*/
export class NavigationTimingsPanel implements IPanel {
/** The name of the panel */
public readonly name: string = "Navigation Timings";

/** The settings for this panel. */
private readonly config: INavigationTimingsPanelConfig;
Expand All @@ -42,7 +49,8 @@ export class NavigationTimingsPanel implements IPanel {
public getButtons(): Button[] {
return [new Button({
parent: this,
emoji: "⏱️",
title: this.config.buttonTitle,
emoji: this.config.buttonEmoji,
getValue: (): string => `${Formatter.duration(this.config.timings.loadEventEnd, this.config.timings.navigationStart)} ms`,
getColor: (): string => this.config.timings.loadEventEnd - this.config.timings.navigationStart <= this.config.goalMs ? "green" : "red",
})];
Expand All @@ -55,7 +63,7 @@ export class NavigationTimingsPanel implements IPanel {
public render(target: HTMLElement): void {
const t: PerformanceTiming = this.config.timings;

target.innerHTML = `
target.innerHTML = `<h1>${this.config.panelName}</h1>
<table>
<tr>
<th>Get Connected</th>
Expand Down
Loading