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(treemap): fix colors #12462

Merged
merged 5 commits into from
May 11, 2021
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
12 changes: 2 additions & 10 deletions lighthouse-treemap/app/debug.json
Original file line number Diff line number Diff line change
Expand Up @@ -12221,8 +12221,7 @@
{
"name": "jquery/2.2.4/jquery.min.js",
"resourceBytes": 85500,
"unusedBytes": 35751,
"duplicatedNormalizedModuleName": "jquery/2.2.4/jquery.min.js"
"unusedBytes": 35751
},
{
"name": "angular.js/1.3.20",
Expand Down Expand Up @@ -12454,16 +12453,9 @@
},
{
"name": "https://www.coursehero.com/sym-assets/js/bundle-8ed4419-3cfb752.js",
"resourceBytes": 1311780,
"resourceBytes": 1111780,
"unusedBytes": 685723,
"children": [
{
"_comment": "fake data, added for duplicatedNormalizedModuleName",
"name": "jquery/2.2.4/jquery.min.js",
"resourceBytes": 200000,
"unusedBytes": 0,
"duplicatedNormalizedModuleName": "jquery/2.2.4/jquery.min.js"
},
{
"name": "coursehero:///",
"resourceBytes": 1111780,
Expand Down
77 changes: 39 additions & 38 deletions lighthouse-treemap/app/src/main.js
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,6 @@ class TreemapViewer {
this.documentUrl = options.lhr.requestedUrl;
this.el = el;
this.getHueForD1NodeName = TreemapUtil.stableHasher(TreemapUtil.COLOR_HUES);
this.getHueForModuleNodeName = TreemapUtil.stableHasher(TreemapUtil.COLOR_HUES);

/* eslint-disable no-unused-expressions */
/** @type {LH.Treemap.Node} */
Expand Down Expand Up @@ -198,8 +197,8 @@ class TreemapViewer {
function createUnusedBytesViewMode(root) {
if (root.unusedBytes === undefined) return;

/** @type {LH.Treemap.NodePath[]} */
const highlightNodePaths = [];
/** @type {LH.Treemap.Highlight[]} */
const highlights = [];
for (const d1Node of root.children || []) {
// Only highlight leaf nodes if entire node (ie a JS bundle) has greater than a certain
// number of unused bytes.
Expand All @@ -213,14 +212,14 @@ class TreemapViewer {
return;
}

highlightNodePaths.push([root.name, ...path]);
highlights.push({path: [root.name, ...path]});
});
}
return {
id: 'unused-bytes',
label: 'Unused Bytes',
subLabel: TreemapUtil.formatBytes(root.unusedBytes),
highlightNodePaths,
highlights,
enabled: true,
};
}
Expand All @@ -230,7 +229,7 @@ class TreemapViewer {
* @return {LH.Treemap.ViewMode|undefined}
*/
const createDuplicateModulesViewMode = (root) => {
/** @type {Map<string, Array<{node: LH.Treemap.Node, path: string[]}>>} */
/** @type {Map<string, Array<{node: LH.Treemap.Node, path: LH.Treemap.NodePath}>>} */
const moduleNameToNodes = new Map();
for (const d1Node of root.children || []) {
TreemapUtil.walk(d1Node, (node, path) => {
Expand All @@ -243,11 +242,12 @@ class TreemapViewer {
});
}

const getHueForModuleNodeName = TreemapUtil.stableHasher(TreemapUtil.COLOR_HUES);
let potentialByteSavings = 0;

/** @type {LH.Treemap.NodePath[]} */
const highlightNodePaths = [];
for (const nodesWithSameModuleName of moduleNameToNodes.values()) {
/** @type {LH.Treemap.Highlight[]} */
const highlights = [];
for (const [moduleName, nodesWithSameModuleName] of moduleNameToNodes.entries()) {
if (nodesWithSameModuleName.length === 1) continue;

const bytes = [];
Expand All @@ -262,13 +262,16 @@ class TreemapViewer {
if (duplicatedBytes < DUPLICATED_MODULES_IGNORE_THRESHOLD) continue;

for (const {path} of nodesWithSameModuleName) {
highlightNodePaths.push([root.name, ...path]);
highlights.push({
path: [root.name, ...path],
color: this.getColorFromHue(getHueForModuleNodeName(moduleName)),
});
}
potentialByteSavings += duplicatedBytes;
}

let enabled = true;
if (highlightNodePaths.length === 0) enabled = false;
if (highlights.length === 0) enabled = false;
if (potentialByteSavings / root.resourceBytes < DUPLICATED_MODULES_IGNORE_ROOT_RATIO) {
enabled = false;
}
Expand All @@ -277,7 +280,7 @@ class TreemapViewer {
id: 'duplicate-modules',
label: 'Duplicate Modules',
subLabel: enabled ? TreemapUtil.formatBytes(potentialByteSavings) : 'N/A',
highlightNodePaths,
highlights,
enabled,
};
};
Expand Down Expand Up @@ -538,42 +541,40 @@ class TreemapViewer {
return parts.join(' · ');
}

/**
* @param {number} hue
*/
getColorFromHue(hue) {
return TreemapUtil.hsl(hue, 60, 90);
}

updateColors() {
TreemapUtil.walk(this.currentTreemapRoot, node => {
let hue;
if (this.currentViewMode.id === 'duplicate-modules') {
hue = this.getHueForModuleNodeName(node.duplicatedNormalizedModuleName || '');
} else {
// Color a depth one node and all children the same color.
const depthOneNode = this.nodeToDepthOneNodeMap.get(node);
hue = this.getHueForD1NodeName(depthOneNode ? depthOneNode.name : node.name);
}

let backgroundColor = 'white';
let color = 'black';

if (hue !== undefined) {
const sat = 60;
const lig = 90;
backgroundColor = TreemapUtil.hsl(hue, sat, lig);
color = lig > 50 ? 'black' : 'white';
} else {
// Ran out of colors.
}
// Color a depth one node and all children the same color.
const depthOneNode = this.nodeToDepthOneNodeMap.get(node);
const hue = depthOneNode &&
this.getHueForD1NodeName(depthOneNode ? depthOneNode.name : node.name);
const depthOneNodeColor = hue !== undefined ? this.getColorFromHue(hue) : 'white';

// A view can set nodes to highlight. If so, don't color anything else.
if (this.currentViewMode.highlightNodePaths) {
let backgroundColor;
if (this.currentViewMode.highlights) {
// A view can set nodes to highlight. If so, don't color anything else.
const path = this.nodeToPathMap.get(node);
const shouldHighlight = path && this.currentViewMode.highlightNodePaths
.some(pathToHighlight => TreemapUtil.pathsAreEqual(pathToHighlight, path));
if (!shouldHighlight) backgroundColor = 'white';
const highlight = path && this.currentViewMode.highlights
.find(highlight => TreemapUtil.pathsAreEqual(path, highlight.path));
if (highlight) {
backgroundColor = highlight.color || depthOneNodeColor;
} else {
backgroundColor = 'white';
}
} else {
backgroundColor = depthOneNodeColor;
}

// @ts-ignore: webtreemap will add a dom node property to every node.
const dom = /** @type {HTMLElement?} */ (node.dom);
if (dom) {
dom.style.backgroundColor = backgroundColor;
dom.style.color = color;
}
});
}
Expand Down
16 changes: 9 additions & 7 deletions lighthouse-treemap/app/src/util.js
Original file line number Diff line number Diff line change
Expand Up @@ -148,21 +148,23 @@ class TreemapUtil {
* The hash function is stable and deterministic, so the same key->item mapping will be
* produced given the same call order.
* @template T
* @param {T[]} items
* @return {(key: string) => T|undefined}
* @param {T[]} originalItems
* @return {(key: string) => T}
*/
static stableHasher(items) {
// Clone.
items = [...items];
static stableHasher(originalItems) {
let items = [...originalItems];

/** @type {Map<string, T>} */
const assignedItems = new Map();
return key => {
// Key has already been assigned an item.
if (assignedItems.has(key)) return assignedItems.get(key);
const alreadyAssignedItem = assignedItems.get(key);
if (alreadyAssignedItem !== undefined) return alreadyAssignedItem;

// Ran out of items.
if (items.length === 0) return;
if (items.length === 0) {
items = [...originalItems];
connorjclark marked this conversation as resolved.
Show resolved Hide resolved
}

// Select a random item using a stable hash.
const hash = [...key].reduce((acc, char) => acc + char.charCodeAt(0), 0);
Expand Down
13 changes: 11 additions & 2 deletions lighthouse-treemap/test/util-test.js
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ describe('TreemapUtil', () => {
});

it('stableHasher works', () => {
const values = [1, 2, 3, 4, 5];
let hasher = TreemapUtil.stableHasher([1, 2, 3, 4, 5]);
const expectedValues = [
hasher('value0'),
Expand All @@ -43,13 +44,18 @@ describe('TreemapUtil', () => {
hasher('value4'),
hasher('value5'),
];

for (const expectedValue of expectedValues) {
expect(values).toContain(expectedValue);
}

// Expect the same values using the same invocation.
expect(hasher('value0')).toBe(expectedValues[0]);
expect(hasher('value1')).toBe(expectedValues[1]);
expect(hasher('value2')).toBe(expectedValues[2]);
expect(hasher('value3')).toBe(expectedValues[3]);
expect(hasher('value4')).toBe(expectedValues[4]);
expect(hasher('value5')).toBeUndefined();
expect(hasher('value5')).toBe(expectedValues[5]);

// Repeat, expecting the same values.
hasher = TreemapUtil.stableHasher([1, 2, 3, 4, 5]);
Expand All @@ -58,6 +64,9 @@ describe('TreemapUtil', () => {
expect(hasher('value2')).toBe(expectedValues[2]);
expect(hasher('value3')).toBe(expectedValues[3]);
expect(hasher('value4')).toBe(expectedValues[4]);
expect(hasher('value5')).toBeUndefined();
expect(hasher('value5')).toBe(expectedValues[5]);

// Expect values array is not modified.
expect(values).toEqual([1, 2, 3, 4, 5]);
});
});
10 changes: 8 additions & 2 deletions types/treemap.d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,13 +17,19 @@ declare global {
value: string;
}

interface Highlight {
path: NodePath;
/** If not set, will use the color based on the d1Node. */
color?: string;
}

interface ViewMode {
id: 'all' | 'unused-bytes' | 'duplicate-modules';
label: string;
subLabel: string;
partitionBy?: 'resourceBytes' | 'unusedBytes';
highlightNodePaths?: NodePath[];
enabled: boolean;
partitionBy?: 'resourceBytes' | 'unusedBytes';
highlights?: Highlight[];
}

interface Node {
Expand Down