Repository: preactjs/preact-devtools
Pull request #368 (2022-05-18 12:00:01)

Accurately track rendered nodes in a commit


Patch:
diff --git a/src/adapter/protocol/events.ts b/src/adapter/protocol/events.ts
index 3ee3b3b..a80cb6f 100644
--- a/src/adapter/protocol/events.ts
+++ b/src/adapter/protocol/events.ts
@@ -107,11 +107,14 @@ export function flush(commit: Commit) {
  * We currently expect all operations to be in order.
  */
 export function applyOperationsV2(store: Store, data: number[]) {
-	const { rootId: commitRootId, roots, tree, reasons, stats } = ops2Tree(
-		store.nodes.$,
-		store.roots.$,
-		data,
-	);
+	const {
+		rootId: commitRootId,
+		rendered,
+		roots,
+		tree,
+		reasons,
+		stats,
+	} = ops2Tree(store.nodes.$, store.roots.$, data);
 
 	// Update store data
 	store.roots.$ = roots;
@@ -127,7 +130,7 @@ export function applyOperationsV2(store: Store, data: number[]) {
 	// If we are profiling, we'll make a frozen copy of the mutable
 	// elements tree because the profiler can step through time
 	if (store.profiler.isRecording.$) {
-		recordProfilerCommit(store.nodes.$, store.profiler, commitRootId);
+		recordProfilerCommit(store.nodes.$, store.profiler, rendered, commitRootId);
 		store.profiler.renderReasons.update(m => {
 			m.set(commitRootId, reasons);
 		});
diff --git a/src/adapter/protocol/operations.ts b/src/adapter/protocol/operations.ts
index ec2b997..142c5d6 100644
--- a/src/adapter/protocol/operations.ts
+++ b/src/adapter/protocol/operations.ts
@@ -17,6 +17,7 @@ export function ops2Tree(oldTree: Tree, existingRoots: ID[], ops: number[]) {
 	const rootId = ops[0];
 	const roots: ID[] = [...existingRoots];
 	const removals: ID[] = [];
+	const rendered = new Set<ID>();
 	const reasons: RenderReasonMap = new Map();
 	let stats: ParsedStats | null = null;
 
@@ -51,6 +52,9 @@ export function ops2Tree(oldTree: Tree, existingRoots: ID[], ops: number[]) {
 					startTime: ops[i + 7] / 1000,
 					endTime: ops[i + 8] / 1000,
 				});
+
+				rendered.add(id);
+
 				i += 8;
 				break;
 			}
@@ -61,6 +65,8 @@ export function ops2Tree(oldTree: Tree, existingRoots: ID[], ops: number[]) {
 				x.startTime = ops[i + 2] / 1000;
 				x.endTime = ops[i + 3] / 1000;
 
+				rendered.add(id);
+
 				i += 3;
 				break;
 			}
@@ -159,5 +165,5 @@ export function ops2Tree(oldTree: Tree, existingRoots: ID[], ops: number[]) {
 		}
 	}
 
-	return { rootId, roots, tree: pending, removals, reasons, stats };
+	return { rootId, roots, tree: pending, removals, rendered, reasons, stats };
 }
diff --git a/src/view/components/profiler/components/RenderReasons.tsx b/src/view/components/profiler/components/RenderReasons.tsx
index 200cff1..46b37e9 100644
--- a/src/view/components/profiler/components/RenderReasons.tsx
+++ b/src/view/components/profiler/components/RenderReasons.tsx
@@ -42,18 +42,8 @@ export function RenderReasons() {
 
 	const hasReasons = reason !== null && reason.items && reason.items.length > 0;
 
-	let rendered = false;
-	if (!captureReason && commit) {
-		const root = commit.nodes.get(commit.commitRootId);
-		if (
-			root &&
-			selected &&
-			selected.startTime >= root.startTime &&
-			selected.endTime <= root.endTime
-		) {
-			rendered = true;
-		}
-	}
+	const rendered =
+		!captureReason && commit && selected && commit.rendered.has(selected.id);
 
 	return (
 		<SidebarPanel title="Render reasons">
diff --git a/src/view/components/profiler/components/RenderedAt/RenderedAt.tsx b/src/view/components/profiler/components/RenderedAt/RenderedAt.tsx
index d0a9fb9..72a0bf5 100644
--- a/src/view/components/profiler/components/RenderedAt/RenderedAt.tsx
+++ b/src/view/components/profiler/components/RenderedAt/RenderedAt.tsx
@@ -9,24 +9,21 @@ export function RenderedAt() {
 	const data = useObserver(() => {
 		const id = store.profiler.selectedNodeId.$;
 
-		return store.profiler.commits.$.filter(x => {
-			if (x.nodes.has(id)) {
-				const node = x.nodes.get(id)!;
-				const root = x.nodes.get(x.commitRootId)!;
-				return node.startTime >= root.startTime && node.endTime <= root.endTime;
-			}
+		return store.profiler.commits.$.reduce<
+			Array<{ index: number; startTime: number; selfDuration: number }>
+		>((acc, commit, i) => {
+			if (!commit.rendered.has(id)) return acc;
 
-			return false;
-		}).map(commit => {
 			const node = commit.nodes.get(id)!;
 
 			const selfDuration = commit.selfDurations.get(id) || 0;
-			return {
-				index: store.profiler.commits.$.findIndex(x => x === commit),
+			acc.push({
+				index: i,
 				startTime: node.startTime,
 				selfDuration,
-			};
-		});
+			});
+			return acc;
+		}, []);
 	});
 
 	const commitIdx = useObserver(() => store.profiler.activeCommitIdx.$);
diff --git a/src/view/components/profiler/data/commits.ts b/src/view/components/profiler/data/commits.ts
index 9b743d9..77df149 100644
--- a/src/view/components/profiler/data/commits.ts
+++ b/src/view/components/profiler/data/commits.ts
@@ -14,6 +14,8 @@ export interface CommitData {
 	rootId: ID;
 	/** Id of the node the commit was triggered on */
 	commitRootId: ID;
+	/** Nodes that are part of the current commit */
+	rendered: Set<ID>;
 	maxSelfDuration: number;
 	duration: number;
 	nodes: Map<ID, DevNode>;
@@ -204,6 +206,7 @@ export function resetProfiler(state: ProfilerState) {
 export function recordProfilerCommit(
 	tree: Map<ID, DevNode>,
 	profiler: ProfilerState,
+	rendered: Set<ID>,
 	commitRootId: number,
 ) {
 	const commitRoot = tree.get(commitRootId)!;
@@ -274,6 +277,7 @@ export function recordProfilerCommit(
 		arr.push({
 			rootId: getRoot(tree, commitRootId),
 			commitRootId: commitRootId,
+			rendered,
 			nodes,
 			maxSelfDuration,
 			duration: totalCommitDuration,
diff --git a/src/view/components/profiler/flamegraph/modes/flamegraph-utils.ts b/src/view/components/profiler/flamegraph/modes/flamegraph-utils.ts
index 0dfc3a2..706fa28 100644
--- a/src/view/components/profiler/flamegraph/modes/flamegraph-utils.ts
+++ b/src/view/components/profiler/flamegraph/modes/flamegraph-utils.ts
@@ -29,7 +29,13 @@ export function placeFlamegraph(
 		parentId = node.parent;
 	}
 
-	parentId = tree.get(selectedId)!.parent;
+	// Account for commits not having the currently selected node
+	// TODO: Move this logic elsewhere
+	const selected = !tree.has(selectedId)
+		? tree.get(rootId)!
+		: tree.get(selectedId)!;
+
+	parentId = selected.parent;
 	while (parentId !== -1) {
 		const node = tree.get(parentId);
 		if (node === undefined) break;
diff --git a/src/view/components/profiler/flamegraph/modes/patchTree.ts b/src/view/components/profiler/flamegraph/modes/patchTree.ts
index db15695..d5e1d64 100644
--- a/src/view/components/profiler/flamegraph/modes/patchTree.ts
+++ b/src/view/components/profiler/flamegraph/modes/patchTree.ts
@@ -32,6 +32,10 @@ export function patchTree(commit: CommitData) {
 export function getSelfDuration(commit: CommitData, node: DevNode) {
 	const duration = node.endTime - node.startTime;
 	const children = node.children.reduce((acc, childId) => {
+		if (!commit.rendered.has(childId)) {
+			return acc;
+		}
+
 		const child = commit.nodes.get(childId);
 		if (!child) return 0;
 
@@ -65,7 +69,7 @@ function placeNode(
 		commitParent: false,
 		maximized: false,
 		visible: false,
-		weight: commit.selfDurations.has(id) ? getGradient(50, selfDuration) : -1,
+		weight: commit.rendered.has(id) ? getGradient(50, selfDuration) : -1,
 		width: selfDuration,
 	};
 	idToTransform.set(id, nodePos);
diff --git a/src/view/components/profiler/flamegraph/placeNodes.ts b/src/view/components/profiler/flamegraph/placeNodes.ts
index 00ece1f..8f61702 100644
--- a/src/view/components/profiler/flamegraph/placeNodes.ts
+++ b/src/view/components/profiler/flamegraph/placeNodes.ts
@@ -1,4 +1,3 @@
-import { CommitData } from "../data/commits";
 import { DevNode } from "../../../store/types";
 
 /**
@@ -32,15 +31,6 @@ export function flattenNodeTree<K, T extends { id: K; children: K[] }>(
 	return out;
 }
 
-export const EMPTY_COMMIT: CommitData = {
-	commitRootId: -1,
-	duration: -1,
-	maxSelfDuration: 1,
-	nodes: new Map(),
-	selfDurations: new Map(),
-	rootId: -1,
-};
-
 export const EMPTY: DevNode = {
 	children: [],
 	depth: 0,


Test patch:
diff --git a/src/adapter/protocol/operations.test.ts b/src/adapter/protocol/operations.test.ts
index 682e12f..a79bd9b 100644
--- a/src/adapter/protocol/operations.test.ts
+++ b/src/adapter/protocol/operations.test.ts
@@ -20,6 +20,7 @@ describe("ops2Tree", () => {
 			expect(ops2Tree(new Map(), [], ops)).to.deep.equal({
 				rootId: 1,
 				roots: [1],
+				rendered: new Set(),
 				removals: [],
 				tree: new Map(),
 				reasons: new Map(),
diff --git a/src/view/components/profiler/flamegraph/modes/patchTree.test.ts b/src/view/components/profiler/flamegraph/modes/patchTree.test.ts
index 35d88f8..36e0c4c 100644
--- a/src/view/components/profiler/flamegraph/modes/patchTree.test.ts
+++ b/src/view/components/profiler/flamegraph/modes/patchTree.test.ts
@@ -25,6 +25,7 @@ export function patch(tree: Tree, rootId: ID, commitRootId: ID) {
 		patchTree({
 			nodes: tree,
 			rootId,
+			rendered: new Set(tree.keys()),
 			commitRootId,
 			duration: 100,
 			maxSelfDuration: 100,
@@ -146,7 +147,7 @@ describe("patchTree", () => {
 	it("should enlarge parent", () => {
 		const a = flames`
 			App ****
-			 Bar *****	
+			 Bar *****
 		`;
 
 		const actual = patch(a.idMap, 1, 2);
diff --git a/src/view/components/profiler/flamegraph/testHelpers.test.ts b/src/view/components/profiler/flamegraph/testHelpers.test.ts
index 7d7cad3..60a1f2a 100644
--- a/src/view/components/profiler/flamegraph/testHelpers.test.ts
+++ b/src/view/components/profiler/flamegraph/testHelpers.test.ts
@@ -273,6 +273,7 @@ describe("FlameGraph DSL", () => {
 			commitRootId: 1,
 			rootId: 1,
 			duration: 110,
+			rendered: new Set(tree.commit.nodes.keys()),
 			maxSelfDuration: 60,
 			nodes: tree.idMap,
 			selfDurations: tree.commit.selfDurations,
diff --git a/src/view/components/profiler/flamegraph/testHelpers.ts b/src/view/components/profiler/flamegraph/testHelpers.ts
index 32d72ed..209cf51 100644
--- a/src/view/components/profiler/flamegraph/testHelpers.ts
+++ b/src/view/components/profiler/flamegraph/testHelpers.ts
@@ -207,6 +207,7 @@ export function flames(
 	const commit: CommitData = {
 		rootId: 1,
 		commitRootId: 1,
+		rendered: new Set(nodes.map(node => node.id)),
 		duration: nodes.length > 0 ? nodes[0]!.endTime - nodes[0]!.startTime : 0,
 		maxSelfDuration: Math.max(
 			0,
diff --git a/test-e2e/test-utils.ts b/test-e2e/test-utils.ts
index 18772a4..ba5372e 100644
--- a/test-e2e/test-utils.ts
+++ b/test-e2e/test-utils.ts
@@ -256,7 +256,7 @@ export async function clickRecordButton(page: Page) {
 	const start = /Start/.test(await getAttribute(page, selector, "title"));
 	await clickSelector(page, selector, {
 		async retryUntil() {
-			await page.evaluate(
+			return await page.evaluate(
 				(s, start) => {
 					return (start ? /Stop Recording/ : /Start Recording/).test(
 						document.querySelector(s).title,