Merge pull request #351 from preactjs/fix-hoc-reorder

Author: marvinhagemeister

src/adapter/shared/traverse.ts

@@ -113,6 +113,11 @@ export function shouldFilter<T extends SharedVNode>(
 		return true;
 	}
 
+	// TODO: Add a virtual root node to be able to filter the actual
+	// ones. Currently we have a workaround on the extension side
+	// that filters it there, but we should really do it here to be
+	// consistent with all other filters.
+
 	if (vnode.type === config.Fragment && filters.type.has("fragment")) {
 		const parent = bindings.getVNodeParent(vnode);
 		// Only filter non-root nodes
@@ -121,6 +126,12 @@ export function shouldFilter<T extends SharedVNode>(
 		return false;
 	} else if (bindings.isElement(vnode) && filters.type.has("dom")) {
 		return true;
+	} else if (filters.type.has("hoc")) {
+		const name = bindings.getDisplayName(vnode, config);
+
+		if (name.indexOf("(") > -1 && !name.startsWith("ForwardRef")) {
+			return true;
+		}
 	}
 
 	if (filters.regex.length > 0) {
@@ -169,75 +180,62 @@ function mount<T extends SharedVNode>(
 	const root = bindings.isRoot(vnode, config);
 
 	const skip = shouldFilter(vnode, filters, config, bindings);
-
-	if (root || !skip) {
-		record: {
-			let name = bindings.getDisplayName(vnode, config);
-
-			if (filters.type.has("hoc")) {
-				const hocName = getHocName(name);
-
-				// Filter out HOC-Components
-				if (hocName) {
-					if (name.startsWith("ForwardRef")) {
-						hocs = [...hocs, hocName];
-						const idx = name.indexOf("(");
-						name = name.slice(idx + 1, -1) || "Anonymous";
-					} else {
-						hocs = [...hocs, hocName];
-						break record;
-					}
-				}
+	let name = bindings.getDisplayName(vnode, config);
+
+	if (filters.type.has("hoc")) {
+		const hocName = getHocName(name);
+		if (hocName) {
+			hocs = [...hocs, hocName];
+			if (name.startsWith("ForwardRef")) {
+				const idx = name.indexOf("(");
+				name = name.slice(idx + 1, -1) || "Anonymous";
 			}
+		}
+	}
 
-			const id = getOrCreateVNodeId(ids, vnode);
-			if (bindings.isRoot(vnode, config)) {
-				commit.operations.push(MsgTypes.ADD_ROOT, id);
-			}
+	if (root || !skip) {
+		const id = getOrCreateVNodeId(ids, vnode);
+		if (bindings.isRoot(vnode, config)) {
+			commit.operations.push(MsgTypes.ADD_ROOT, id);
+		}
 
-			if (!timings.start.has(id)) {
-				timings.start.set(id, timingsByVNode.start.get(vnode) || 0);
-			}
-			if (!timings.end.has(id)) {
-				timings.end.set(id, timingsByVNode.end.get(vnode) || 0);
-			}
+		if (!timings.start.has(id)) {
+			timings.start.set(id, timingsByVNode.start.get(vnode) || 0);
+		}
+		if (!timings.end.has(id)) {
+			timings.end.set(id, timingsByVNode.end.get(vnode) || 0);
+		}
 
-			commit.operations.push(
-				MsgTypes.ADD_VNODE,
-				id,
-				getDevtoolsType(vnode, bindings), // Type
-				ancestorId,
-				9999, // owner
-				getStringId(commit.strings, name),
-				vnode.key ? getStringId(commit.strings, vnode.key) : 0,
-				// Multiply, because operations array only supports integers
-				// and would otherwise cut off floats
-				(timings.start.get(id) || 0) * 1000,
-				(timings.end.get(id) || 0) * 1000,
-			);
+		commit.operations.push(
+			MsgTypes.ADD_VNODE,
+			id,
+			getDevtoolsType(vnode, bindings), // Type
+			ancestorId,
+			9999, // owner
+			getStringId(commit.strings, name),
+			vnode.key ? getStringId(commit.strings, vnode.key) : 0,
+			// Multiply, because operations array only supports integers
+			// and would otherwise cut off floats
+			(timings.start.get(id) || 0) * 1000,
+			(timings.end.get(id) || 0) * 1000,
+		);
 
-			if (hocs.length > 0) {
-				addHocs(commit, id, hocs);
-				hocs = [];
-			}
+		if (hocs.length > 0) {
+			addHocs(commit, id, hocs);
+			hocs = [];
+		}
 
-			// Capture render reason (mount here)
-			if (profiler.isProfiling && profiler.captureRenderReasons) {
-				commit.operations.push(
-					MsgTypes.RENDER_REASON,
-					id,
-					RenderReason.MOUNT,
-					0,
-				);
-			}
+		// Capture render reason (mount here)
+		if (profiler.isProfiling && profiler.captureRenderReasons) {
+			commit.operations.push(MsgTypes.RENDER_REASON, id, RenderReason.MOUNT, 0);
+		}
 
-			updateHighlight(profiler, vnode, bindings);
+		updateHighlight(profiler, vnode, bindings);
 
-			ancestorId = id;
-		}
+		ancestorId = id;
 	}
 
-	if (skip && typeof vnode.type !== "function") {
+	if (skip && !bindings.isComponent(vnode)) {
 		const dom = bindings.getDom(vnode);
 		if (dom) domCache.set(dom, vnode);
 	}

src/view/components/elements/TreeView.tsx

@@ -169,7 +169,7 @@ export function MarkResult(props: { text: string; id: ID }) {
 		const start = idx > 0 ? text.slice(0, idx) : "";
 		const end = idx < text.length ? text.slice(idx + m[0].length) : "";
 		return (
-			<span ref={ref}>
+			<span ref={ref} data-testid="node-name">
 				{start}
 				<mark
 					class={`${s.mark} ${isActive ? s.markSelected : ""}`}
@@ -181,7 +181,7 @@ export function MarkResult(props: { text: string; id: ID }) {
 			</span>
 		);
 	}
-	return <span>{text}</span>;
+	return <span data-testid="node-name">{text}</span>;
 }
 
 export function TreeItem(props: { key: any; id: ID; top: number }) {

src/view/components/profiler/flamegraph/modes/FlamegraphLayout.tsx

@@ -47,13 +47,13 @@ export function FlamegraphLayout({
 				const node = commit.nodes.get(pos.id)!;
 				let text: any = "";
 				if (pos.commitParent || pos.weight === -1) {
-					text = node.name;
+					text = <span data-testid="node-name">{node.name}</span>;
 				} else {
 					const self = formatTime(commit.selfDurations.get(node.id)!);
 					const total = formatTime(node.endTime - node.startTime);
 					text = (
 						<>
-							{node.name}
+							<span data-testid="node-name">{node.name}</span>
 							{filterHoc && node.hocs ? (
 								<HocLabels hocs={node.hocs} nodeId={node.id} />
 							) : (

src/view/components/profiler/flamegraph/ranked/RankedLayout.tsx

@@ -68,7 +68,7 @@ export function RankedLayout({
 					);
 				const text = (
 					<>
-						{node.name}
+						<span data-testid="node-name">{node.name}</span>
 						{hocs} ({formatTime(selfDuration)})
 					</>
 				);

test-e2e/tests/hooks/hook-name.test.ts

@@ -1,6 +1,6 @@
 import { newTestPage, getHooks, waitForSelector } from "../../test-utils";
 import { expect } from "chai";
-import { clickNestedText } from "pentf/browser_utils";
+import { clickNestedText, clickSelector } from "pentf/browser_utils";
 
 export const description = "Show custom hook name";
 
@@ -10,7 +10,7 @@ export async function run(config: any) {
 	const hooksPanel = '[data-testid="props-row"]';
 
 	// Counter
-	await clickNestedText(devtools, /Counter$/, {
+	await clickSelector(devtools, '[data-name="Counter"]', {
 		retryUntil: async () => {
 			return !!(await devtools.$('[data-testid="Hooks"]'));
 		},

test-e2e/tests/profiler/flamegraph/utils.ts

@@ -3,13 +3,9 @@ import { Page } from "puppeteer";
 export async function getFlameNodes(page: Page) {
 	return await page.$$eval('[data-type="flamegraph"] > *', els => {
 		return els.map(el => {
-			const text = el.textContent!;
 			return {
 				maximized: el.hasAttribute("data-maximized"),
-				name: text.slice(
-					0,
-					text.includes("(") ? text.lastIndexOf("(") - 1 : text.length,
-				),
+				name: el.querySelector('[data-testid="node-name"]')!.textContent,
 				visible: el.hasAttribute("data-visible"),
 			};
 		});