Skip to content

Commit 08f0ade

Browse files
fix(components): stop repeated node ids from looping the tree ancestry (#447)
A component report with repeated node ids could make the panel's parent map loop, so ownerOf and reveal walked it forever and froze the panel. The report guard now drops a node whose id the tree already used, along with its subtree. The panel keeps only the first row with an id in its index, and both ancestry walks stop at a repeated id. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com>
1 parent e4ff5c8 commit 08f0ade

7 files changed

Lines changed: 149 additions & 76 deletions

File tree

Lines changed: 23 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,23 @@
1+
import { describe, expect, it } from 'vitest';
2+
import { ancestors } from '../pages/component-tree';
3+
4+
describe('ancestors', () => {
5+
it('lists the ids above a node, nearest first', () => {
6+
const parents = new Map([
7+
['c3', 'c2'],
8+
['c2', 'c1'],
9+
]);
10+
expect(ancestors(parents, 'c3')).toEqual(['c2', 'c1']);
11+
expect(ancestors(parents, 'c1')).toEqual([]);
12+
});
13+
14+
it('stops when an id repeats instead of looping', () => {
15+
const parents = new Map([
16+
['a', 'b'],
17+
['b', 'c'],
18+
['c', 'b'],
19+
]);
20+
expect(ancestors(parents, 'a')).toEqual(['b', 'c']);
21+
expect(ancestors(new Map([['a', 'a']]), 'a')).toEqual([]);
22+
});
23+
});

‎app/src/pages/component-tree.ts‎

Lines changed: 15 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -178,6 +178,17 @@ function bare(name: string): string {
178178
return name.replace(/^_(?=[A-Z])/, '');
179179
}
180180

181+
/** The ids above `id`, nearest first; stops at a repeated id so a bad map cannot loop. */
182+
export function ancestors(parents: ReadonlyMap<string, string>, id: string): string[] {
183+
const out: string[] = [];
184+
const seen = new Set([id]);
185+
for (let parent = parents.get(id); parent && !seen.has(parent); parent = parents.get(parent)) {
186+
seen.add(parent);
187+
out.push(parent);
188+
}
189+
return out;
190+
}
191+
181192
const DIRECTIVE_HOSTS_KEY = 'pangular:components:directive-hosts';
182193

183194
/** Whether this viewer last chose to show directive hosts; shown unless they hid them. */
@@ -1822,6 +1833,8 @@ export class ComponentTree {
18221833
const parents = new Map<string, string>();
18231834
const walk = (nodes: LiveNode[], parent: string | null) => {
18241835
for (const node of nodes) {
1836+
// The first row with an id wins, so `parents` can never loop.
1837+
if (map.has(node.id)) continue;
18251838
map.set(node.id, node);
18261839
if (parent) parents.set(node.id, parent);
18271840
walk(node.children, node.id);
@@ -2401,7 +2414,7 @@ export class ComponentTree {
24012414
/** The nearest component above a row, such as the one whose template renders a directive host. */
24022415
ownerOf(id: string): LiveNode | null {
24032416
const { map, parents } = this.index();
2404-
for (let parent = parents.get(id); parent; parent = parents.get(parent)) {
2417+
for (const parent of ancestors(parents, id)) {
24052418
const node = map.get(parent);
24062419
if (node && node.kind !== 'directive') return node;
24072420
}
@@ -2447,7 +2460,7 @@ export class ComponentTree {
24472460
const { parents } = this.index();
24482461
this.collapsed.update((set) => {
24492462
const next = new Set(set);
2450-
for (let parent = parents.get(id); parent; parent = parents.get(parent)) next.delete(parent);
2463+
for (const parent of ancestors(parents, id)) next.delete(parent);
24512464
return next;
24522465
});
24532466
if (!this.rows().some((row) => row.node.id === id)) this.setFilter('');

extension/ui/assets/browser-agent-rpc-KH4HHvHf-Coz0PPwM.js renamed to extension/ui/assets/browser-agent-rpc-KH4HHvHf-DDC7kv8R.js

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 72 additions & 72 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

‎extension/ui/index.html‎

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

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

Lines changed: 33 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,6 +117,39 @@ describe('malformed component reports', () => {
117117
},
118118
]);
119119
});
120+
121+
it('drops a node whose id the tree already used, with its subtree', () => {
122+
const page = toComponentPage({
123+
pageId: 'p',
124+
count: 3,
125+
detail: null,
126+
roots: [
127+
{
128+
id: 'c1',
129+
name: 'App',
130+
tag: 'app-root',
131+
children: [
132+
{
133+
id: 'c2',
134+
name: 'Card',
135+
tag: 'app-card',
136+
kind: 'directive',
137+
children: [{ id: 'c1', name: 'App', tag: 'app-root', children: [] }],
138+
},
139+
],
140+
},
141+
{ id: 'c2', name: 'Again', tag: 'app-again', children: [] },
142+
] as never,
143+
});
144+
expect(page.roots).toEqual([
145+
{
146+
id: 'c1',
147+
name: 'App',
148+
tag: 'app-root',
149+
children: [{ id: 'c2', name: 'Card', tag: 'app-card', kind: 'directive', children: [] }],
150+
},
151+
]);
152+
});
120153
});
121154

122155
describe('page requests with no page to answer', () => {

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

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,13 @@ const records = (value: unknown): AnyRecord[] =>
4040
/** The roots with every node that has an `id`, `name` and `tag`; other nodes and their subtrees go. */
4141
export function guardComponentRoots(value: unknown): LiveComponentNode[] {
4242
let budget = MAX_NODES;
43+
// Ids index the tree in the panel and the tools; a repeated id is dropped with its subtree.
44+
const seen = new Set<string>();
4345
const node = (raw: unknown, depth: number): LiveComponentNode | null => {
4446
if (budget <= 0 || !isRecord(raw)) return null;
4547
if (!str(raw['id']) || !raw['id'] || !str(raw['name']) || !str(raw['tag'])) return null;
48+
if (seen.has(raw['id'])) return null;
49+
seen.add(raw['id']);
4650
budget--;
4751
const out: LiveComponentNode = {
4852
id: raw['id'],

0 commit comments

Comments
 (0)