fix(components): stop repeated node ids from looping the tree ancestry

app/src/__tests__/component-tree-ancestors.test.ts

@@ -0,0 +1,23 @@
+import { describe, expect, it } from 'vitest';
+import { ancestors } from '../pages/component-tree';
+
+describe('ancestors', () => {
+  it('lists the ids above a node, nearest first', () => {
+    const parents = new Map([
+      ['c3', 'c2'],
+      ['c2', 'c1'],
+    ]);
+    expect(ancestors(parents, 'c3')).toEqual(['c2', 'c1']);
+    expect(ancestors(parents, 'c1')).toEqual([]);
+  });
+
+  it('stops when an id repeats instead of looping', () => {
+    const parents = new Map([
+      ['a', 'b'],
+      ['b', 'c'],
+      ['c', 'b'],
+    ]);
+    expect(ancestors(parents, 'a')).toEqual(['b', 'c']);
+    expect(ancestors(new Map([['a', 'a']]), 'a')).toEqual([]);
+  });
+});

app/src/pages/component-tree.ts

@@ -178,6 +178,17 @@ function bare(name: string): string {
   return name.replace(/^_(?=[A-Z])/, '');
 }
 
+/** The ids above `id`, nearest first; stops at a repeated id so a bad map cannot loop. */
+export function ancestors(parents: ReadonlyMap<string, string>, id: string): string[] {
+  const out: string[] = [];
+  const seen = new Set([id]);
+  for (let parent = parents.get(id); parent && !seen.has(parent); parent = parents.get(parent)) {
+    seen.add(parent);
+    out.push(parent);
+  }
+  return out;
+}
+
 const DIRECTIVE_HOSTS_KEY = 'pangular:components:directive-hosts';
 
 /** Whether this viewer last chose to show directive hosts; shown unless they hid them. */
@@ -1822,6 +1833,8 @@ export class ComponentTree {
     const parents = new Map<string, string>();
     const walk = (nodes: LiveNode[], parent: string | null) => {
       for (const node of nodes) {
+        // The first row with an id wins, so `parents` can never loop.
+        if (map.has(node.id)) continue;
         map.set(node.id, node);
         if (parent) parents.set(node.id, parent);
         walk(node.children, node.id);
@@ -2401,7 +2414,7 @@ export class ComponentTree {
   /** The nearest component above a row, such as the one whose template renders a directive host. */
   ownerOf(id: string): LiveNode | null {
     const { map, parents } = this.index();
-    for (let parent = parents.get(id); parent; parent = parents.get(parent)) {
+    for (const parent of ancestors(parents, id)) {
       const node = map.get(parent);
       if (node && node.kind !== 'directive') return node;
     }
@@ -2447,7 +2460,7 @@ export class ComponentTree {
     const { parents } = this.index();
     this.collapsed.update((set) => {
       const next = new Set(set);
-      for (let parent = parents.get(id); parent; parent = parents.get(parent)) next.delete(parent);
+      for (const parent of ancestors(parents, id)) next.delete(parent);
       return next;
     });
     if (!this.rows().some((row) => row.node.id === id)) this.setFilter('');

packages/devtools/src/__tests__/rpc-boundary.test.ts

@@ -117,6 +117,39 @@ describe('malformed component reports', () => {
       },
     ]);
   });
+
+  it('drops a node whose id the tree already used, with its subtree', () => {
+    const page = toComponentPage({
+      pageId: 'p',
+      count: 3,
+      detail: null,
+      roots: [
+        {
+          id: 'c1',
+          name: 'App',
+          tag: 'app-root',
+          children: [
+            {
+              id: 'c2',
+              name: 'Card',
+              tag: 'app-card',
+              kind: 'directive',
+              children: [{ id: 'c1', name: 'App', tag: 'app-root', children: [] }],
+            },
+          ],
+        },
+        { id: 'c2', name: 'Again', tag: 'app-again', children: [] },
+      ] as never,
+    });
+    expect(page.roots).toEqual([
+      {
+        id: 'c1',
+        name: 'App',
+        tag: 'app-root',
+        children: [{ id: 'c2', name: 'Card', tag: 'app-card', kind: 'directive', children: [] }],
+      },
+    ]);
+  });
 });
 
 describe('page requests with no page to answer', () => {

packages/devtools/src/rpc/component-report-guard.ts

@@ -40,9 +40,13 @@ const records = (value: unknown): AnyRecord[] =>
 /** The roots with every node that has an `id`, `name` and `tag`; other nodes and their subtrees go. */
 export function guardComponentRoots(value: unknown): LiveComponentNode[] {
   let budget = MAX_NODES;
+  // Ids index the tree in the panel and the tools; a repeated id is dropped with its subtree.
+  const seen = new Set<string>();
   const node = (raw: unknown, depth: number): LiveComponentNode | null => {
     if (budget <= 0 || !isRecord(raw)) return null;
     if (!str(raw['id']) || !raw['id'] || !str(raw['name']) || !str(raw['tag'])) return null;
+    if (seen.has(raw['id'])) return null;
+    seen.add(raw['id']);
     budget--;
     const out: LiveComponentNode = {
       id: raw['id'],